kern: dup: do not assume oldfde is valid

oldfde may be invalidated if the table has grown due to the operation that
we're performing, either via fdalloc() or a direct fdgrowtable_exp().

This was technically OK before rS367927 because the old table remained valid
until the filedesc became unused, but now it may be freed immediately if
it's an unshared table in a single-threaded process, so it is no longer a
good assumption to make.

This fixes dup/dup2 invocations that grow the file table; in the initial
report, it manifested as a kernel panic in devel/gmake's configure script.

Reported by:	Guy Yur <guyyur gmail com>
Reviewed by:	rew
Differential Revision:	https://reviews.freebsd.org/D27319
This commit is contained in:
Kyle Evans 2020-11-23 00:33:06 +00:00
parent 7511a63825
commit f96078b8fe

@ -870,7 +870,7 @@ kern_dup(struct thread *td, u_int mode, int flags, int old, int new)
struct filedesc *fdp;
struct filedescent *oldfde, *newfde;
struct proc *p;
struct file *delfp;
struct file *delfp, *oldfp;
u_long *oioctls, *nioctls;
int error, maxfd;
@ -910,7 +910,8 @@ kern_dup(struct thread *td, u_int mode, int flags, int old, int new)
}
oldfde = &fdp->fd_ofiles[old];
if (!fhold(oldfde->fde_file))
oldfp = oldfde->fde_file;
if (!fhold(oldfp))
goto unlock;
/*
@ -922,14 +923,14 @@ kern_dup(struct thread *td, u_int mode, int flags, int old, int new)
case FDDUP_NORMAL:
case FDDUP_FCNTL:
if ((error = fdalloc(td, new, &new)) != 0) {
fdrop(oldfde->fde_file, td);
fdrop(oldfp, td);
goto unlock;
}
break;
case FDDUP_MUSTREPLACE:
/* Target file descriptor must exist. */
if (fget_locked(fdp, new) == NULL) {
fdrop(oldfde->fde_file, td);
fdrop(oldfp, td);
goto unlock;
}
break;
@ -948,7 +949,7 @@ kern_dup(struct thread *td, u_int mode, int flags, int old, int new)
error = racct_set_unlocked(p, RACCT_NOFILE, new + 1);
if (error != 0) {
error = EMFILE;
fdrop(oldfde->fde_file, td);
fdrop(oldfp, td);
goto unlock;
}
}
@ -964,6 +965,12 @@ kern_dup(struct thread *td, u_int mode, int flags, int old, int new)
KASSERT(old != new, ("new fd is same as old"));
/* Refetch oldfde because the table may have grown and old one freed. */
oldfde = &fdp->fd_ofiles[old];
KASSERT(oldfp == oldfde->fde_file,
("fdt_ofiles shift from growth observed at fd %d",
old));
newfde = &fdp->fd_ofiles[new];
delfp = newfde->fde_file;