svn commit: r368486 - head/sys/kern

[Mark Johnston]

Author: markj
Date: Wed Dec  9 14:05:08 2020
New Revision: 368486
URL: https://svnweb.freebsd.org/changeset/base/368486

Log:
  Plug a race between fd table teardown and several loops
  
  To export information from fd tables we have several loops which do
  this:
  
  FILDESC_SLOCK(fdp);
  for (i = 0; fdp->fd_refcount > 0 && i <= lastfile; i++)
  	<export info for fd i>;
  FILDESC_SUNLOCK(fdp);
  
  Before r367777, fdescfree() acquired the fd table exclusive lock between
  decrementing fdp->fd_refcount and freeing table entries.  This
  serialized with the loop above, so the file at descriptor i would remain
  valid until the lock is dropped.  Now there is no serialization, so the
  loops may race with teardown of file descriptor tables.
  
  Acquire the exclusive fdtable lock after releasing the final table
  reference to provide a barrier synchronizing with these loops.
  
  Reported by:	pho
  Reviewed by:	kib (previous version), mjg
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D27513

Modified:
  head/sys/kern/kern_descrip.c

Modified: head/sys/kern/kern_descrip.c
==============================================================================
--- head/sys/kern/kern_descrip.c	Wed Dec  9 14:04:54 2020	(r368485)
+++ head/sys/kern/kern_descrip.c	Wed Dec  9 14:05:08 2020	(r368486)
@@ -2463,6 +2463,13 @@ fdescfree_fds(struct thread *td, struct filedesc *fdp,
 	struct file *fp;
 	int i, lastfile;
 
+	KASSERT(refcount_load(&fdp->fd_refcnt) == 0,
+	    ("%s: fd table %p carries references", __func__, fdp));
+
+	/* Serialize with threads iterating over the table. */
+	FILEDESC_XLOCK(fdp);
+	FILEDESC_XUNLOCK(fdp);
+
 	lastfile = fdlastfile_single(fdp);
 	for (i = 0; i <= lastfile; i++) {
 		fde = &fdp->fd_ofiles[i];
@@ -2536,6 +2543,11 @@ pdescfree(struct thread *td)
 void
 fdescfree_remapped(struct filedesc *fdp)
 {
+#ifdef INVARIANTS
+	/* fdescfree_fds() asserts that fd_refcnt == 0. */
+	if (!refcount_release(&fdp->fd_refcnt))
+		panic("%s: fd table %p has extra references", __func__, fdp);
+#endif
 	fdescfree_fds(curthread, fdp, 0);
 }