MFC r286541: 5531 NULL pointer dereference in dsl_prop_get_ds()

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Dan McDonald <danmcd@omniti.com>
Reviewed by: George Wilson <george@delphix.com>
Reviewed by: Bayard Bell <buffer.g.overflow@gmail.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Justin T. Gibbs <justing@spectralogic.com>

illumos/illumos-gate@e57a022b8f718889ffa92adbde47a8f08abcdb25

1 files changed, 38 insertions, 7 deletions

diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_prop.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_prop.c
index 398c571..f67a296 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_prop.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_prop.c
@@ -442,9 +442,31 @@ dsl_prop_notify_all_cb(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg)
 	    cbr = list_next(&dd->dd_prop_cbs, cbr)) {
 		uint64_t value;
 
+		/*
+		 * Callback entries do not have holds on their datasets
+		 * so that datasets with registered callbacks are still
+		 * eligible for eviction.  Unlike operations on callbacks
+		 * for a single dataset, we are performing a recursive
+		 * descent of related datasets and the calling context
+		 * for this iteration only has a dataset hold on the root.
+		 * Without a hold, the callback's pointer to the dataset
+		 * could be invalidated by eviction at any time.
+		 *
+		 * Use dsl_dataset_try_add_ref() to verify that the
+		 * dataset has not begun eviction processing and to
+		 * prevent eviction from occurring for the duration
+		 * of the callback.  If the hold attempt fails, this
+		 * object is already being evicted and the callback can
+		 * be safely ignored.
+		 */
+		if (!dsl_dataset_try_add_ref(dp, cbr->cbr_ds, FTAG))
+			continue;
+
 		if (dsl_prop_get_ds(cbr->cbr_ds, cbr->cbr_propname,
 		    sizeof (value), 1, &value, NULL) == 0)
 			cbr->cbr_func(cbr->cbr_arg, value);
+
+		dsl_dataset_rele(cbr->cbr_ds, FTAG);
 	}
 	mutex_exit(&dd->dd_lock);
 
@@ -497,19 +519,28 @@ dsl_prop_changed_notify(dsl_pool_t *dp, uint64_t ddobj,
 	mutex_enter(&dd->dd_lock);
 	for (cbr = list_head(&dd->dd_prop_cbs); cbr;
 	    cbr = list_next(&dd->dd_prop_cbs, cbr)) {
-		uint64_t propobj = dsl_dataset_phys(cbr->cbr_ds)->ds_props_obj;
+		uint64_t propobj;
 
-		if (strcmp(cbr->cbr_propname, propname) != 0)
+		/*
+		 * cbr->cbf_ds may be invalidated due to eviction,
+		 * requiring the use of dsl_dataset_try_add_ref().
+		 * See comment block in dsl_prop_notify_all_cb()
+		 * for details.
+		 */
+		if (strcmp(cbr->cbr_propname, propname) != 0 ||
+		    !dsl_dataset_try_add_ref(dp, cbr->cbr_ds, FTAG))
 			continue;
 
+		propobj = dsl_dataset_phys(cbr->cbr_ds)->ds_props_obj;
+
 		/*
-		 * If the property is set on this ds, then it is not
-		 * inherited here; don't call the callback.
+		 * If the property is not set on this ds, then it is
+		 * inherited here; call the callback.
 		 */
-		if (propobj && 0 == zap_contains(mos, propobj, propname))
-			continue;
+		if (propobj == 0 || zap_contains(mos, propobj, propname) != 0)
+			cbr->cbr_func(cbr->cbr_arg, value);
 
-		cbr->cbr_func(cbr->cbr_arg, value);
+		dsl_dataset_rele(cbr->cbr_ds, FTAG);
 	}
 	mutex_exit(&dd->dd_lock);