- Conditionalize Giant around VFS operations for ALQ, ktrace, and

  generating a coredump as the result of a signal.
- Fix a bug where we could leak a Giant lock if vn_start_write() failed
  in coredump().

Reported by:	jmg (2)

3 files changed, 35 insertions, 32 deletions

diff --git a/sys/kern/kern_alq.c b/sys/kern/kern_alq.c
index fbedf08..b6b24de 100644
--- a/sys/kern/kern_alq.c
+++ b/sys/kern/kern_alq.c
@@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/kthread.h>
 #include <sys/lock.h>
 #include <sys/mac.h>
+#include <sys/mount.h>
 #include <sys/mutex.h>
 #include <sys/namei.h>
 #include <sys/proc.h>
@@ -172,8 +173,6 @@ ald_daemon(void)
 	int needwakeup;
 	struct alq *alq;
 
-	mtx_lock(&Giant);
-
 	ald_thread = FIRST_THREAD_IN_PROC(ald_proc);
 
 	EVENTHANDLER_REGISTER(shutdown_pre_sync, ald_shutdown, NULL,
@@ -250,6 +249,7 @@ alq_doio(struct alq *alq)
 	struct ale *alstart;
 	int totlen;
 	int iov;
+	int vfslocked;
 
 	vp = alq->aq_vp;
 	td = curthread;
@@ -291,6 +291,7 @@ alq_doio(struct alq *alq)
 	/*
 	 * Do all of the junk required to write now.
 	 */
+	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	vn_start_write(vp, &mp, V_WAIT);
 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);
 	VOP_LEASE(vp, td, alq->aq_cred, LEASE_WRITE);
@@ -303,6 +304,7 @@ alq_doio(struct alq *alq)
 		VOP_WRITE(vp, &auio, IO_UNIT | IO_APPEND, alq->aq_cred);
 	VOP_UNLOCK(vp, 0, td);
 	vn_finished_write(mp);
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	ALQ_LOCK(alq);
 	alq->aq_flags &= ~AQ_FLUSHING;
@@ -345,21 +347,23 @@ alq_open(struct alq **alqp, const char *file, struct ucred *cred, int cmode,
 	char *bufp;
 	int flags;
 	int error;
-	int i;
+	int i, vfslocked;
 
 	*alqp = NULL;
 	td = curthread;
 
-	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, file, td);
+	NDINIT(&nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_SYSSPACE, file, td);
 	flags = FWRITE | O_NOFOLLOW | O_CREAT;
 
 	error = vn_open_cred(&nd, &flags, cmode, cred, -1);
 	if (error)
 		return (error);
 
+	vfslocked = NDHASGIANT(&nd);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	/* We just unlock so we hold a reference */
 	VOP_UNLOCK(nd.ni_vp, 0, td);
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	alq = malloc(sizeof(*alq), M_ALD, M_WAITOK|M_ZERO);
 	alq->aq_entbuf = malloc(count * size, M_ALD, M_WAITOK|M_ZERO);
diff --git a/sys/kern/kern_ktrace.c b/sys/kern/kern_ktrace.c
index 7f42f67..cd846d6 100644
--- a/sys/kern/kern_ktrace.c
+++ b/sys/kern/kern_ktrace.c
@@ -598,7 +598,7 @@ ktrace(td, uap)
 	int ops = KTROP(uap->ops);
 	int descend = uap->ops & KTRFLAG_DESCEND;
 	int nfound, ret = 0;
-	int flags, error = 0;
+	int flags, error = 0, vfslocked;
 	struct nameidata nd;
 	struct ucred *cred;
 
@@ -613,25 +613,25 @@ ktrace(td, uap)
 		/*
 		 * an operation which requires a file argument.
 		 */
-		NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_USERSPACE, uap->fname, td);
+		NDINIT(&nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_USERSPACE,
+		    uap->fname, td);
 		flags = FREAD | FWRITE | O_NOFOLLOW;
-		mtx_lock(&Giant);
 		error = vn_open(&nd, &flags, 0, -1);
 		if (error) {
-			mtx_unlock(&Giant);
 			ktrace_exit(td);
 			return (error);
 		}
+		vfslocked = NDHASGIANT(&nd);
 		NDFREE(&nd, NDF_ONLY_PNBUF);
 		vp = nd.ni_vp;
 		VOP_UNLOCK(vp, 0, td);
 		if (vp->v_type != VREG) {
 			(void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
-			mtx_unlock(&Giant);
+			VFS_UNLOCK_GIANT(vfslocked);
 			ktrace_exit(td);
 			return (EACCES);
 		}
-		mtx_unlock(&Giant);
+		VFS_UNLOCK_GIANT(vfslocked);
 	}
 	/*
 	 * Clear all uses of the tracefile.
@@ -649,10 +649,10 @@ ktrace(td, uap)
 					p->p_traceflag = 0;
 					mtx_unlock(&ktrace_mtx);
 					PROC_UNLOCK(p);
-					mtx_lock(&Giant);
+					vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 					(void) vn_close(vp, FREAD|FWRITE,
 						cred, td);
-					mtx_unlock(&Giant);
+					VFS_UNLOCK_GIANT(vfslocked);
 					crfree(cred);
 				} else {
 					PROC_UNLOCK(p);
@@ -732,9 +732,9 @@ ktrace(td, uap)
 		error = EPERM;
 done:
 	if (vp != NULL) {
-		mtx_lock(&Giant);
+		vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 		(void) vn_close(vp, FWRITE, td->td_ucred, td);
-		mtx_unlock(&Giant);
+		VFS_UNLOCK_GIANT(vfslocked);
 	}
 	ktrace_exit(td);
 	return (error);
@@ -888,7 +888,7 @@ ktr_writerequest(struct thread *td, struct ktr_request *req)
 	struct iovec aiov[3];
 	struct mount *mp;
 	int datalen, buflen, vrele_count;
-	int error;
+	int error, vfslocked;
 
 	/*
 	 * We hold the vnode and credential for use in I/O in case ktrace is
@@ -944,7 +944,7 @@ ktr_writerequest(struct thread *td, struct ktr_request *req)
 		auio.uio_iovcnt++;
 	}
 
-	mtx_lock(&Giant);
+	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	vn_start_write(vp, &mp, V_WAIT);
 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);
 	(void)VOP_LEASE(vp, td, cred, LEASE_WRITE);
@@ -956,7 +956,7 @@ ktr_writerequest(struct thread *td, struct ktr_request *req)
 	VOP_UNLOCK(vp, 0, td);
 	vn_finished_write(mp);
 	vrele(vp);
-	mtx_unlock(&Giant);
+	VFS_UNLOCK_GIANT(vfslocked);
 	if (!error)
 		return;
 	/*
@@ -1000,10 +1000,10 @@ ktr_writerequest(struct thread *td, struct ktr_request *req)
 	 * them but not yet committed them, as those are per-thread.  The
 	 * thread will have to clear it itself on system call return.
 	 */
-	mtx_lock(&Giant);
+	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	while (vrele_count-- > 0)
 		vrele(vp);
-	mtx_unlock(&Giant);
+	VFS_UNLOCK_GIANT(vfslocked);
 }
 
 /*
diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c
index 7e919c8..2d99ff1 100644
--- a/sys/kern/kern_sig.c
+++ b/sys/kern/kern_sig.c
@@ -3062,6 +3062,7 @@ coredump(struct thread *td)
 	struct mount *mp;
 	char *name;			/* name of corefile */
 	off_t limit;
+	int vfslocked;
 
 	PROC_LOCK_ASSERT(p, MA_OWNED);
 	MPASS((p->p_flag & P_HADTHREADS) == 0 || p->p_singlethread == td);
@@ -3085,21 +3086,17 @@ coredump(struct thread *td)
 	if (limit == 0)
 		return (EFBIG);
 
-	mtx_lock(&Giant);
 restart:
 	name = expand_name(p->p_comm, td->td_ucred->cr_uid, p->p_pid);
-	if (name == NULL) {
-		mtx_unlock(&Giant);
+	if (name == NULL)
 		return (EINVAL);
-	}
-	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, td); /* XXXKSE */
+	NDINIT(&nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_SYSSPACE, name, td);
 	flags = O_CREAT | FWRITE | O_NOFOLLOW;
 	error = vn_open(&nd, &flags, S_IRUSR | S_IWUSR, -1);
 	free(name, M_TEMP);
-	if (error) {
-		mtx_unlock(&Giant);		
+	if (error)
 		return (error);
-	}
+	vfslocked = NDHASGIANT(&nd);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	vp = nd.ni_vp;
 
@@ -3108,7 +3105,7 @@ restart:
 	    VOP_GETATTR(vp, &vattr, cred, td) || vattr.va_nlink != 1) {
 		VOP_UNLOCK(vp, 0, td);
 		error = EFAULT;
-		goto out;
+		goto close;
 	}
 
 	VOP_UNLOCK(vp, 0, td);
@@ -3123,9 +3120,10 @@ restart:
 		if (locked)
 			VOP_ADVLOCK(vp, (caddr_t)p, F_UNLCK, &lf, F_FLOCK);
 		if ((error = vn_close(vp, FWRITE, cred, td)) != 0)
-			return (error);
+			goto out;
 		if ((error = vn_start_write(NULL, &mp, V_XSLEEP | PCATCH)) != 0)
-			return (error);
+			goto out;
+		VFS_UNLOCK_GIANT(vfslocked);
 		goto restart;
 	}
 
@@ -3150,11 +3148,12 @@ restart:
 		VOP_ADVLOCK(vp, (caddr_t)p, F_UNLCK, &lf, F_FLOCK);
 	}
 	vn_finished_write(mp);
-out:
+close:
 	error1 = vn_close(vp, FWRITE, cred, td);
-	mtx_unlock(&Giant);
 	if (error == 0)
 		error = error1;
+out:
+	VFS_UNLOCK_GIANT(vfslocked);
 	return (error);
 }