Date: Thu, 20 Jul 2006 20:23:21 GMT From: John Baldwin <jhb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 102024 for review Message-ID: <200607202023.k6KKNLw7044233@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200607202023.k6KKNLw7044233>