Fix some lock order reversals where we called free() while holding a proc

lock.  We now use temporary variables to save the process argument pointer
and just update the pointer while holding the lock.  We then perform the
free on the cached pointer after releasing the lock.

2 files changed, 14 insertions, 11 deletions

diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index ec663a6..ecadfed 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -110,6 +110,7 @@ execve(p, uap)
 	struct image_params image_params, *imgp;
 	struct vattr attr;
 	int (*img_first) __P((struct image_params *));
+	struct pargs *pa;
 
 	imgp = &image_params;
 
@@ -385,23 +386,24 @@ interpret:
 	    imgp->ps_strings);
 
 	/* Free any previous argument cache */
-	if (p->p_args && --p->p_args->ar_ref == 0)
-		FREE(p->p_args, M_PARGS);
+	pa = p->p_args;
 	p->p_args = NULL;
+	PROC_UNLOCK(p);
+	if (pa != NULL && --pa->ar_ref == 0)
+		FREE(pa, M_PARGS);
 
 	/* Cache arguments if they fit inside our allowance */
 	i = imgp->endargs - imgp->stringbase;
 	if (ps_arg_cache_limit >= i + sizeof(struct pargs)) {
-		PROC_UNLOCK(p);
-		MALLOC(p->p_args, struct pargs *, sizeof(struct pargs) + i, 
+		MALLOC(pa, struct pargs *, sizeof(struct pargs) + i, 
 		    M_PARGS, M_WAITOK);
-		KASSERT(p->p_args != NULL, ("malloc of p_args failed"));
+		pa->ar_ref = 1;
+		pa->ar_length = i;
+		bcopy(imgp->stringbase, pa->ar_args, i);
 		PROC_LOCK(p);
-		p->p_args->ar_ref = 1;
-		p->p_args->ar_length = i;
-		bcopy(imgp->stringbase, p->p_args->ar_args, i);
+		p->p_args = pa;
+		PROC_UNLOCK(p);
 	}
-	PROC_UNLOCK(p);
 
 exec_fail_dealloc:
 
diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index f45ac26..883e2fa 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -709,10 +709,11 @@ sysctl_kern_proc_args(SYSCTL_HANDLER_ARGS)
 		return (error);
 
 	PROC_LOCK(p);
-	if (p->p_args && --p->p_args->ar_ref == 0) 
-		FREE(p->p_args, M_PARGS);
+	pa = p->p_args;
 	p->p_args = NULL;
 	PROC_UNLOCK(p);
+	if (pa != NULL && --pa->ar_ref == 0) 
+		FREE(pa, M_PARGS);
 
 	if (req->newlen + sizeof(struct pargs) > ps_arg_cache_limit)
 		return (error);