On 25/04/18(Wed) 17:07, Visa Hankala wrote:
> On Wed, Apr 25, 2018 at 12:12:29PM +0200, Martin Pieuchot wrote:
> > The goal is to avoid races between fd_getfile() and FREF(). So we want
> > a properly refcounted 'struct file *' as soon as possible.
>
> Boot hangs with this patch. The last line on the console is
> "setting tty flags".
>
> Two issues spotted so far:
>
> > @@ -201,9 +202,10 @@ fd_getfile_mode(struct filedesc *fdp, in
> > KASSERT(mode != 0);
> >
> > fp = fd_getfile(fdp, fd);
> > -
> > - if (fp == NULL || (fp->f_flag & mode) == 0)
> > + if (fp == NULL || (fp->f_flag & mode) == 0) {
> > + FRELE(fp, curproc);
> > return (NULL);
> > + }
> >
> > return (fp);
> > }
>
> * The FRELE() above can dereference a NULL pointer.
>
> * sys_close() lacks an FRELE().
Thanks, updated diff below.
Index: kern/kern_descrip.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.153
diff -u -p -r1.153 kern_descrip.c
--- kern/kern_descrip.c 26 Apr 2018 06:28:43 -0000 1.153
+++ kern/kern_descrip.c 26 Apr 2018 06:33:10 -0000
@@ -212,6 +212,7 @@ fd_getfile(struct filedesc *fdp, int fd)
if (!FILE_IS_USABLE(fp))
return (NULL);
+ FREF(fp);
return (fp);
}
@@ -223,9 +224,13 @@ fd_getfile_mode(struct filedesc *fdp, in
KASSERT(mode != 0);
fp = fd_getfile(fdp, fd);
+ if (fp == NULL)
+ return (NULL);
- if (fp == NULL || (fp->f_flag & mode) == 0)
+ if ((fp->f_flag & mode) == 0) {
+ FRELE(fp, curproc);
return (NULL);
+ }
return (fp);
}
@@ -252,7 +257,6 @@ sys_dup(struct proc *p, void *v, registe
restart:
if ((fp = fd_getfile(fdp, old)) == NULL)
return (EBADF);
- FREF(fp);
fdplock(fdp);
if ((error = fdalloc(p, 0, &new)) != 0) {
FRELE(fp, p);
@@ -312,7 +316,6 @@ dodup3(struct proc *p, int old, int new,
restart:
if ((fp = fd_getfile(fdp, old)) == NULL)
return (EBADF);
- FREF(fp);
if ((u_int)new >= p->p_rlimit[RLIMIT_NOFILE].rlim_cur ||
(u_int)new >= maxfiles) {
FRELE(fp, p);
@@ -379,7 +382,6 @@ sys_fcntl(struct proc *p, void *v, regis
restart:
if ((fp = fd_getfile(fdp, fd)) == NULL)
return (EBADF);
- FREF(fp);
switch (SCARG(uap, cmd)) {
case F_DUPFD:
@@ -553,8 +555,6 @@ restart:
}
fp2 = fd_getfile(fdp, fd);
- if (fp2 != NULL)
- FREF(fp2);
if (fp != fp2) {
/*
* We have lost the race with close() or dup2();
@@ -701,9 +701,13 @@ sys_close(struct proc *p, void *v, regis
} */ *uap = v;
int fd = SCARG(uap, fd), error;
struct filedesc *fdp = p->p_fd;
+ struct file *fp;
- if (fd_getfile(fdp, fd) == NULL)
+ fp = fd_getfile(fdp, fd);
+ if (fp == NULL)
return (EBADF);
+ FRELE(fp, p);
+
fdplock(fdp);
error = fdrelease(p, fd);
fdpunlock(fdp);
@@ -729,7 +733,6 @@ sys_fstat(struct proc *p, void *v, regis
if ((fp = fd_getfile(fdp, fd)) == NULL)
return (EBADF);
- FREF(fp);
error = (*fp->f_ops->fo_stat)(fp, &ub, p);
FRELE(fp, p);
if (error == 0) {
@@ -767,7 +770,6 @@ sys_fpathconf(struct proc *p, void *v, r
if ((fp = fd_getfile(fdp, fd)) == NULL)
return (EBADF);
- FREF(fp);
switch (fp->f_type) {
case DTYPE_PIPE:
case DTYPE_SOCKET:
@@ -1220,7 +1222,6 @@ sys_flock(struct proc *p, void *v, regis
if ((fp = fd_getfile(fdp, fd)) == NULL)
return (EBADF);
- FREF(fp);
if (fp->f_type != DTYPE_VNODE) {
error = EOPNOTSUPP;
goto out;
@@ -1309,7 +1310,6 @@ dupfdopen(struct proc *p, int indx, int
*/
if ((wfp = fd_getfile(fdp, dupfd)) == NULL)
return (EBADF);
- FREF(wfp);
/*
* Check that the mode the file is being opened for is a
Index: kern/kern_event.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.87
diff -u -p -r1.87 kern_event.c
--- kern/kern_event.c 10 Apr 2018 09:17:45 -0000 1.87
+++ kern/kern_event.c 26 Apr 2018 06:33:10 -0000
@@ -481,7 +481,6 @@ sys_kevent(struct proc *p, void *v, regi
if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
return (EBADF);
- FREF(fp);
if (fp->f_type != DTYPE_KQUEUE) {
error = EBADF;
@@ -583,7 +582,6 @@ kqueue_register(struct kqueue *kq, struc
return (EBADF);
if ((fp = fd_getfile(fdp, kev->ident)) == NULL)
return (EBADF);
- FREF(fp);
if (kev->ident < fdp->fd_knlistsize) {
SLIST_FOREACH(kn, &fdp->fd_knlist[kev->ident], kn_link)
{
Index: kern/kern_exec.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_exec.c,v
retrieving revision 1.193
diff -u -p -r1.193 kern_exec.c
--- kern/kern_exec.c 2 Jan 2018 06:38:45 -0000 1.193
+++ kern/kern_exec.c 26 Apr 2018 06:33:11 -0000
@@ -608,6 +608,8 @@ sys_execve(struct proc *p, void *v, regi
fp->f_ops = &vnops;
fp->f_data = (caddr_t)vp;
FILE_SET_MATURE(fp, p);
+ } else {
+ FRELE(fp, p);
}
}
fdpunlock(p->p_fd);
Index: kern/sys_generic.c
===================================================================
RCS file: /cvs/src/sys/kern/sys_generic.c,v
retrieving revision 1.117
diff -u -p -r1.117 sys_generic.c
--- kern/sys_generic.c 9 Apr 2018 09:53:13 -0000 1.117
+++ kern/sys_generic.c 26 Apr 2018 06:33:11 -0000
@@ -94,8 +94,6 @@ sys_read(struct proc *p, void *v, regist
iov.iov_base = SCARG(uap, buf);
iov.iov_len = SCARG(uap, nbyte);
- FREF(fp);
-
/* dofilereadv() will FRELE the descriptor for us */
return (dofilereadv(p, fd, fp, &iov, 1, 0, &fp->f_offset, retval));
}
@@ -117,7 +115,6 @@ sys_readv(struct proc *p, void *v, regis
if ((fp = fd_getfile_mode(fdp, fd, FREAD)) == NULL)
return (EBADF);
- FREF(fp);
/* dofilereadv() will FRELE the descriptor for us */
return (dofilereadv(p, fd, fp, SCARG(uap, iovp), SCARG(uap, iovcnt), 1,
@@ -246,8 +243,6 @@ sys_write(struct proc *p, void *v, regis
iov.iov_base = (void *)SCARG(uap, buf);
iov.iov_len = SCARG(uap, nbyte);
- FREF(fp);
-
/* dofilewritev() will FRELE the descriptor for us */
return (dofilewritev(p, fd, fp, &iov, 1, 0, &fp->f_offset, retval));
}
@@ -269,7 +264,6 @@ sys_writev(struct proc *p, void *v, regi
if ((fp = fd_getfile_mode(fdp, fd, FWRITE)) == NULL)
return (EBADF);
- FREF(fp);
/* dofilewritev() will FRELE the descriptor for us */
return (dofilewritev(p, fd, fp, SCARG(uap, iovp), SCARG(uap, iovcnt), 1,
@@ -403,7 +397,6 @@ sys_ioctl(struct proc *p, void *v, regis
fdp = p->p_fd;
if ((fp = fd_getfile_mode(fdp, SCARG(uap, fd), FREAD|FWRITE)) == NULL)
return (EBADF);
- FREF(fp);
if (fp->f_type == DTYPE_SOCKET) {
struct socket *so = fp->f_data;
@@ -745,7 +738,6 @@ selscan(struct proc *p, fd_set *ibits, f
bits &= ~(1 << j);
if ((fp = fd_getfile(fdp, fd)) == NULL)
return (EBADF);
- FREF(fp);
if ((*fp->f_ops->fo_poll)(fp, flag[msk], p)) {
FD_SET(fd, pobits);
n++;
@@ -842,7 +834,6 @@ pollscan(struct proc *p, struct pollfd *
n++;
continue;
}
- FREF(fp);
pl->revents = (*fp->f_ops->fo_poll)(fp, pl->events, p);
FRELE(fp, p);
if (pl->revents != 0)
Index: kern/uipc_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.168
diff -u -p -r1.168 uipc_syscalls.c
--- kern/uipc_syscalls.c 28 Mar 2018 09:54:00 -0000 1.168
+++ kern/uipc_syscalls.c 26 Apr 2018 06:33:11 -0000
@@ -1160,7 +1160,6 @@ getsock(struct proc *p, int fdes, struct
if ((fp = fd_getfile(p->p_fd, fdes)) == NULL)
return (EBADF);
- FREF(fp);
if (fp->f_type != DTYPE_SOCKET) {
FRELE(fp, p);
return (ENOTSOCK);
Index: kern/uipc_usrreq.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.124
diff -u -p -r1.124 uipc_usrreq.c
--- kern/uipc_usrreq.c 18 Apr 2018 09:56:57 -0000 1.124
+++ kern/uipc_usrreq.c 26 Apr 2018 06:33:11 -0000
@@ -838,7 +838,6 @@ morespace:
error = EBADF;
goto fail;
}
- FREF(fp);
if (fp->f_count == LONG_MAX-2) {
error = EDEADLK;
goto fail;
Index: kern/vfs_lookup.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_lookup.c,v
retrieving revision 1.66
diff -u -p -r1.66 vfs_lookup.c
--- kern/vfs_lookup.c 9 Apr 2018 09:59:32 -0000 1.66
+++ kern/vfs_lookup.c 26 Apr 2018 06:33:11 -0000
@@ -190,7 +190,6 @@ fail:
pool_put(&namei_pool, cnp->cn_pnbuf);
return (EBADF);
}
- FREF(fp);
dp = (struct vnode *)fp->f_data;
if (fp->f_type != DTYPE_VNODE || dp->v_type != VDIR) {
FRELE(fp, p);
Index: kern/vfs_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.279
diff -u -p -r1.279 vfs_syscalls.c
--- kern/vfs_syscalls.c 3 Apr 2018 09:10:02 -0000 1.279
+++ kern/vfs_syscalls.c 26 Apr 2018 06:33:11 -0000
@@ -745,7 +745,6 @@ sys_fchdir(struct proc *p, void *v, regi
if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
return (EBADF);
- FREF(fp);
vp = fp->f_data;
if (fp->f_type != DTYPE_VNODE || vp->v_type != VDIR) {
FRELE(fp, p);
@@ -1616,7 +1615,6 @@ sys_lseek(struct proc *p, void *v, regis
if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL)
return (EBADF);
- FREF(fp);
vp = fp->f_data;
if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
error = ESPIPE;
@@ -2900,7 +2898,6 @@ getvnode(struct proc *p, int fd, struct
if ((fp = fd_getfile(p->p_fd, fd)) == NULL)
return (EBADF);
- FREF(fp);
if (fp->f_type != DTYPE_VNODE) {
FRELE(fp, p);
@@ -2943,7 +2940,6 @@ sys_pread(struct proc *p, void *v, regis
if ((fp = fd_getfile_mode(fdp, fd, FREAD)) == NULL)
return (EBADF);
- FREF(fp);
vp = fp->f_data;
if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO ||
@@ -2983,7 +2979,6 @@ sys_preadv(struct proc *p, void *v, regi
if ((fp = fd_getfile_mode(fdp, fd, FREAD)) == NULL)
return (EBADF);
- FREF(fp);
vp = fp->f_data;
if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO ||
@@ -3028,7 +3023,6 @@ sys_pwrite(struct proc *p, void *v, regi
if ((fp = fd_getfile_mode(fdp, fd, FWRITE)) == NULL)
return (EBADF);
- FREF(fp);
vp = fp->f_data;
if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO ||
@@ -3068,7 +3062,6 @@ sys_pwritev(struct proc *p, void *v, reg
if ((fp = fd_getfile_mode(fdp, fd, FWRITE)) == NULL)
return (EBADF);
- FREF(fp);
vp = fp->f_data;
if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO ||
Index: miscfs/fuse/fuse_vfsops.c
===================================================================
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vfsops.c,v
retrieving revision 1.33
diff -u -p -r1.33 fuse_vfsops.c
--- miscfs/fuse/fuse_vfsops.c 28 Mar 2018 16:34:28 -0000 1.33
+++ miscfs/fuse/fuse_vfsops.c 26 Apr 2018 06:33:11 -0000
@@ -84,7 +84,6 @@ fusefs_mount(struct mount *mp, const cha
if ((fp = fd_getfile(p->p_fd, args->fd)) == NULL)
return (EBADF);
- FREF(fp);
if (fp->f_type != DTYPE_VNODE) {
error = EINVAL;
Index: uvm/uvm_mmap.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_mmap.c,v
retrieving revision 1.149
diff -u -p -r1.149 uvm_mmap.c
--- uvm/uvm_mmap.c 12 Apr 2018 17:13:44 -0000 1.149
+++ uvm/uvm_mmap.c 26 Apr 2018 06:33:11 -0000
@@ -434,8 +434,6 @@ sys_mmap(struct proc *p, void *v, regist
return (EBADF);
}
- FREF(fp);
-
if (fp->f_type != DTYPE_VNODE) {
error = ENODEV; /* only mmap vnodes! */
goto out;
