Fix a file descriptor race I reintroduced when I split accept1() up into
kern_accept() and accept1(). If another thread closed the new file descriptor and the first thread later got an error trying to copyout the socket address, then it would attempt to close the wrong file object. To fix, add a struct file ** argument to kern_accept(). If it is non-NULL, then on success kern_accept() will store a pointer to the new file object there and not release any of the references. It is up to the calling code to drop the references appropriately (including a call to fdclose() in case of error to safely handle the aforementioned race). While I'm at it, go ahead and fix the svr4 streams code to not leak the accept fd if it gets an error trying to copyout the streams structures.
This commit is contained in:
parent
00f1856905
commit
f30e89ced3
@ -1637,10 +1637,12 @@ svr4_do_getmsg(td, uap, fp)
|
||||
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 @@ svr4_do_getmsg(td, uap, fp)
|
||||
* 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 @@ svr4_do_getmsg(td, uap, fp)
|
||||
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,28 +1919,37 @@ svr4_do_getmsg(td, uap, fp)
|
||||
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;
|
||||
|
||||
#ifdef DEBUG_SVR4
|
||||
|
@ -299,16 +299,17 @@ accept1(td, uap, compat)
|
||||
{
|
||||
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 @@ accept1(td, uap, compat)
|
||||
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 @@ noconnection:
|
||||
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);
|
||||
|
@ -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 @@ struct sendfile_args;
|
||||
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,
|
||||
|
Loading…
x
Reference in New Issue
Block a user