svn commit: r367942 - head/sys/kern

[Kyle Evans]

Author: kevans
Date: Mon Nov 23 00:33:06 2020
New Revision: 367942
URL: https://svnweb.freebsd.org/changeset/base/367942

Log:
  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

Modified:
  head/sys/kern/kern_descrip.c

Modified: head/sys/kern/kern_descrip.c
==============================================================================
--- head/sys/kern/kern_descrip.c	Sun Nov 22 20:21:10 2020	(r367941)
+++ head/sys/kern/kern_descrip.c	Mon Nov 23 00:33:06 2020	(r367942)
@@ -870,7 +870,7 @@ kern_dup(struct thread *td, u_int mode, int flags, int
 	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
 	}
 
 	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
 	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
 				error = racct_set_unlocked(p, RACCT_NOFILE, new + 1);
 				if (error != 0) {
 					error = EMFILE;
-					fdrop(oldfde->fde_file, td);
+					fdrop(oldfp, td);
 					goto unlock;
 				}
 			}
@@ -963,6 +964,12 @@ kern_dup(struct thread *td, u_int mode, int flags, int
 	}
 
 	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;