Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 2 Aug 2018 23:46:31 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r337205 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <201808022346.w72NkV5U024376@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Thu Aug  2 23:46:30 2018
New Revision: 337205
URL: https://svnweb.freebsd.org/changeset/base/337205

Log:
  MFV r337204: 9439 ZFS double-free due to failure to dirty indirect block
  
  illumos/illumos-gate@99a19144e82244f3426f055cc73af8a937c0135c
  
  Reviewed by: George Wilson <george.wilson@delphix.com>
  Reviewed by: Paul Dagnelie <pcd@delphix.com>
  Approved by: Robert Mustacchi <rm@joyent.com>
  Author:     Matthew Ahrens <mahrens@delphix.com>

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode_sync.c
Directory Properties:
  head/sys/cddl/contrib/opensolaris/   (props changed)

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c	Thu Aug  2 23:45:24 2018	(r337204)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c	Thu Aug  2 23:46:30 2018	(r337205)
@@ -1616,13 +1616,11 @@ dnode_free_range(dnode_t *dn, uint64_t off, uint64_t l
 		if (off == 0 && len >= blksz) {
 			/*
 			 * Freeing the whole block; fast-track this request.
-			 * Note that we won't dirty any indirect blocks,
-			 * which is fine because we will be freeing the entire
-			 * file and thus all indirect blocks will be freed
-			 * by free_children().
 			 */
 			blkid = 0;
 			nblks = 1;
+			if (dn->dn_nlevels > 1)
+				dnode_dirty_l1(dn, 0, tx);
 			goto done;
 		} else if (off >= blksz) {
 			/* Freeing past end-of-data */

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode_sync.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode_sync.c	Thu Aug  2 23:45:24 2018	(r337204)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode_sync.c	Thu Aug  2 23:46:30 2018	(r337205)
@@ -263,6 +263,24 @@ free_children(dmu_buf_impl_t *db, uint64_t blkid, uint
 	if (db->db_state != DB_CACHED)
 		(void) dbuf_read(db, NULL, DB_RF_MUST_SUCCEED);
 
+	/*
+	 * If we modify this indirect block, and we are not freeing the
+	 * dnode (!free_indirects), then this indirect block needs to get
+	 * written to disk by dbuf_write().  If it is dirty, we know it will
+	 * be written (otherwise, we would have incorrect on-disk state
+	 * because the space would be freed but still referenced by the BP
+	 * in this indirect block).  Therefore we VERIFY that it is
+	 * dirty.
+	 *
+	 * Our VERIFY covers some cases that do not actually have to be
+	 * dirty, but the open-context code happens to dirty.  E.g. if the
+	 * blocks we are freeing are all holes, because in that case, we
+	 * are only freeing part of this indirect block, so it is an
+	 * ancestor of the first or last block to be freed.  The first and
+	 * last L1 indirect blocks are always dirtied by dnode_free_range().
+	 */
+	VERIFY(BP_GET_FILL(db->db_blkptr) == 0 || db->db_dirtycnt > 0);
+
 	dbuf_release_bp(db);
 	bp = db->db.db_data;
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808022346.w72NkV5U024376>