6332949 V240 panics with kmem_flags set, buffer corruption in streams/Dtrace while testing arbitrary patches

6340028 assertion failed: dtrace_deferred_pid != NULL, file: ../../common/dtrace/dtrace.c, line: 12485

3 files changed, 64 insertions, 35 deletions

diff --git a/usr/src/uts/common/dtrace/dtrace.c b/usr/src/uts/common/dtrace/dtrace.c
index 028b6219fe..5974bf5486 100644
--- a/usr/src/uts/common/dtrace/dtrace.c
+++ b/usr/src/uts/common/dtrace/dtrace.c
@@ -6696,6 +6696,7 @@ dtrace_meta_register(const char *name, const dtrace_mops_t *mops, void *arg,
 		next = help->dthps_next;
 		help->dthps_next = NULL;
 		help->dthps_prev = NULL;
+		help->dthps_deferred = 0;
 		help = next;
 	}
 
@@ -12080,6 +12081,7 @@ dtrace_helper_provider_register(proc_t *p, dtrace_helpers_t *help,
 
 		if (help->dthps_next == NULL && help->dthps_prev == NULL &&
 		    dtrace_deferred_pid != help) {
+			help->dthps_deferred = 1;
 			help->dthps_pid = p->p_pid;
 			help->dthps_next = dtrace_deferred_pid;
 			help->dthps_prev = NULL;
@@ -12397,7 +12399,7 @@ dtrace_helper_slurp(dof_hdr_t *dof, dof_helper_t *dhp)
 
 	if (dhp != NULL) {
 		uintptr_t daddr = (uintptr_t)dof;
-		int err = 0;
+		int err = 0, count = 0;
 
 		/*
 		 * Look for helper probes.
@@ -12413,10 +12415,13 @@ dtrace_helper_slurp(dof_hdr_t *dof, dof_helper_t *dhp)
 				err = 1;
 				break;
 			}
+
+			count++;
 		}
 
 		dhp->dofhp_dof = (uint64_t)(uintptr_t)dof;
-		if (err == 0 && dtrace_helper_provider_add(dhp) == 0)
+		if (err == 0 && count > 0 &&
+		    dtrace_helper_provider_add(dhp) == 0)
 			destroy = 0;
 		else
 			dhp = NULL;
@@ -12506,7 +12511,10 @@ dtrace_helpers_destroy(void)
 			}
 		} else {
 			mutex_enter(&dtrace_lock);
-			ASSERT(dtrace_deferred_pid != NULL);
+			ASSERT(help->dthps_deferred == 0 ||
+			    help->dthps_next != NULL ||
+			    help->dthps_prev != NULL ||
+			    help == dtrace_deferred_pid);
 
 			/*
 			 * Remove the helper from the deferred list.
diff --git a/usr/src/uts/common/dtrace/fasttrap.c b/usr/src/uts/common/dtrace/fasttrap.c
index be164bd353..57fb51c44b 100644
--- a/usr/src/uts/common/dtrace/fasttrap.c
+++ b/usr/src/uts/common/dtrace/fasttrap.c
@@ -158,8 +158,8 @@ static void fasttrap_tracepoint_disable(proc_t *, fasttrap_probe_t *, uint_t);
 
 static fasttrap_provider_t *fasttrap_provider_lookup(pid_t, const char *,
     const dtrace_pattr_t *);
+static void fasttrap_provider_retire(pid_t, const char *, int);
 static void fasttrap_provider_free(fasttrap_provider_t *);
-static void fasttrap_provider_retire(fasttrap_provider_t *);
 
 static fasttrap_proc_t *fasttrap_proc_lookup(pid_t);
 static void fasttrap_proc_release(fasttrap_proc_t *);
@@ -441,8 +441,6 @@ fasttrap_fork(proc_t *p, proc_t *cp)
 static void
 fasttrap_exec_exit(proc_t *p)
 {
-	fasttrap_provider_t *provider;
-
 	ASSERT(p == curproc);
 	ASSERT(MUTEX_HELD(&p->p_lock));
 
@@ -451,10 +449,11 @@ fasttrap_exec_exit(proc_t *p)
 	/*
 	 * We clean up the pid provider for this process here; user-land
 	 * static probes are handled by the meta-provider remove entry point.
+	 * Note that the consumer count is not artificially elevated on the
+	 * pid provider as it is on USDT providers so there's no need to drop
+	 * it here.
 	 */
-	if ((provider = fasttrap_provider_lookup(p->p_pid,
-	    FASTTRAP_PID_NAME, NULL)) != NULL)
-		fasttrap_provider_retire(provider);
+	fasttrap_provider_retire(p->p_pid, FASTTRAP_PID_NAME, 0);
 
 	mutex_enter(&p->p_lock);
 }
@@ -1282,6 +1281,7 @@ fasttrap_provider_lookup(pid_t pid, const char *name,
 	uid_t uid = (uid_t)-1;
 
 	ASSERT(strlen(name) < sizeof (fp->ftp_name));
+	ASSERT(pattr != NULL);
 
 	bucket = &fasttrap_provs.fth_table[FASTTRAP_PROVS_INDEX(pid, name)];
 	mutex_enter(&bucket->ftb_mtx);
@@ -1305,12 +1305,6 @@ fasttrap_provider_lookup(pid_t pid, const char *name,
 	mutex_exit(&bucket->ftb_mtx);
 
 	/*
-	 * If we didn't want to create a new provider, just return failure.
-	 */
-	if (pattr == NULL)
-		return (NULL);
-
-	/*
 	 * Make sure the process exists, isn't a child created as the result
 	 * of a vfork(2), and isn't a zombie (but may be in fork). Record the
 	 * process's uid to pass to dtrace_register().
@@ -1428,9 +1422,28 @@ fasttrap_provider_free(fasttrap_provider_t *provider)
 }
 
 static void
-fasttrap_provider_retire(fasttrap_provider_t *provider)
+fasttrap_provider_retire(pid_t pid, const char *name, int ccount)
 {
-	dtrace_provider_id_t provid = provider->ftp_provid;
+	fasttrap_provider_t *fp;
+	fasttrap_bucket_t *bucket;
+	dtrace_provider_id_t provid;
+
+	ASSERT(strlen(name) < sizeof (fp->ftp_name));
+	ASSERT(ccount == 0 || ccount == 1);
+
+	bucket = &fasttrap_provs.fth_table[FASTTRAP_PROVS_INDEX(pid, name)];
+	mutex_enter(&bucket->ftb_mtx);
+
+	for (fp = bucket->ftb_data; fp != NULL; fp = fp->ftp_next) {
+		if (fp->ftp_pid == pid && strcmp(fp->ftp_name, name) == 0 &&
+		    !fp->ftp_retired)
+			break;
+	}
+
+	if (fp == NULL) {
+		mutex_exit(&bucket->ftb_mtx);
+		return;
+	}
 
 	/*
 	 * Mark the provider to be removed in our post-processing step,
@@ -1440,11 +1453,22 @@ fasttrap_provider_retire(fasttrap_provider_t *provider)
 	 * setting the retired flag indicates that we're done with this
 	 * provider; setting the proc to be defunct indicates that all
 	 * tracepoints associated with the traced process should be ignored.
+	 * We also drop the consumer count here by the amount specified.
+	 *
+	 * We obviously need to take the bucket lock before the provider lock
+	 * to perform the lookup, but we need to drop the provider lock
+	 * before calling into the DTrace framework since we acquire the
+	 * provider lock in callbacks invoked from the DTrace framework. The
+	 * bucket lock therefore protects the integrity of the provider hash
+	 * table.
 	 */
-	provider->ftp_proc->ftpc_defunct = 1;
-	provider->ftp_retired = 1;
-	provider->ftp_marked = 1;
-	mutex_exit(&provider->ftp_mtx);
+	mutex_enter(&fp->ftp_mtx);
+	fp->ftp_proc->ftpc_defunct = 1;
+	fp->ftp_retired = 1;
+	fp->ftp_marked = 1;
+	fp->ftp_ccount -= ccount;
+	provid = fp->ftp_provid;
+	mutex_exit(&fp->ftp_mtx);
 
 	/*
 	 * We don't have to worry about invalidating the same provider twice
@@ -1453,6 +1477,8 @@ fasttrap_provider_retire(fasttrap_provider_t *provider)
 	 */
 	dtrace_invalidate(provid);
 
+	mutex_exit(&bucket->ftb_mtx);
+
 	fasttrap_pid_cleanup();
 }
 
@@ -1750,19 +1776,13 @@ fasttrap_meta_create_probe(void *arg, void *parg,
 static void
 fasttrap_meta_remove(void *arg, dtrace_helper_provdesc_t *dhpv, pid_t pid)
 {
-	fasttrap_provider_t *provider;
-
-	if ((provider = fasttrap_provider_lookup(pid,
-	    dhpv->dthpv_provname, NULL)) != NULL) {
-		/*
-		 * Drop the consumer count now that we're done with this
-		 * provider. If there are no other consumers retire it now.
-		 */
-		if (--provider->ftp_ccount == 0)
-			fasttrap_provider_retire(provider);
-		else
-			mutex_exit(&provider->ftp_mtx);
-	}
+	/*
+	 * Clean up the USDT provider. There may be active consumers of the
+	 * provider busy adding probes, no damage will actually befall the
+	 * provider until that count has dropped to zero. This just puts
+	 * the provider on death row.
+	 */
+	fasttrap_provider_retire(pid, dhpv->dthpv_provname, 1);
 }
 
 static dtrace_mops_t fasttrap_mops = {
@@ -2118,7 +2138,7 @@ fasttrap_detach(dev_info_t *devi, ddi_detach_cmd_t cmd)
 			 * waiting for any other consumer to finish with
 			 * this provider. A thread must first acquire the
 			 * bucket lock so there's no chance of another thread
-			 * blocking on the providers lock.
+			 * blocking on the provider's lock.
 			 */
 			mutex_enter(&fp->ftp_mtx);
 			mutex_exit(&fp->ftp_mtx);
diff --git a/usr/src/uts/common/sys/dtrace_impl.h b/usr/src/uts/common/sys/dtrace_impl.h
index 26d72dec22..fdb284d192 100644
--- a/usr/src/uts/common/sys/dtrace_impl.h
+++ b/usr/src/uts/common/sys/dtrace_impl.h
@@ -1005,6 +1005,7 @@ typedef struct dtrace_helpers {
 	uint_t dthps_nprovs;			/* count of providers */
 	int dthps_generation;			/* current generation */
 	pid_t dthps_pid;			/* pid of associated proc */
+	int dthps_deferred;			/* helper in deferred list */
 	struct dtrace_helpers *dthps_next;	/* next pointer */
 	struct dtrace_helpers *dthps_prev;	/* prev pointer */
 } dtrace_helpers_t;