From 5b09abdb34fdf32479377b0142c2f8757ff268d9 Mon Sep 17 00:00:00 2001 From: asomers Date: Mon, 18 Nov 2013 16:51:56 +0000 Subject: [PATCH] opensolaris/uts/common/dtrace/fasttrap.c Fix several problems that can cause panics on kldload and kldunload. * kproc_create(fasttrap_pid_cleanup_cb, ...) gets called before fasttrap_provs.fth_table gets allocated. This can lead to a panic on module load, because fasttrap_pid_cleanup_cb references fasttrap_provs.fth_table. Move kproc_create down after the point that fasttrap_provs.fth_table gets allocated, and modify the error handling accordingly. * dtrace_fasttrap_{fork,exec,exit} weren't getting NULLed until after fasttrap_provs.fth_table got freed. That caused panics on module unload because fasttrap_exec_exit calls fasttrap_provider_retire, which references fasttrap_provs.fth_table. NULL those function pointers earlier. * There wasn't any code to destroy the fasttrap_{tpoints,provs,procs}.fth_table mutexes on module unload, leading to a resource leak when WITNESS is enabled. Destroy those mutexes during fasttrap_unload(). Reviewed by: markj Approved by: ken (mentor) Sponsored by: Spectra Logic MFC after: 4 weeks --- .../opensolaris/uts/common/dtrace/fasttrap.c | 63 ++++++++++++------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c b/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c index bf46eb79ca03..fc468b065026 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c +++ b/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c @@ -2271,13 +2271,6 @@ fasttrap_load(void) mutex_init(&fasttrap_count_mtx, "fasttrap count mtx", MUTEX_DEFAULT, NULL); - ret = kproc_create(fasttrap_pid_cleanup_cb, NULL, - &fasttrap_cleanup_proc, 0, 0, "ftcleanup"); - if (ret != 0) { - destroy_dev(fasttrap_cdev); - return (ret); - } - #if defined(sun) fasttrap_max = ddi_getprop(DDI_DEV_T_ANY, devi, DDI_PROP_DONTPASS, "fasttrap-max-probes", FASTTRAP_MAX_DEFAULT); @@ -2331,6 +2324,24 @@ fasttrap_load(void) "providers bucket mtx", MUTEX_DEFAULT, NULL); #endif + ret = kproc_create(fasttrap_pid_cleanup_cb, NULL, + &fasttrap_cleanup_proc, 0, 0, "ftcleanup"); + if (ret != 0) { + destroy_dev(fasttrap_cdev); +#if !defined(sun) + for (i = 0; i < fasttrap_provs.fth_nent; i++) + mutex_destroy(&fasttrap_provs.fth_table[i].ftb_mtx); + for (i = 0; i < fasttrap_tpoints.fth_nent; i++) + mutex_destroy(&fasttrap_tpoints.fth_table[i].ftb_mtx); +#endif + kmem_free(fasttrap_provs.fth_table, fasttrap_provs.fth_nent * + sizeof (fasttrap_bucket_t)); + mtx_destroy(&fasttrap_cleanup_mtx); + mutex_destroy(&fasttrap_count_mtx); + return (ret); + } + + /* * ... and the procs hash table. */ @@ -2423,6 +2434,20 @@ fasttrap_unload(void) return (-1); } + /* + * Stop new processes from entering these hooks now, before the + * fasttrap_cleanup thread runs. That way all processes will hopefully + * be out of these hooks before we free fasttrap_provs.fth_table + */ + ASSERT(dtrace_fasttrap_fork == &fasttrap_fork); + dtrace_fasttrap_fork = NULL; + + ASSERT(dtrace_fasttrap_exec == &fasttrap_exec_exit); + dtrace_fasttrap_exec = NULL; + + ASSERT(dtrace_fasttrap_exit == &fasttrap_exec_exit); + dtrace_fasttrap_exit = NULL; + mtx_lock(&fasttrap_cleanup_mtx); fasttrap_cleanup_drain = 1; /* Wait for the cleanup thread to finish up and signal us. */ @@ -2438,6 +2463,14 @@ fasttrap_unload(void) mutex_exit(&fasttrap_count_mtx); #endif +#if !defined(sun) + for (i = 0; i < fasttrap_tpoints.fth_nent; i++) + mutex_destroy(&fasttrap_tpoints.fth_table[i].ftb_mtx); + for (i = 0; i < fasttrap_provs.fth_nent; i++) + mutex_destroy(&fasttrap_provs.fth_table[i].ftb_mtx); + for (i = 0; i < fasttrap_procs.fth_nent; i++) + mutex_destroy(&fasttrap_procs.fth_table[i].ftb_mtx); +#endif kmem_free(fasttrap_tpoints.fth_table, fasttrap_tpoints.fth_nent * sizeof (fasttrap_bucket_t)); fasttrap_tpoints.fth_nent = 0; @@ -2450,22 +2483,6 @@ fasttrap_unload(void) fasttrap_procs.fth_nent * sizeof (fasttrap_bucket_t)); fasttrap_procs.fth_nent = 0; - /* - * We know there are no tracepoints in any process anywhere in - * the system so there is no process which has its p_dtrace_count - * greater than zero, therefore we know that no thread can actively - * be executing code in fasttrap_fork(). Similarly for p_dtrace_probes - * and fasttrap_exec() and fasttrap_exit(). - */ - ASSERT(dtrace_fasttrap_fork == &fasttrap_fork); - dtrace_fasttrap_fork = NULL; - - ASSERT(dtrace_fasttrap_exec == &fasttrap_exec_exit); - dtrace_fasttrap_exec = NULL; - - ASSERT(dtrace_fasttrap_exit == &fasttrap_exec_exit); - dtrace_fasttrap_exit = NULL; - #if !defined(sun) destroy_dev(fasttrap_cdev); mutex_destroy(&fasttrap_count_mtx);