The patch I posted yesterday has a race condition, if a user process
quickly sets two attributes on two different filesystems, then the second
one will panic on VFS_ROOT() in namei() because the root vnode is already
locked.
I tried an alternative version that does not resolve a path from the root,
but I get various hang, crashes and panic. I struggle to find what is wrong
here. Any hint anyone?
/*
* Autocreate an attribute storage
*/
static struct ufs_extattr_list_entry *
ufs_extattr_autocreate_attr(struct ufsmount *ump, int attrnamespace,
const char *attrname, struct lwp *l)
{
struct vnode *root_vp;
struct vnode *backing_vp;
struct nameidata nd;
struct vattr va;
char path[PATH_MAX + 1];
struct ufs_extattr_fileheader uef;
struct ufs_extattr_list_entry *uele;
int need_close;
int error;
root_vp = NULL;
backing_vp = NULL;
nd.ni_vp = NULL;
nd.ni_dvp = NULL;
uele = NULL;
need_close = 0;
/*
* We only support system and user namespace autocreation
*/
switch (attrnamespace) {
case EXTATTR_NAMESPACE_SYSTEM:
(void)snprintf(path, PATH_MAX, "%s/%s/%s",
UFS_EXTATTR_FSROOTSUBDIR,
UFS_EXTATTR_SUBDIR_SYSTEM,
attrname);
break;
case EXTATTR_NAMESPACE_USER:
(void)snprintf(path, PATH_MAX, "%s/%s/%s",
UFS_EXTATTR_FSROOTSUBDIR,
UFS_EXTATTR_SUBDIR_USER,
attrname);
break;
default:
return NULL;
break;
}
/*
* Get filesystem root
*/
error = vfs_busy(ump->um_mountp, NULL);
if (error != 0) {
printf("%s: vfs_busy() returned %d\n", __func__, error);
return NULL;
}
if ((error = VFS_ROOT(ump->um_mountp, &root_vp)) != 0)
printf("%s: VFS_ROOT() returned %d\n", __func__, error);
vfs_unbusy(ump->um_mountp, false, NULL);
if (error != 0)
return NULL;
VOP_UNLOCK(root_vp, 0);
/*
* Lookup backing store for creation
*/
NDINIT(&nd, CREATE, LOCKPARENT|LOCKLEAF, UIO_SYSSPACE, path);
nd.ni_startdir = root_vp;
nd.ni_rootdir = root_vp;
if ((error = lookup(&nd)) != 0)
goto out;
KASSERT(nd.ni_vp == NULL);
KASSERT(nd.ni_dvp != NULL);
KASSERT(VOP_ISLOCKED(nd.ni_dvp) == LK_EXCLUSIVE);
KASSERT(VOP_ISLOCKED(root_vp) == 0);
/*
* Create backing store
*/
VATTR_NULL(&va);
va.va_type = VREG;
va.va_mode = 0600;
va.va_vaflags |= VA_EXCLUSIVE;
error = VOP_CREATE(nd.ni_dvp, &backing_vp, &nd.ni_cnd, &va);
if (error != 0) {
printf("%s: cannot create %s, error = %d\n",
__func__, path, error);
goto out;
}
KASSERT(backing_vp != NULL);
KASSERT(VOP_ISLOCKED(backing_vp) == LK_EXCLUSIVE);
KASSERT(VOP_ISLOCKED(nd.ni_dvp) == LK_EXCLUSIVE);
/*
* Open backing store for writing
*/
error = VOP_OPEN(backing_vp, FREAD|FWRITE, l->l_cred);
if (error != 0) {
printf("%s: cannot open %s, error = %d\n",
__func__, path, error);
goto out;
}
/*
* Remember we have to close the file on failure
*/
need_close = 1;
/*
* This is decreased by vn_close
*/
mutex_enter(&backing_vp->v_interlock);
backing_vp->v_writecount++;
mutex_exit(&backing_vp->v_interlock);
/*
* Now write extended attribute header in backing store fiie
*/
uef.uef_magic = UFS_EXTATTR_MAGIC;
uef.uef_version = UFS_EXTATTR_VERSION;
uef.uef_size = UFS_EXTATTR_AUTOCREATE;
error = vn_rdwr(UIO_WRITE, backing_vp, &uef, sizeof(uef), 0,
UIO_SYSSPACE, IO_NODELOCKED|IO_APPEND,
l->l_cred, NULL, l);
KASSERT(VOP_ISLOCKED(backing_vp) == LK_EXCLUSIVE);
VOP_UNLOCK(backing_vp, 0);
if (error != 0) {
printf("%s: write uef header failed for %s, error = %d\n",
__func__, attrname, error);
goto out;
}
/*
* ufs_extattr_enable_with_open does it, we do it too
* but I am not sure of the purpose
*/
vref(backing_vp);
/*
* Enable the attribute.
*/
error = ufs_extattr_enable(ump, attrnamespace,
attrname, backing_vp, l);
if (error != 0) {
printf("%s: enable %s failed, error %d\n",
__func__, attrname, error);
goto out;
}
uele = ufs_extattr_find_attr(ump, attrnamespace, attrname);
if (uele == NULL) {
printf("%s: attribute %s created but not found!\n",
__func__, attrname);
goto out;
}
out:
if (backing_vp != NULL) {
/*
* Success, we keep the vnode open and unlocked
*/
if (uele != NULL) {
KASSERT(VOP_ISLOCKED(backing_vp) == 0);
printf("%s: UFS autocreated EA backing store "
"for attribute %s\n",
ump->um_mountp->mnt_stat.f_mntonname,
attrname);
}
/*
* Failure and it was left open, close it.
*/
else if (need_close) {
KASSERT(VOP_ISLOCKED(backing_vp) == 0);
vn_close(backing_vp, FREAD|FWRITE, l->l_cred);
}
/*
* Failure and it was unopen (but locked), just forget it
*/
else {
KASSERT(VOP_ISLOCKED(backing_vp) == LK_EXCLUSIVE);
vput(backing_vp);
}
}
if (nd.ni_dvp != NULL) {
KASSERT(VOP_ISLOCKED(nd.ni_dvp) == LK_EXCLUSIVE);
vput(nd.ni_dvp);
}
if (root_vp != NULL) {
KASSERT(VOP_ISLOCKED(root_vp) == 0);
vrele(root_vp);
}
return uele;
}
--
Emmanuel Dreyfus
m...@netbsd.org
