From 464e801e77f8e4207ef040861cd7561c47e4f913 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Fri, 3 Jul 2009 10:48:26 -0700 Subject: [PATCH 1/1] 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. --- sys/kern/kern_descrip.c | 51 +++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c index e7bcc96e59..7e45f3a622 100644 --- a/sys/kern/kern_descrip.c +++ b/sys/kern/kern_descrip.c @@ -1809,19 +1809,14 @@ fdfree(struct proc *p, struct filedesc *repl) struct vnode *vp; 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,10 +1895,40 @@ 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 @@ -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) { -- 2.41.0