Aren/NetBSD
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add a mutex for operations that touch size (setattr, getattr, write, fsync).

Browse Source 
This is required to avoid data corruption bugs, where a getattr slices
itself within a setattr operation, and sets the size to the stall value
it got from the filesystem. That value is smaller than the one set by
setattr, and the call to uvm_vnp_setsize() trigged a spurious truncate.
The result is a chunk of zeroed data in the file.

Such a situation can easily happen when the ioflush thread issue a
VOP_FSYNC/puffs_vnop_sync/flushvncache/dosetattrn while andother process
do a sys_stat/VOP_GETATTR/puffs_vnop_getattr.

This mutex on size operation can be removed the day we decide VOP_GETATTR
has to operated on a locked vnode, since the other operations that touch
size already require that.
This commit is contained in:
manu 2011-08-29 04:12:45 +00:00
parent 501f07ce79
commit 96c935e33b
3 changed files with 46 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
sys/fs/puffs/puffs_node.c
View File

@ -1,4 +1,4 @@
/* $NetBSD: puffs_node.c,v 1.19 2011/06/30 20:09:41 wiz Exp $ */
/* $NetBSD: puffs_node.c,v 1.20 2011/08/29 04:12:45 manu Exp $ */
/*
* Copyright (c) 2005, 2006, 2007 Antti Kantee. All Rights Reserved.
@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: puffs_node.c,v 1.19 2011/06/30 20:09:41 wiz Exp $");
__KERNEL_RCSID(0, "$NetBSD: puffs_node.c,v 1.20 2011/08/29 04:12:45 manu Exp $");
#include <sys/param.h>
#include <sys/hash.h>
@ -155,6 +155,7 @@ puffs_getvnode(struct mount *mp, puffs_cookie_t ck, enum vtype type,
}
KASSERT(pnc != NULL);
}
mutex_init(&pnode->pn_sizemtx, MUTEX_DEFAULT, IPL_NONE);
mutex_exit(&pmp->pmp_lock);
vp->v_data = pnode;
@ -463,6 +464,7 @@ puffs_releasenode(struct puffs_node *pn)
if (--pn->pn_refcount == 0) {
mutex_exit(&pn->pn_mtx);
mutex_destroy(&pn->pn_mtx);
mutex_destroy(&pn->pn_sizemtx);
seldestroy(&pn->pn_sel);
pool_put(&puffs_pnpool, pn);
} else {

4
sys/fs/puffs/puffs_sys.h
View File

@ -1,4 +1,4 @@
/* $NetBSD: puffs_sys.h,v 1.77 2011/01/11 14:04:54 kefren Exp $ */
/* $NetBSD: puffs_sys.h,v 1.78 2011/08/29 04:12:45 manu Exp $ */
/*
* Copyright (c) 2005, 2006 Antti Kantee. All Rights Reserved.
@ -209,6 +209,8 @@ struct puffs_node {
struct lockf * pn_lockf;
kmutex_t pn_sizemtx; /* size modification mutex */
LIST_ENTRY(puffs_node) pn_hashent;
};

45
sys/fs/puffs/puffs_vnops.c
View File

@ -1,4 +1,4 @@
/* $NetBSD: puffs_vnops.c,v 1.154 2011/07/04 08:07:30 manu Exp $ */
/* $NetBSD: puffs_vnops.c,v 1.155 2011/08/29 04:12:45 manu Exp $ */
/*
* Copyright (c) 2005, 2006, 2007 Antti Kantee. All Rights Reserved.
@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: puffs_vnops.c,v 1.154 2011/07/04 08:07:30 manu Exp $");
__KERNEL_RCSID(0, "$NetBSD: puffs_vnops.c,v 1.155 2011/08/29 04:12:45 manu Exp $");
#include <sys/param.h>
#include <sys/buf.h>
@ -839,6 +839,18 @@ puffs_vnop_getattr(void *v)
struct puffs_node *pn = VPTOPP(vp);
int error = 0;
/*
* A lock is required so that we do not race with
* setattr, write and fsync when changing vp->v_size.
* This is critical, since setting a stall smaler value
* triggers a file truncate in uvm_vnp_setsize(), which
* most of the time means data corruption (a chunk of
* data is replaced by zeroes). This can be removed if
* we decide one day that VOP_GETATTR must operate on
* a locked vnode.
*/
mutex_enter(&pn->pn_sizemtx);
REFPN(pn);
vap = ap->a_vap;
@ -889,6 +901,9 @@ puffs_vnop_getattr(void *v)
out:
puffs_releasenode(pn);
PUFFS_MSG_RELEASE(getattr);
mutex_exit(&pn->pn_sizemtx);
return error;
}
@ -902,6 +917,8 @@ dosetattr(struct vnode *vp, struct vattr *vap, kauth_cred_t cred, int flags)
struct puffs_node *pn = vp->v_data;
int error = 0;
KASSERT(!(flags & SETATTR_CHSIZE) || mutex_owned(&pn->pn_sizemtx));
if ((vp->v_mount->mnt_flag & MNT_RDONLY) &&
(vap->va_uid != (uid_t)VNOVAL || vap->va_gid != (gid_t)VNOVAL
|| vap->va_atime.tv_sec != VNOVAL || vap->va_mtime.tv_sec != VNOVAL
@ -972,8 +989,14 @@ puffs_vnop_setattr(void *v)
struct vattr *a_vap;
kauth_cred_t a_cred;
} */ *ap = v;
struct puffs_node *pn = ap->a_vp->v_data;
int error;
return dosetattr(ap->a_vp, ap->a_vap, ap->a_cred, SETATTR_CHSIZE);
mutex_enter(&pn->pn_sizemtx);
error = dosetattr(ap->a_vp, ap->a_vap, ap->a_cred, SETATTR_CHSIZE);
mutex_exit(&pn->pn_sizemtx);
return error;
}
static __inline int
@ -1023,6 +1046,7 @@ puffs_vnop_inactive(void *v)
int error;
pnode = vp->v_data;
mutex_enter(&pnode->pn_sizemtx);
if (doinact(pmp, pnode->pn_stat & PNODE_DOINACT)) {
flushvncache(vp, 0, 0, false);
@ -1045,6 +1069,7 @@ puffs_vnop_inactive(void *v)
*ap->a_recycle = true;
}
mutex_exit(&pnode->pn_sizemtx);
VOP_UNLOCK(vp);
return 0;
@ -1328,14 +1353,15 @@ puffs_vnop_fsync(void *v)
} */ *ap = v;
PUFFS_MSG_VARS(vn, fsync);
struct vnode *vp = ap->a_vp;
struct puffs_mount *pmp = MPTOPUFFSMP(vp->v_mount);
struct puffs_node *pn = VPTOPP(vp);
struct puffs_mount *pmp = MPTOPUFFSMP(vp->v_mount);
int error, dofaf;
mutex_enter(&pn->pn_sizemtx);
error = flushvncache(vp, ap->a_offlo, ap->a_offhi,
(ap->a_flags & FSYNC_WAIT) == FSYNC_WAIT);
if (error)
return error;
goto out;
/*
* HELLO! We exit already here if the user server does not
@ -1343,8 +1369,9 @@ puffs_vnop_fsync(void *v)
* has references neither in the kernel or the fs server.
* Otherwise we continue to issue fsync() forward.
*/
error = 0;
if (!EXISTSOP(pmp, FSYNC) || (pn->pn_stat & PNODE_DYING))
return 0;
goto out;
dofaf = (ap->a_flags & FSYNC_WAIT) == 0 || ap->a_flags == FSYNC_LAZY;
/*
@ -1377,6 +1404,8 @@ puffs_vnop_fsync(void *v)
error = checkerr(pmp, error, __func__);
out:
mutex_exit(&pn->pn_sizemtx);
return error;
}
@ -1902,6 +1931,7 @@ puffs_vnop_write(void *v)
} */ *ap = v;
PUFFS_MSG_VARS(vn, write);
struct vnode *vp = ap->a_vp;
struct puffs_node *pn = VPTOPP(vp);
struct puffs_mount *pmp = MPTOPUFFSMP(vp->v_mount);
struct uio *uio = ap->a_uio;
size_t tomove, argsize;
@ -1913,6 +1943,8 @@ puffs_vnop_write(void *v)
error = uflags = 0;
write_msg = NULL;
mutex_enter(&pn->pn_sizemtx);
if (vp->v_type == VREG && PUFFS_USE_PAGECACHE(pmp)) {
ubcflags = UBC_WRITE | UBC_PARTIALOK | UBC_UNMAP_FLAG(vp);
@ -2034,6 +2066,7 @@ puffs_vnop_write(void *v)
puffs_msgmem_release(park_write);
}
mutex_exit(&pn->pn_sizemtx);
return error;
}