frevoke - More fixes to fdfree()

* Don't replace p->p_fd until after all closef()'s have been called.  Fixed
  panic in kqueue's close function.

* Move softrefs check to after the closef point, but before we actually
  start kfree()ing the structures.  Lock the closef's normally since
  p->p_fd is still valid.

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index e7bcc96..7e45f3a 100644 (file)
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1810,18 +1810,13 @@ fdfree(struct proc *p, struct filedesc *repl)
        struct flock lf;
 
        /*
-        * Interlock against an allproc scan operations (typically frevoke).
-        */
-       spin_lock_wr(&p->p_spin);
-       fdp = p->p_fd;
-       p->p_fd = repl;
-       spin_unlock_wr(&p->p_spin);
-
-       /*
         * Certain daemons might not have file descriptors.
         */
-       if (fdp == NULL)
+       fdp = p->p_fd;
+       if (fdp == NULL) {
+               p->p_fd = repl;
                return;
+       }
 
        /*
         * Severe messing around to follow.
@@ -1900,11 +1895,41 @@ fdfree(struct proc *p, struct filedesc *repl)
        }
        if (--fdp->fd_refcnt > 0) {
                spin_unlock_wr(&fdp->fd_spin);
+               spin_lock_wr(&p->p_spin);
+               p->p_fd = repl;
+               spin_unlock_wr(&p->p_spin);
                return;
        }
+
+       /*
+        * Even though we are the last reference to the structure allproc
+        * scans may still reference the structure.  Maintain proper
+        * locks until we can replace p->p_fd.
+        *
+        * Also note that kqueue's closef still needs to reference the
+        * fdp via p->p_fd, so we have to close the descriptors before
+        * we replace p->p_fd.
+        */
+       for (i = 0; i <= fdp->fd_lastfile; ++i) {
+               if (fdp->fd_files[i].fp) {
+                       fp = funsetfd_locked(fdp, i);
+                       if (fp) {
+                               spin_unlock_wr(&fdp->fd_spin);
+                               closef(fp, p);
+                               spin_lock_wr(&fdp->fd_spin);
+                       }
+               }
+       }
        spin_unlock_wr(&fdp->fd_spin);
 
        /*
+        * Interlock against an allproc scan operations (typically frevoke).
+        */
+       spin_lock_wr(&p->p_spin);
+       p->p_fd = repl;
+       spin_unlock_wr(&p->p_spin);
+
+       /*
         * Wait for any softrefs to go away.  This race rarely occurs so
         * we can use a non-critical-path style poll/sleep loop.  The
         * race only occurs against allproc scans.
@@ -1918,14 +1943,6 @@ fdfree(struct proc *p, struct filedesc *repl)
                        tsleep(&fdp->fd_softrefs, 0, "fdsoft", 1);
        }
 
-       /*
-        * we are the last reference to the structure, we can
-        * safely assume it will not change out from under us.
-        */
-       for (i = 0; i <= fdp->fd_lastfile; ++i) {
-               if (fdp->fd_files[i].fp)
-                       closef(fdp->fd_files[i].fp, p);
-       }
        if (fdp->fd_files != fdp->fd_builtin_files)
                kfree(fdp->fd_files, M_FILEDESC);
        if (fdp->fd_cdir) {