PERFORCE change 102024 for review

[John Baldwin]

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

Change 102024 by jhb@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,
_______________________________________________
p4-projects@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/p4-projects
To unsubscribe, send any mail to "p4-projects-unsubscribe@xxxxxxxxxxx"