Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Apr 2002 11:41:39 -0700 (PDT)
From:      John Baldwin <jhb@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 9523 for review
Message-ID:  <200204101841.g3AIfdQ56595@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://people.freebsd.org/~peter/p4db/chv.cgi?CH=9523

Change 9523 by jhb@jhb_laptop on 2002/04/10 11:41:27

	Merge a bunch of stuff over from jhb_proc.

Affected files ...

... //depot/projects/smpng/sys/alpha/osf1/osf1_misc.c#14 integrate
... //depot/projects/smpng/sys/compat/linprocfs/linprocfs.c#15 edit
... //depot/projects/smpng/sys/compat/linux/linux_misc.c#18 edit
... //depot/projects/smpng/sys/compat/linux/linux_uid16.c#10 edit
... //depot/projects/smpng/sys/compat/svr4/svr4_sysvec.c#5 edit
... //depot/projects/smpng/sys/ddb/db_ps.c#6 edit
... //depot/projects/smpng/sys/fs/procfs/procfs_ctl.c#9 edit
... //depot/projects/smpng/sys/fs/procfs/procfs_dbregs.c#7 edit
... //depot/projects/smpng/sys/fs/procfs/procfs_fpregs.c#7 edit
... //depot/projects/smpng/sys/fs/procfs/procfs_mem.c#5 edit
... //depot/projects/smpng/sys/fs/procfs/procfs_regs.c#7 edit
... //depot/projects/smpng/sys/fs/procfs/procfs_status.c#10 edit
... //depot/projects/smpng/sys/fs/pseudofs/pseudofs_vnops.c#13 edit
... //depot/projects/smpng/sys/kern/kern_exec.c#33 edit
... //depot/projects/smpng/sys/kern/kern_exit.c#31 edit
... //depot/projects/smpng/sys/kern/kern_fork.c#35 edit
... //depot/projects/smpng/sys/kern/kern_intr.c#16 edit
... //depot/projects/smpng/sys/kern/kern_ktrace.c#19 edit
... //depot/projects/smpng/sys/kern/kern_prot.c#57 edit
... //depot/projects/smpng/sys/kern/kern_resource.c#20 edit
... //depot/projects/smpng/sys/kern/kern_sig.c#30 edit
... //depot/projects/smpng/sys/kern/sys_process.c#13 edit
... //depot/projects/smpng/sys/security/lomac/kernel_log.c#3 edit
... //depot/projects/smpng/sys/sys/proc.h#43 edit

Differences ...

==== //depot/projects/smpng/sys/alpha/osf1/osf1_misc.c#14 (text+ko) ====

@@ -1060,13 +1060,18 @@
 
 	p = td->td_proc;
 	uid = SCARG(uap, uid);
+	newcred = crget();
+	PROC_LOCK(p);
 	oldcred = p->p_ucred;
 
 	if ((error = suser_cred(p->p_ucred, PRISON_ROOT)) != 0 &&
-	    uid != oldcred->cr_ruid && uid != oldcred->cr_svuid)
+	    uid != oldcred->cr_ruid && uid != oldcred->cr_svuid) {
+		PROC_UNLOCK(p);
+		crfree(newcred);
 		return (error);
+	}
 
-	newcred = crdup(oldcred);
+	crcopy(newcred, oldcred);
 	if (error == 0) {
 		if (uid != oldcred->cr_ruid) {
 			change_ruid(newcred, uid);
@@ -1082,6 +1087,7 @@
 		setsugid(p);
 	}
 	p->p_ucred = newcred;
+	PROC_UNLOCK(p);
 	crfree(oldcred);
 	return (0);
 }
@@ -1106,13 +1112,18 @@
 
 	p = td->td_proc;
 	gid = SCARG(uap, gid);
+	newcred = crget();
+	PROC_LOCK(p);
 	oldcred = p->p_ucred;
 
 	if (((error = suser_cred(p->p_ucred, PRISON_ROOT)) != 0 ) &&
-	    gid != oldcred->cr_rgid && gid != oldcred->cr_svgid)
+	    gid != oldcred->cr_rgid && gid != oldcred->cr_svgid) {
+		PROC_UNLOCK(p);
+		crfree(newcred);
 		return (error);
+	}
 
-	newcred = crdup(oldcred);
+	crcopy(newcred, oldcred);
 	if (error == 0) {
 		if (gid != oldcred->cr_rgid) {
 			change_rgid(newcred, gid);
@@ -1128,6 +1139,7 @@
 		setsugid(p);
 	}
 	p->p_ucred = newcred;
+	PROC_UNLOCK(p);
 	crfree(oldcred);
 	return (0);
 }

==== //depot/projects/smpng/sys/compat/linprocfs/linprocfs.c#15 (text+ko) ====

@@ -672,23 +672,21 @@
 	 * Linux behaviour is to return zero-length in this case.
 	 */
 
-	if (ps_argsopen || !p_cansee(td->td_proc, p)) {
-		PROC_LOCK(p);
-		if (p->p_args) {
-			sbuf_bcpy(sb, p->p_args->ar_args, p->p_args->ar_length);
-			PROC_UNLOCK(p);
-		} else if (p != td->td_proc) {
-			sbuf_printf(sb, "%.*s", MAXCOMLEN, p->p_comm);
-			PROC_UNLOCK(p);
-		} else {
-			PROC_UNLOCK(p);
-			error = copyin((void*)PS_STRINGS, &pstr, sizeof(pstr));
-			if (error)
-				return (error);
-			for (i = 0; i < pstr.ps_nargvstr; i++) {
-				sbuf_copyin(sb, pstr.ps_argvstr[i], 0);
-				sbuf_printf(sb, "%c", '\0');
-			}
+	PROC_LOCK(p);
+	if (p->p_args && (ps_argsopen || !p_cansee(td->td_proc, p))) {
+		sbuf_bcpy(sb, p->p_args->ar_args, p->p_args->ar_length);
+		PROC_UNLOCK(p);
+	} else if (p != td->td_proc) {
+		PROC_UNLOCK(p);
+		sbuf_printf(sb, "%.*s", MAXCOMLEN, p->p_comm);
+	} else {
+		PROC_UNLOCK(p);
+		error = copyin((void*)PS_STRINGS, &pstr, sizeof(pstr));
+		if (error)
+			return (error);
+		for (i = 0; i < pstr.ps_nargvstr; i++) {
+			sbuf_copyin(sb, pstr.ps_argvstr[i], 0);
+			sbuf_printf(sb, "%c", '\0');
 		}
 	}
 

==== //depot/projects/smpng/sys/compat/linux/linux_misc.c#18 (text+ko) ====

@@ -970,9 +970,19 @@
 	l_gid_t linux_gidset[NGROUPS];
 	gid_t *bsd_gidset;
 	int ngrp, error;
+	struct proc *p;
 
 	ngrp = args->gidsetsize;
-	oldcred = td->td_proc->p_ucred;
+	if (ngrp >= NGROUPS)
+		return (EINVAL);
+	error = copyin((caddr_t)args->grouplist, linux_gidset,
+	    ngrp * sizeof(l_gid_t));
+	if (error)
+		return (error);
+	newcred = crget();
+	p = td->td_proc;
+	PROC_LOCK(p);
+	oldcred = p->p_ucred;
 
 	/*
 	 * cr_groups[0] holds egid. Setting the whole set from
@@ -980,19 +990,14 @@
 	 * Keep cr_groups[0] unchanged to prevent that.
 	 */
 
-	if ((error = suser_cred(oldcred, PRISON_ROOT)) != 0)
+	if ((error = suser_cred(oldcred, PRISON_ROOT)) != 0) {
+		PROC_UNLOCK(p);
+		crfree(newcred);
 		return (error);
+	}
 
-	if (ngrp >= NGROUPS)
-		return (EINVAL);
-
-	newcred = crdup(oldcred);
+	crcopy(newcred, oldcred);
 	if (ngrp > 0) {
-		error = copyin((caddr_t)args->grouplist, linux_gidset,
-			       ngrp * sizeof(l_gid_t));
-		if (error)
-			return (error);
-
 		newcred->cr_ngroups = ngrp + 1;
 
 		bsd_gidset = newcred->cr_groups;
@@ -1005,8 +1010,9 @@
 	else
 		newcred->cr_ngroups = 1;
 
-	setsugid(td->td_proc);
-	td->td_proc->p_ucred = newcred;
+	setsugid(p);
+	p->p_ucred = newcred;
+	PROC_UNLOCK(p);
 	crfree(oldcred);
 	return (0);
 }
@@ -1019,7 +1025,7 @@
 	gid_t *bsd_gidset;
 	int bsd_gidsetsz, ngrp, error;
 
-	cred = td->td_proc->p_ucred;
+	cred = td->td_ucred;
 	bsd_gidset = cred->cr_groups;
 	bsd_gidsetsz = cred->cr_ngroups - 1;
 
@@ -1310,7 +1316,7 @@
 linux_getgid(struct thread *td, struct linux_getgid_args *args)
 {
 
-	td->td_retval[0] = td->td_proc->p_ucred->cr_rgid;
+	td->td_retval[0] = td->td_ucred->cr_rgid;
 	return (0);
 }
 
@@ -1318,7 +1324,7 @@
 linux_getuid(struct thread *td, struct linux_getuid_args *args)
 {
 
-	td->td_retval[0] = td->td_proc->p_ucred->cr_ruid;
+	td->td_retval[0] = td->td_ucred->cr_ruid;
 	return (0);
 }
 

==== //depot/projects/smpng/sys/compat/linux/linux_uid16.c#10 (text+ko) ====

@@ -30,6 +30,8 @@
 
 #include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/lock.h>
+#include <sys/mutex.h>
 #include <sys/proc.h>
 #include <sys/sysproto.h>
 
@@ -93,6 +95,7 @@
 	l_gid16_t linux_gidset[NGROUPS];
 	gid_t *bsd_gidset;
 	int ngrp, error;
+	struct proc *p;
 
 #ifdef DEBUG
 	if (ldebug(setgroups16))
@@ -100,7 +103,16 @@
 #endif
 
 	ngrp = args->gidsetsize;
-	oldcred = td->td_proc->p_ucred;
+	if (ngrp >= NGROUPS)
+		return (EINVAL);
+	error = copyin((caddr_t)args->gidset, linux_gidset,
+	    ngrp * sizeof(l_gid16_t));
+	if (error)
+		return (error);
+	newcred = crget();
+	p = td->td_proc;
+	PROC_LOCK(p);
+	oldcred = p->p_ucred;
 
 	/*
 	 * cr_groups[0] holds egid. Setting the whole set from
@@ -108,19 +120,14 @@
 	 * Keep cr_groups[0] unchanged to prevent that.
 	 */
 
-	if ((error = suser_cred(oldcred, PRISON_ROOT)) != 0)
+	if ((error = suser_cred(oldcred, PRISON_ROOT)) != 0) {
+		PROC_UNLOCK(p);
+		crfree(newcred);
 		return (error);
+	}
 
-	if (ngrp >= NGROUPS)
-		return (EINVAL);
-
-	newcred = crdup(oldcred);
+	crcopy(newcred, oldcred);
 	if (ngrp > 0) {
-		error = copyin((caddr_t)args->gidset, linux_gidset,
-		    ngrp * sizeof(l_gid16_t));
-		if (error)
-			return (error);
-
 		newcred->cr_ngroups = ngrp + 1;
 
 		bsd_gidset = newcred->cr_groups;
@@ -134,7 +141,8 @@
 		newcred->cr_ngroups = 1;
 
 	setsugid(td->td_proc);
-	td->td_proc->p_ucred = newcred;
+	p->p_ucred = newcred;
+	PROC_UNLOCK(p);
 	crfree(oldcred);
 	return (0);
 }

==== //depot/projects/smpng/sys/compat/svr4/svr4_sysvec.c#5 (text+ko) ====

@@ -44,7 +44,9 @@
 #include <sys/imgact.h>
 #include <sys/imgact_elf.h>
 #include <sys/socket.h>
+#include <sys/lock.h>
 #include <sys/malloc.h>
+#include <sys/mutex.h>
 #include <sys/namei.h>
 #include <sys/vnode.h>
 #include <sys/module.h>
@@ -213,10 +215,12 @@
 	AUXARGS_ENTRY(pos, AT_FLAGS, args->flags);
 	AUXARGS_ENTRY(pos, AT_ENTRY, args->entry);
 	AUXARGS_ENTRY(pos, AT_BASE, args->base);
+	PROC_LOCK(imgp->proc);
 	AUXARGS_ENTRY(pos, AT_UID, imgp->proc->p_ucred->cr_ruid);
 	AUXARGS_ENTRY(pos, AT_EUID, imgp->proc->p_ucred->cr_svuid);
 	AUXARGS_ENTRY(pos, AT_GID, imgp->proc->p_ucred->cr_rgid);
 	AUXARGS_ENTRY(pos, AT_EGID, imgp->proc->p_ucred->cr_svgid);
+	PROC_UNLOCK(imgp->proc);
 	AUXARGS_ENTRY(pos, AT_NULL, 0);
 	
 	free(imgp->auxargs, M_TEMP);      

==== //depot/projects/smpng/sys/ddb/db_ps.c#6 (text+ko) ====

@@ -55,6 +55,7 @@
 
 	np = nprocs;
 
+	/* sx_slock(&allproc_lock); */
 	if (!LIST_EMPTY(&allproc))
 		p = LIST_FIRST(&allproc);
 	else
@@ -90,6 +91,7 @@
 			printf("oops, ran out of processes early!\n");
 			break;
 		}
+		/* PROC_LOCK(p); */
 		pp = p->p_pptr;
 		if (pp == NULL)
 			pp = p;
@@ -126,8 +128,11 @@
 			}
 			db_printf(" %s\n", p->p_comm);
 		}
+		/* PROC_UNLOCK(p); */
+
 		p = LIST_NEXT(p, p_list);
 		if (p == NULL && np > 0)
 			p = LIST_FIRST(&zombproc);
     	}
+	/* sx_sunlock(&allproc_lock); */
 }

==== //depot/projects/smpng/sys/fs/procfs/procfs_ctl.c#9 (text+ko) ====

@@ -108,37 +108,29 @@
 	{ 0 },
 };
 
-static int	procfs_control(struct proc *curp, struct proc *p, int op);
+static int	procfs_control(struct thread *td, struct proc *p, int op);
 
 static int
-procfs_control(struct proc *curp, struct proc *p, int op)
+procfs_control(struct thread *td, struct proc *p, int op)
 {
 	int error = 0;
 
 	/*
-	 * Authorization check: rely on normal debugging protection, except
-	 * allow processes to disengage debugging on a process onto which
-	 * they have previously attached, but no longer have permission to
-	 * debug.
-	 */
-	if (op != PROCFS_CTL_DETACH &&
-	    ((error = p_candebug(curp, p))))
-		return (error);
-
-	/*
 	 * Attach - attaches the target process for debugging
 	 * by the calling process.
 	 */
 	if (op == PROCFS_CTL_ATTACH) {
 		sx_xlock(&proctree_lock);
 		PROC_LOCK(p);
+		if ((error = p_candebug(td->td_proc, p)) != 0)
+			goto out;
 		if (p->p_flag & P_TRACED) {
 			error = EBUSY;
 			goto out;
 		}
 
 		/* Can't trace yourself! */
-		if (p->p_pid == curp->p_pid) {
+		if (p->p_pid == td->td_proc->p_pid) {
 			error = EINVAL;
 			goto out;
 		}
@@ -154,9 +146,9 @@
 		p->p_flag |= P_TRACED;
 		faultin(p);
 		p->p_xstat = 0;		/* XXX ? */
-		if (p->p_pptr != curp) {
+		if (p->p_pptr != td->td_proc) {
 			p->p_oppid = p->p_pptr->p_pid;
-			proc_reparent(p, curp);
+			proc_reparent(p, td->td_proc);
 		}
 		psignal(p, SIGSTOP);
 out:
@@ -166,7 +158,20 @@
 	}
 
 	/*
-	 * Target process must be stopped, owned by (curp) and
+	 * Authorization check: rely on normal debugging protection, except
+	 * allow processes to disengage debugging on a process onto which
+	 * they have previously attached, but no longer have permission to
+	 * debug.
+	 */
+	PROC_LOCK(p);
+	if (op != PROCFS_CTL_DETACH &&
+	    ((error = p_candebug(td->td_proc, p)))) {
+		PROC_UNLOCK(p);
+		return (error);
+	}
+
+	/*
+	 * Target process must be stopped, owned by (td) and
 	 * be set up for tracing (P_TRACED flag set).
 	 * Allow DETACH to take place at any time for sanity.
 	 * Allow WAIT any time, of course.
@@ -177,15 +182,10 @@
 		break;
 
 	default:
-		PROC_LOCK(p);
-		mtx_lock_spin(&sched_lock);
-		if (!TRACE_WAIT_P(curp, p)) {
-			mtx_unlock_spin(&sched_lock);
+		if (!TRACE_WAIT_P(td->td_proc, p)) {
 			PROC_UNLOCK(p);
 			return (EBUSY);
 		}
-		mtx_unlock_spin(&sched_lock);
-		PROC_UNLOCK(p);
 	}
 
 
@@ -201,7 +201,6 @@
 	 * To continue with a signal, just send
 	 * the signal name to the ctl file
 	 */
-	PROC_LOCK(p);
 	p->p_xstat = 0;
 
 	switch (op) {
@@ -241,7 +240,7 @@
 		PROC_UNLOCK(p);
 		sx_xunlock(&proctree_lock);
 
-		wakeup((caddr_t) curp);	/* XXX for CTL_WAIT below ? */
+		wakeup((caddr_t) td->td_proc);	/* XXX for CTL_WAIT below ? */
 
 		break;
 
@@ -272,31 +271,19 @@
 	 */
 	case PROCFS_CTL_WAIT:
 		if (p->p_flag & P_TRACED) {
-			mtx_lock_spin(&sched_lock);
 			while (error == 0 &&
 					(p->p_stat != SSTOP) &&
 					(p->p_flag & P_TRACED) &&
-					(p->p_pptr == curp)) {
-				mtx_unlock_spin(&sched_lock);
+					(p->p_pptr == td->td_proc))
 				error = msleep((caddr_t) p, &p->p_mtx,
 						PWAIT|PCATCH, "procfsx", 0);
-				mtx_lock_spin(&sched_lock);
-			}
-			if (error == 0 && !TRACE_WAIT_P(curp, p))
+			if (error == 0 && !TRACE_WAIT_P(td->td_proc, p))
 				error = EBUSY;
-			mtx_unlock_spin(&sched_lock);
-			PROC_UNLOCK(p);
-		} else {
-			PROC_UNLOCK(p);
-			mtx_lock_spin(&sched_lock);
-			while (error == 0 && p->p_stat != SSTOP) {
-				mtx_unlock_spin(&sched_lock);
-				error = tsleep((caddr_t) p,
+		} else
+			while (error == 0 && p->p_stat != SSTOP)
+				error = msleep((caddr_t) p, &p->p_mtx,
 						PWAIT|PCATCH, "procfs", 0);
-				mtx_lock_spin(&sched_lock);
-			}
-			mtx_unlock_spin(&sched_lock);
-		}
+		PROC_UNLOCK(p);
 		return (error);
 
 	default:
@@ -346,13 +333,12 @@
 	nm = findname(ctlnames, sbuf_data(sb), sbuf_len(sb));
 	if (nm) {
 		printf("procfs: got a %s command\n", sbuf_data(sb));
-		error = procfs_control(td->td_proc, p, nm->nm_val);
+		error = procfs_control(td, p, nm->nm_val);
 	} else {
 		nm = findname(signames, sbuf_data(sb), sbuf_len(sb));
 		if (nm) {
 			printf("procfs: got a sig%s\n", sbuf_data(sb));
 			PROC_LOCK(p);
-			mtx_lock_spin(&sched_lock);
 
 			/* This is very broken XXXKSE: */
 			if (TRACE_WAIT_P(td->td_proc, p)) {
@@ -361,13 +347,12 @@
 				/* XXXKSE: */
 				FIX_SSTEP(FIRST_THREAD_IN_PROC(p));
 #endif
+				mtx_lock_spin(&sched_lock);
 				/* XXXKSE: */
 				setrunnable(FIRST_THREAD_IN_PROC(p));
 				mtx_unlock_spin(&sched_lock);
-			} else {
-				mtx_unlock_spin(&sched_lock);
+			} else
 				psignal(p, nm->nm_val);
-			}
 			PROC_UNLOCK(p);
 			error = 0;
 		}

==== //depot/projects/smpng/sys/fs/procfs/procfs_dbregs.c#7 (text+ko) ====

@@ -68,8 +68,11 @@
 	char *kv;
 	int kl;
 
-	if (p_candebug(td->td_proc, p))
+	PROC_LOCK(p);
+	if (p_candebug(td->td_proc, p) != 0) {
+		PROC_UNLOCK(p);
 		return (EPERM);
+	}
 	kl = sizeof(r);
 	kv = (char *) &r;
 
@@ -78,7 +81,7 @@
 	if (kl > uio->uio_resid)
 		kl = uio->uio_resid;
 
-	PHOLD(p);
+	_PHOLD(p);
 	if (kl < 0)
 		error = EINVAL;
 	else
@@ -93,7 +96,8 @@
 			/* XXXKSE: */
 			error = proc_write_dbregs(FIRST_THREAD_IN_PROC(p), &r);
 	}
-	PRELE(p);
+	_PRELE(p);
+	PROC_UNLOCK(p);
 
 	uio->uio_offset = 0;
 	return (error);

==== //depot/projects/smpng/sys/fs/procfs/procfs_fpregs.c#7 (text+ko) ====

@@ -62,8 +62,11 @@
 	char *kv;
 	int kl;
 
-	if (p_candebug(td->td_proc, p))
+	PROC_LOCK(p);
+	if (p_candebug(td->td_proc, p)) {
+		PROC_UNLOCK(p);
 		return (EPERM);
+	}
 	kl = sizeof(r);
 	kv = (char *) &r;
 
@@ -72,7 +75,7 @@
 	if (kl > uio->uio_resid)
 		kl = uio->uio_resid;
 
-	PHOLD(p);
+	_PHOLD(p);
 	if (kl < 0)
 		error = EINVAL;
 	else
@@ -87,7 +90,8 @@
 			/* XXXKSE: */
 			error = proc_write_fpregs(FIRST_THREAD_IN_PROC(p), &r);
 	}
-	PRELE(p);
+	_PRELE(p);
+	PROC_UNLOCK(p);
 
 	uio->uio_offset = 0;
 	return (error);

==== //depot/projects/smpng/sys/fs/procfs/procfs_mem.c#5 (text+ko) ====

@@ -64,10 +64,11 @@
 	if (uio->uio_resid == 0)
 		return (0);
 
+	PROC_LOCK(p);
 	error = p_candebug(td->td_proc, p);
-	if (error)
-		return (error);
-	error = proc_rwmem(p, uio);
+	PROC_UNLOCK(p);
+	if (error == 0)
+		error = proc_rwmem(p, uio);
 
 	return (error);
 }

==== //depot/projects/smpng/sys/fs/procfs/procfs_regs.c#7 (text+ko) ====

@@ -62,8 +62,11 @@
 	char *kv;
 	int kl;
 
-	if (p_candebug(td->td_proc, p))
+	PROC_LOCK(p);
+	if (p_candebug(td->td_proc, p)) {
+		PROC_UNLOCK(p);
 		return (EPERM);
+	}
 	kl = sizeof(r);
 	kv = (char *) &r;
 
@@ -72,7 +75,8 @@
 	if (kl > uio->uio_resid)
 		kl = uio->uio_resid;
 
-	PHOLD(p);
+	_PHOLD(p);
+	PROC_UNLOCK(p);
 	if (kl < 0)
 		error = EINVAL;
 	else
@@ -80,6 +84,7 @@
 		error = proc_read_regs(FIRST_THREAD_IN_PROC(p), &r);
 	if (error == 0)
 		error = uiomove(kv, kl, uio);
+	PROC_LOCK(p);
 	if (error == 0 && uio->uio_rw == UIO_WRITE) {
 		if (p->p_stat != SSTOP)
 			error = EBUSY;
@@ -87,7 +92,8 @@
 			/* XXXKSE: */
 			error = proc_write_regs(FIRST_THREAD_IN_PROC(p), &r);
 	}
-	PRELE(p);
+	_PRELE(p);
+	PROC_UNLOCK(p);
 
 	uio->uio_offset = 0;
 	return (error);

==== //depot/projects/smpng/sys/fs/procfs/procfs_status.c#10 (text+ko) ====

@@ -109,7 +109,6 @@
 		sep = ",";
 	}
 	SESS_UNLOCK(sess);
-	PROC_UNLOCK(p);
 	if (*sep != ',') {
 		sbuf_printf(sb, "noflags");
 	}
@@ -160,6 +159,7 @@
 	} else {
 		sbuf_printf(sb, " -");
 	}
+	PROC_UNLOCK(p);
 	sbuf_printf(sb, "\n");
 
 	return (0);
@@ -181,23 +181,22 @@
 	 * Linux behaviour is to return zero-length in this case.
 	 */
 
-	if (ps_argsopen || !p_cansee(td->td_proc, p)) {
-		PROC_LOCK(p);
-		if (p->p_args) {
-			sbuf_bcpy(sb, p->p_args->ar_args, p->p_args->ar_length);
-			PROC_UNLOCK(p);
-		} else if (p != td->td_proc) {
-			sbuf_printf(sb, "%.*s", MAXCOMLEN, p->p_comm);
-			PROC_UNLOCK(p);
-		} else {
-			PROC_UNLOCK(p);
-			error = copyin((void*)PS_STRINGS, &pstr, sizeof(pstr));
-			if (error)
-				return (error);
-			for (i = 0; i < pstr.ps_nargvstr; i++) {
-				sbuf_copyin(sb, pstr.ps_argvstr[i], 0);
-				sbuf_printf(sb, "%c", '\0');
-			}
+	PROC_LOCK(p);
+	if (p->p_args && (ps_argsopen || !p_cansee(td->td_proc, p))) {
+		sbuf_bcpy(sb, p->p_args->ar_args, p->p_args->ar_length);
+		PROC_UNLOCK(p);
+		return (0);
+	}
+	PROC_UNLOCK(p);
+	if (p != td->td_proc) {
+		sbuf_printf(sb, "%.*s", MAXCOMLEN, p->p_comm);
+	} else {
+		error = copyin((void*)PS_STRINGS, &pstr, sizeof(pstr));
+		if (error)
+			return (error);
+		for (i = 0; i < pstr.ps_nargvstr; i++) {
+			sbuf_copyin(sb, pstr.ps_argvstr[i], 0);
+			sbuf_printf(sb, "%c", '\0');
 		}
 	}
 

==== //depot/projects/smpng/sys/fs/pseudofs/pseudofs_vnops.c#13 (text+ko) ====

@@ -86,10 +86,13 @@
 	if (pid != NO_PID) {
 		if ((proc = pfind(pid)) == NULL)
 			PFS_RETURN (0);
-		/* XXX should lock td->td_proc? */
 		if (p_cansee(td->td_proc, proc) != 0 ||
 		    (pn->pn_vis != NULL && !(pn->pn_vis)(td, proc, pn)))
 			r = 0;
+		/*
+		 * XXX: We might should return with the proc locked to
+		 * avoid some races.
+		 */
 		PROC_UNLOCK(proc);
 	}
 	PFS_RETURN (r);

==== //depot/projects/smpng/sys/kern/kern_exec.c#33 (text+ko) ====

@@ -127,13 +127,15 @@
 {
 	struct proc *p = td->td_proc;
 	struct nameidata nd, *ndp;
-	struct ucred *newcred, *oldcred;
+	struct ucred *newcred = NULL, *oldcred;
 	register_t *stack_base;
 	int error, len, i;
 	struct image_params image_params, *imgp;
 	struct vattr attr;
 	int (*img_first)(struct image_params *);
-	struct pargs *pa;
+	struct pargs *oldargs, *newargs = NULL;
+	struct procsig *newprocsig = NULL;
+	struct vnode *tracevp = NULL, *textvp = NULL;
 
 	imgp = &image_params;
 
@@ -295,16 +297,27 @@
 		FILEDESC_UNLOCK(p->p_fd);
 
 	/*
+	 * Malloc things before we need locks.
+	 */
+	MALLOC(newprocsig, struct procsig *, sizeof(struct procsig),
+	    M_SUBPROC, M_WAITOK);
+	newcred = crget();
+	i = imgp->endargs - imgp->stringbase;
+	if (ps_arg_cache_limit >= i + sizeof(struct pargs))
+		newargs = pargs_alloc(i);
+
+	/* close files on exec */
+	fdcloseexec(td);
+
+	/*
 	 * For security and other reasons, signal handlers cannot
 	 * be shared after an exec. The new process gets a copy of the old
 	 * handlers. In execsigs(), the new process will have its signals
 	 * reset.
 	 */
+	PROC_LOCK(p);
+	mp_fixme("procsig needs a lock");
 	if (p->p_procsig->ps_refcnt > 1) {
-		struct procsig *newprocsig;
-
-		MALLOC(newprocsig, struct procsig *, sizeof(struct procsig),
-		       M_SUBPROC, M_WAITOK);
 		bcopy(p->p_procsig, newprocsig, sizeof(*newprocsig));
 		p->p_procsig->ps_refcnt--;
 		p->p_procsig = newprocsig;
@@ -318,9 +331,6 @@
 	/* Stop profiling */
 	stopprofclock(p);
 
-	/* close files on exec */
-	fdcloseexec(td);
-
 	/* reset caught signals */
 	execsigs(p);
 
@@ -333,7 +343,6 @@
 	 * mark as execed, wakeup the process that vforked (if any) and tell
 	 * it that it now has its own resources back
 	 */
-	PROC_LOCK(p);
 	p->p_flag |= P_EXEC;
 	if (p->p_pptr && (p->p_flag & P_PPWAIT)) {
 		p->p_flag &= ~P_PPWAIT;
@@ -347,12 +356,10 @@
 	 * the process is being traced.
 	 */
 	oldcred = p->p_ucred;
-	newcred = NULL;
 	if ((((attr.va_mode & VSUID) && oldcred->cr_uid != attr.va_uid) ||
 	     ((attr.va_mode & VSGID) && oldcred->cr_gid != attr.va_gid)) &&
 	    (imgp->vp->v_mount->mnt_flag & MNT_NOSUID) == 0 &&
 	    (p->p_flag & P_TRACED) == 0) {
-		PROC_UNLOCK(p);
 		/*
 		 * Turn off syscall tracing for set-id programs, except for
 		 * root.  Record any set-id flags first to make sure that
@@ -360,66 +367,56 @@
 		 */
 		setsugid(p);
 		if (p->p_tracep && suser_cred(oldcred, PRISON_ROOT)) {
-			struct vnode *vtmp;
-
-			if ((vtmp = p->p_tracep) != NULL) {
-				p->p_tracep = NULL;
-				p->p_traceflag = 0;
-				vrele(vtmp);
-			}
+			p->p_traceflag = 0;
+			tracevp = p->p_tracep;
+			p->p_tracep = NULL;
 		}
 		/*
 		 * Set the new credentials.
 		 */
-		newcred = crdup(oldcred);
+		crcopy(newcred, oldcred);
 		if (attr.va_mode & VSUID)
 			change_euid(newcred, attr.va_uid);
 		if (attr.va_mode & VSGID)
 			change_egid(newcred, attr.va_gid);
 		setugidsafety(td);
+		/*
+		 * Implement correct POSIX saved-id behavior.
+		 */
+		change_svuid(newcred, newcred->cr_uid);
+		change_svgid(newcred, newcred->cr_gid);
+		p->p_ucred = newcred;
+		newcred = NULL;
 	} else {
 		if (oldcred->cr_uid == oldcred->cr_ruid &&
 		    oldcred->cr_gid == oldcred->cr_rgid)
 			p->p_flag &= ~P_SUGID;
-		PROC_UNLOCK(p);
-	}
-
-	/*
-	 * Implement correct POSIX saved-id behavior.
-	 *
-	 * XXX: It's not clear that the existing behavior is
-	 * POSIX-compliant.  A number of sources indicate that the saved
-	 * uid/gid should only be updated if the new ruid is not equal to
-	 * the old ruid, or the new euid is not equal to the old euid and
-	 * the new euid is not equal to the old ruid.  The FreeBSD code
-	 * always updates the saved uid/gid.  Also, this code uses the new
-	 * (replaced) euid and egid as the source, which may or may not be
-	 * the right ones to use.
-	 */
-	if (newcred == NULL) {
+		/*
+		 * Implement correct POSIX saved-id behavior.
+		 *
+		 * XXX: It's not clear that the existing behavior is
+		 * POSIX-compliant.  A number of sources indicate that the
+		 * saved uid/gid should only be updated if the new ruid is
+		 * not equal to the old ruid, or the new euid is not equal
+		 * to the old euid and the new euid is not equal to the old
+		 * ruid.  The FreeBSD code always updates the saved uid/gid.
+		 * Also, this code uses the new (replaced) euid and egid as
+		 * the source, which may or may not be the right ones to use.
+		 */
 		if (oldcred->cr_svuid != oldcred->cr_uid ||
 		    oldcred->cr_svgid != oldcred->cr_gid) {
-			newcred = crdup(oldcred);
+			crcopy(newcred, oldcred);
 			change_svuid(newcred, newcred->cr_uid);
 			change_svgid(newcred, newcred->cr_gid);
+			p->p_ucred = newcred;
+			newcred = NULL;
 		}
-	} else {
-		change_svuid(newcred, newcred->cr_uid);
-		change_svgid(newcred, newcred->cr_gid);
-	}
-
-	if (newcred != NULL) {
-		PROC_LOCK(p);
-		p->p_ucred = newcred;
-		PROC_UNLOCK(p);
-		crfree(oldcred);
 	}
 
 	/*
 	 * Store the vp for use in procfs
 	 */
-	if (p->p_textvp)		/* release old reference */
-		vrele(p->p_textvp);
+	textvp = p->p_textvp;
 	VREF(ndp->ni_vp);
 	p->p_textvp = ndp->ni_vp;
 
@@ -427,7 +424,6 @@
 	 * Notify others that we exec'd, and clear the P_INEXEC flag
 	 * as we're now a bona fide freshly-execed process.
 	 */
-	PROC_LOCK(p);
 	KNOTE(&p->p_klist, NOTE_EXEC);
 	p->p_flag &= ~P_INEXEC;
 
@@ -444,24 +440,39 @@
 	p->p_acflag &= ~AFORK;
 
 	/* Free any previous argument cache */
-	pa = p->p_args;
+	oldargs = p->p_args;
 	p->p_args = NULL;
-	PROC_UNLOCK(p);
-	pargs_drop(pa);
 
 	/* Set values passed into the program in registers. */
 	setregs(td, imgp->entry_addr, (u_long)(uintptr_t)stack_base,
 	    imgp->ps_strings);
 
 	/* Cache arguments if they fit inside our allowance */
-	i = imgp->endargs - imgp->stringbase;
 	if (ps_arg_cache_limit >= i + sizeof(struct pargs)) {
-		pa = pargs_alloc(i);
-		bcopy(imgp->stringbase, pa->ar_args, i);
-		PROC_LOCK(p);
-		p->p_args = pa;
-		PROC_UNLOCK(p);
+		bcopy(imgp->stringbase, newargs->ar_args, i);
+		p->p_args = newargs;
+		newargs = NULL;
 	}
+	PROC_UNLOCK(p);
+
+	/*
+	 * Free any resources malloc'd earlier that we didn't use.
+	 */
+	if (newprocsig != NULL)
+		FREE(newprocsig, M_SUBPROC);
+	if (newcred == NULL)
+		crfree(oldcred);
+	else
+		crfree(newcred);
+	KASSERT(newargs == NULL, ("leaking p_args"));
+	/*
+	 * Handle deferred decrement of ref counts.
+	 */
+	if (textvp != NULL)
+		vrele(textvp);
+	if (tracevp != NULL)
+		vrele(tracevp);
+	pargs_drop(oldargs);
 
 exec_fail_dealloc:
 

==== //depot/projects/smpng/sys/kern/kern_exit.c#31 (text+ko) ====

@@ -127,6 +127,9 @@
 	struct exitlist *ep;
 	struct vnode *ttyvp;
 	struct tty *tp;
+#ifdef KTRACE
+	struct vnode *tracevp;
+#endif
 
 	GIANT_REQUIRED;
 
@@ -293,11 +296,13 @@
 	/*
 	 * release trace file
 	 */
+	PROC_LOCK(p);
 	p->p_traceflag = 0;	/* don't trace the vrele() */
-	if ((vtmp = p->p_tracep) != NULL) {

>>> TRUNCATED FOR MAIL (1000 lines) <<<

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe p4-projects" in the body of the message




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