PERFORCE change 102024 for review

[John Baldwin]

http://perforce.freebsd.org/chv.cgi?CH=102024

Change 102024 by jhb at jhb_mutex on 2006/07/20 20:22:35

	Grrr.  Re-close a race I just re-opened in accept() with another
	thread closing the file descriptor out from under us.  Instead,
	return a struct file * to the caller (if they provide a place to
	return it) and let the caller release the reference when they are
	done with it, letting the caller safely call fdclose() in error
	cases.  Since I went to that trouble, fix the svr4 code to not leak
	the accept fd on error as well.  This is a bit hairy I'm afraid. :(

Affected files ...

.. //depot/projects/smpng/sys/compat/svr4/svr4_stream.c#37 edit
.. //depot/projects/smpng/sys/kern/uipc_syscalls.c#91 edit
.. //depot/projects/smpng/sys/sys/syscallsubr.h#48 edit

Differences ...

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

@@ -1637,10 +1637,12 @@
 	struct sockaddr *sa;
 	socklen_t sasize;
 	struct svr4_strm *st;
+	struct file *afp;
 	int fl;
 
 	retval = td->td_retval;
 	error = 0;
+	afp = NULL;
 
 	FILE_LOCK_ASSERT(fp, MA_NOTOWNED);
 
@@ -1778,7 +1780,7 @@
 		 * We are after a listen, so we try to accept...
 		 */
 
-		error = kern_accept(td, uap->fd, &sa, &sasize);
+		error = kern_accept(td, uap->fd, &sa, &sasize, &afp);
 		if (error) {
 			DPRINTF(("getmsg: accept failed %d\n", error));
 			return error;
@@ -1809,6 +1811,9 @@
 			break;
 
 		default:
+			fdclose(td->td_proc->p_fd, afp, st->s_afd, td);
+			fdrop(afp, td);
+			st->s_afd = -1;
 			free(sa, M_SONAME);
 			return ENOSYS;
 		}
@@ -1914,27 +1919,36 @@
 		return EINVAL;
 	}
 
-	/* XXX: We leak the accept fd if we get an error here. */
 	if (uap->ctl) {
 		if (ctl.len > sizeof(sc))
 			ctl.len = sizeof(sc);
 		if (ctl.len != -1)
-			if ((error = copyout(&sc, ctl.buf, ctl.len)) != 0)
-				return error;
+			error = copyout(&sc, ctl.buf, ctl.len);
 
-		if ((error = copyout(&ctl, uap->ctl, sizeof(ctl))) != 0)
-			return error;
+		if (error == 0)
+			error = copyout(&ctl, uap->ctl, sizeof(ctl));
 	}
 
 	if (uap->dat) {
-		if ((error = copyout(&dat, uap->dat, sizeof(dat))) != 0)
-			return error;
+		if (error == 0)
+			error = copyout(&dat, uap->dat, sizeof(dat));
 	}
 
 	if (uap->flags) { /* XXX: Need translation */
-		if ((error = copyout(&fl, uap->flags, sizeof(fl))) != 0)
-			return error;
+		if (error == 0)
+			error = copyout(&fl, uap->flags, sizeof(fl));
+	}
+
+	if (error) {
+		if (afp) {
+			fdclose(td->td_proc->p_fd, afp, st->s_afd, td);
+			fdrop(afp, td);
+			st->s_afd = -1;
+		}
+		return (error);
 	}
+	if (afp)
+		fdrop(afp, td);
 
 	*retval = 0;
 

==== //depot/projects/smpng/sys/kern/uipc_syscalls.c#91 (text+ko) ====

@@ -299,16 +299,17 @@
 {
 	struct sockaddr *name;
 	socklen_t namelen;
+	struct file *fp;
 	int error;
 
 	if (uap->name == NULL)
-		return (kern_accept(td, uap->s, NULL, NULL));
+		return (kern_accept(td, uap->s, NULL, NULL, NULL));
 
 	error = copyin(uap->anamelen, &namelen, sizeof (namelen));
 	if (error)
 		return (error);
 
-	error = kern_accept(td, uap->s, &name, &namelen);
+	error = kern_accept(td, uap->s, &name, &namelen, &fp);
 
 	/*
 	 * return a namelen of zero for older code which might
@@ -332,14 +333,15 @@
 		error = copyout(&namelen, uap->anamelen,
 		    sizeof(namelen));
 	if (error)
-		kern_close(td, td->td_retval[0]);
+		fdclose(td->td_proc->p_fd, fp, td->td_retval[0], td);
+	fdrop(fp, td);
 	free(name, M_SONAME);
 	return (error);
 }
 
 int
 kern_accept(struct thread *td, int s, struct sockaddr **name,
-    socklen_t *namelen)
+    socklen_t *namelen, struct file **fp)
 {
 	struct filedesc *fdp;
 	struct file *headfp, *nfp = NULL;
@@ -478,9 +480,17 @@
 		fdclose(fdp, nfp, fd, td);
 
 	/*
-	 * Release explicitly held references before returning.
+	 * Release explicitly held references before returning.  We return
+	 * a reference on nfp to the caller on success if they request it.
 	 */
 done:
+	if (fp != NULL) {
+		if (error == 0) {
+			*fp = nfp;
+			nfp = NULL;
+		} else
+			*fp = NULL;
+	}
 	if (nfp != NULL)
 		fdrop(nfp, td);
 	fdrop(headfp, td);

==== //depot/projects/smpng/sys/sys/syscallsubr.h#48 (text+ko) ====

@@ -34,6 +34,7 @@
 #include <sys/mac.h>
 #include <sys/mount.h>
 
+struct file;
 struct itimerval;
 struct image_args;
 struct mbuf;
@@ -51,7 +52,7 @@
 int	kern___getcwd(struct thread *td, u_char *buf, enum uio_seg bufseg,
 	    u_int buflen);
 int	kern_accept(struct thread *td, int s, struct sockaddr **name,
-	    socklen_t *namelen);
+	    socklen_t *namelen, struct file **fp);
 int	kern_access(struct thread *td, char *path, enum uio_seg pathseg,
 	    int flags);
 int	kern_adjtime(struct thread *td, struct timeval *delta,