Skip to content

Commit

Permalink
kern: dup: do not assume oldfde is valid
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kevans91 committed Nov 23, 2020
1 parent 81270f8 commit 3d4ae1b
Showing 1 changed file with 12 additions and 5 deletions.
17 changes: 12 additions & 5 deletions sys/kern/kern_descrip.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;

/*
Expand All @@ -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;
Expand All @@ -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;
}
}
Expand All @@ -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;

Expand Down

0 comments on commit 3d4ae1b

Please sign in to comment.