falloc allocates a file structure and adds it to the file descriptor

table, acquiring the necessary locks as it works. It usually returns
two references to the new descriptor: one in the descriptor table
and one via a pointer argument.

As falloc releases the FILEDESC lock before returning, there is a
potential for a process to close the reference in the file descriptor
table before falloc's caller gets to use the file. I don't think this
can happen in practice at the moment, because Giant indirectly protects
closes.

To stop the file being completly closed in this situation, this change
makes falloc set the refcount to two when both references are returned.
This makes life easier for several of falloc's callers, because the
first thing they previously did was grab an extra reference on the
file.

Reviewed by:	iedowse
Idea run past:	jhb

1 files changed, 2 insertions, 10 deletions

diff --git a/sys/kern/vfs_extattr.c b/sys/kern/vfs_extattr.c
index 5eeda6c..3783010 100644
--- a/sys/kern/vfs_extattr.c
+++ b/sys/kern/vfs_extattr.c
@@ -676,15 +676,11 @@ kern_open(struct thread *td, char *path, enum uio_seg pathseg, int flags,
 	error = falloc(td, &nfp, &indx);
 	if (error)
 		return (error);
+	/* An extra reference on `nfp' has been held for us by falloc(). */
 	fp = nfp;
 	cmode = ((mode &~ fdp->fd_cmask) & ALLPERMS) &~ S_ISTXT;
 	NDINIT(&nd, LOOKUP, FOLLOW, pathseg, path, td);
 	td->td_dupfd = -1;		/* XXX check for fdopen */
-	/*
-	 * Bump the ref count to prevent another process from closing
-	 * the descriptor while we are blocked in vn_open()
-	 */
-	fhold(fp);
 	error = vn_open(&nd, &flags, cmode, indx);
 	if (error) {
 
@@ -3673,13 +3669,9 @@ fhopen(td, uap)
 			vp->v_writecount--;
 		goto bad;
 	}
+	/* An extra reference on `nfp' has been held for us by falloc(). */
 	fp = nfp;	
 
-	/*
-	 * Hold an extra reference to avoid having fp ripped out 
-	 * from under us while we block in the lock op
-	 */
-	fhold(fp);
 	nfp->f_vnode = vp;
 	nfp->f_data = vp;
 	nfp->f_flag = fmode & FMASK;