Date: Mon, 18 Mar 2002 16:53:25 -0800 From: Alfred Perlstein <alfred@freebsd.org> To: Dag-Erling Smorgrav <des@ofug.org> Cc: Kris Kennaway <kris@obsecurity.org>, current@freebsd.org, fs@freebsd.org Subject: Re: panic: bwrite: buffer is not busy??? Message-ID: <20020319005325.GA455@elvis.mu.org> In-Reply-To: <xzphendlbtk.fsf@flood.ping.uio.no> References: <xzpadt6r1xr.fsf@flood.ping.uio.no> <20020318061739.GB894@elvis.mu.org> <xzpvgbupdqa.fsf@flood.ping.uio.no> <20020318071623.GD894@elvis.mu.org> <20020318010245.A48956@xor.obsecurity.org> <xzp4rjep7m5.fsf@flood.ping.uio.no> <20020318143204.GA688@elvis.mu.org> <xzplmcpn8un.fsf@flood.ping.uio.no> <20020318223631.GA23014@elvis.mu.org> <xzphendlbtk.fsf@flood.ping.uio.no>
next in thread | previous in thread | raw e-mail | index | archive | help
* Dag-Erling Smorgrav <des@ofug.org> [020318 15:03] wrote:
> Alfred Perlstein <alfred@freebsd.org> writes:
> > * Dag-Erling Smorgrav <des@ofug.org> [020318 08:23] wrote:
> > > Alfred Perlstein <alfred@freebsd.org> writes:
> > > > I think you're right, I'm pretty sure the fix is basically moving
> > > > the p->p_fd = NULL to after the closef will fix things [...]
> > > There will still be a race...
> > Are you sure? :)
>
> Almost, though I think the window will be much smaller than it is now.
> The only way I see of avoiding it alltogether is to protect p->p_fd
> and its mutex with allproc_lock (IOW, destroy the table as the last
> thing you do before zombifying the process)
Actually...
if checkdirs wins the race for proc lock it will do its magic and
fdfree will wait while it does that.
if fdfree wins, then checkdirs will see a NULL p_fd pointer.
> > Btw, is there a way to easily reproduce this bug?
>
> No, it's a race condition, which makes it hard to trigger on purpose.
>
> The problem with your patch is that *every* place in the kernel that
> calls FILEDESC_LOCK needs to first acquire the proc lock and check if
> p->p_fd is NULL.
No, only when a sideways access occurs, like in checkdirs().
I think this ought to fix it.
Index: kern/vfs_syscalls.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.231
diff -u -r1.231 vfs_syscalls.c
--- kern/vfs_syscalls.c 12 Mar 2002 04:00:10 -0000 1.231
+++ kern/vfs_syscalls.c 18 Mar 2002 23:18:34 -0000
@@ -446,29 +446,34 @@
{
struct filedesc *fdp;
struct proc *p;
+ int nrele;
if (olddp->v_usecount == 1)
return;
sx_slock(&allproc_lock);
LIST_FOREACH(p, &allproc, p_list) {
+ PROC_LOCK(p);
fdp = p->p_fd;
- if (fdp == NULL)
+ if (fdp == NULL) {
+ PROC_UNLOCK(p);
continue;
+ }
+ nrele = 0;
FILEDESC_LOCK(fdp);
if (fdp->fd_cdir == olddp) {
VREF(newdp);
fdp->fd_cdir = newdp;
- FILEDESC_UNLOCK(fdp);
- vrele(olddp);
- FILEDESC_LOCK(fdp);
+ nrele++;
}
if (fdp->fd_rdir == olddp) {
VREF(newdp);
fdp->fd_rdir = newdp;
- FILEDESC_UNLOCK(fdp);
+ nrele++;
+ }
+ FILEDESC_UNLOCK(fdp);
+ PROC_UNLOCK(p);
+ while (nrele--)
vrele(olddp);
- } else
- FILEDESC_UNLOCK(fdp);
}
sx_sunlock(&allproc_lock);
if (rootvnode == olddp) {
Index: kern/kern_descrip.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.128
diff -u -r1.128 kern_descrip.c
--- kern/kern_descrip.c 15 Mar 2002 08:03:46 -0000 1.128
+++ kern/kern_descrip.c 18 Mar 2002 19:04:24 -0000
@@ -1321,10 +1321,11 @@
fdfree(td)
struct thread *td;
{
- register struct filedesc *fdp = td->td_proc->p_fd;
+ register struct filedesc *fdp;
struct file **fpp;
register int i;
+ fdp = td->td_proc->p_fd;
/* Certain daemons might not have file descriptors. */
if (fdp == NULL)
return;
@@ -1344,6 +1345,11 @@
if (*fpp)
(void) closef(*fpp, td);
}
+
+ PROC_LOCK(td->td_proc);
+ td->td_proc->p_fd = NULL;
+ PROC_UNLOCK(td->td_proc);
+
if (fdp->fd_nfiles > NDFILE)
FREE(fdp->fd_ofiles, M_FILEDESC);
if (fdp->fd_cdir)
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-fs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020319005325.GA455>