Fix a race condition that would cause the mountpoint to be cleaned

from under someone waiting for the fs server response in puffs_unmount()
if the descriptor was closed during the response wait (such as bug
leading to a crash in fs implementation unmount()).
This commit is contained in:
pooka 2006-12-10 22:33:31 +00:00
parent 4cb6e56c76
commit 84295069e0
3 changed files with 52 additions and 8 deletions

View File

@ -1,4 +1,4 @@
/* $NetBSD: puffs_sys.h,v 1.13 2006/12/05 23:41:24 pooka Exp $ */
/* $NetBSD: puffs_sys.h,v 1.14 2006/12/10 22:33:31 pooka Exp $ */
/*
* Copyright (c) 2005, 2006 Antti Kantee. All Rights Reserved.
@ -138,12 +138,13 @@ struct puffs_mount {
unsigned int pmp_nextreq;
uint8_t pmp_status;
uint8_t pmp_unmounting;
};
#define PUFFSTAT_BEFOREINIT 0
#define PUFFSTAT_MOUNTING 1
#define PUFFSTAT_RUNNING 2
#define PUFFSTAT_DYING 3
#define PUFFSTAT_DYING 3 /* Do you want your possessions identified? */
#define PNODE_INACTIVE 0x01
#define PNODE_LOCKED 0x02

View File

@ -1,4 +1,4 @@
/* $NetBSD: puffs_transport.c,v 1.1 2006/12/05 23:41:24 pooka Exp $ */
/* $NetBSD: puffs_transport.c,v 1.2 2006/12/10 22:33:31 pooka Exp $ */
/*
* Copyright (c) 2005, 2006 Antti Kantee. All Rights Reserved.
@ -159,10 +159,11 @@ puffs_fop_close(struct file *fp, struct lwp *l)
struct puffs_instance *pi;
struct puffs_mount *pmp;
struct mount *mp;
int gone;
int gone, rv;
DPRINTF(("puffs_fop_close: device closed, force filesystem unmount\n"));
restart:
simple_lock(&pi_lock);
pmp = FPTOPMP(fp);
/*
@ -175,6 +176,10 @@ puffs_fop_close(struct file *fp, struct lwp *l)
TAILQ_REMOVE(&puffs_ilist, pi, pi_entries);
simple_unlock(&pi_lock);
FREE(pi, M_PUFFS);
DPRINTF(("puffs_fop_close: pmp associated with fp %p was "
"embryonic\n", fp));
return 0;
}
@ -189,6 +194,10 @@ puffs_fop_close(struct file *fp, struct lwp *l)
simple_unlock(&pi_lock);
pi = FPTOPI(fp);
FREE(pi, M_PUFFS);
DPRINTF(("puffs_fop_close: pmp associated with fp %p was "
"dead\n", fp));
return 0;
}
@ -208,6 +217,28 @@ puffs_fop_close(struct file *fp, struct lwp *l)
*/
puffs_userdead(pmp);
/*
* Make sure someone from puffs_unmount() isn't currently in
* userspace. If we don't take this precautionary step,
* they might notice that the mountpoint has disappeared
* from under them once they return. Especially note that we
* cannot simply test for an unmounter before calling
* dounmount(), since it might be possible that that particular
* invocation of unmount was called without MNT_FORCE. Here we
* *must* make sure unmount succeeds. Also, restart is necessary
* since pmp isn't locked. We might end up with PMP_DEAD after
* restart and exit from there.
*/
simple_lock(&pmp->pmp_lock);
if (pmp->pmp_unmounting) {
ltsleep(&pmp->pmp_unmounting, PNORELOCK | PVFS, "puffsum",
0, &pmp->pmp_lock);
DPRINTF(("puffs_fop_close: unmount was in progress for pmp %p, "
"restart\n", pmp));
goto restart;
}
simple_unlock(&pmp->pmp_lock);
/*
* Detach from VFS. First do necessary XXX-dance (from
* sys_unmount() & other callers of dounmount()
@ -253,7 +284,7 @@ puffs_fop_close(struct file *fp, struct lwp *l)
}
/* Once we have the mount point, unmount() can't interfere */
dounmount(mp, MNT_FORCE, l);
rv = dounmount(mp, MNT_FORCE, l);
return 0;
}

View File

@ -1,4 +1,4 @@
/* $NetBSD: puffs_vfsops.c,v 1.15 2006/12/09 16:11:51 chs Exp $ */
/* $NetBSD: puffs_vfsops.c,v 1.16 2006/12/10 22:33:31 pooka Exp $ */
/*
* Copyright (c) 2005, 2006 Antti Kantee. All Rights Reserved.
@ -33,7 +33,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: puffs_vfsops.c,v 1.15 2006/12/09 16:11:51 chs Exp $");
__KERNEL_RCSID(0, "$NetBSD: puffs_vfsops.c,v 1.16 2006/12/10 22:33:31 pooka Exp $");
#include <sys/param.h>
#include <sys/mount.h>
@ -226,23 +226,35 @@ puffs_unmount(struct mount *mp, int mntflags, struct lwp *l)
* If we are not DYING, we should ask userspace's opinion
* about the situation
*/
simple_lock(&pmp->pmp_lock);
if (pmp->pmp_status != PUFFSTAT_DYING) {
pmp->pmp_unmounting = 1;
simple_unlock(&pmp->pmp_lock);
unmount_arg.pvfsr_flags = mntflags;
unmount_arg.pvfsr_pid = puffs_lwp2pid(l);
error = puffs_vfstouser(pmp, PUFFS_VFS_UNMOUNT,
&unmount_arg, sizeof(unmount_arg));
DPRINTF(("puffs_unmount: error %d force %d\n", error, force));
simple_lock(&pmp->pmp_lock);
pmp->pmp_unmounting = 0;
wakeup(&pmp->pmp_unmounting);
}
/*
* if userspace cooperated or we really need to die,
* screw what userland thinks and just die.
*/
DPRINTF(("puffs_unmount: error %d force %d\n", error, force));
if (error == 0 || force) {
pmp->pmp_status = PUFFSTAT_DYING;
puffs_nukebypmp(pmp);
simple_unlock(&pmp->pmp_lock);
FREE(pmp, M_PUFFS);
error = 0;
} else {
simple_unlock(&pmp->pmp_lock);
}
out: