Module Name: src
Committed By: bouyer
Date: Sat Sep 17 18:53:30 UTC 2011
Modified Files:
src/sys/fs/puffs [netbsd-5]: puffs_node.c puffs_sys.h puffs_vnops.c
Log Message:
Pull up following revision(s) (requested by manu in ticket #1666):
sys/fs/puffs/puffs_sys.h: revision 1.78 via patch
sys/fs/puffs/puffs_node.c: revision 1.20 via patch
sys/fs/puffs/puffs_vnops.c: revision 1.155 via patch
Add a mutex for operations that touch size (setattr, getattr, write, fsync).
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.
To generate a diff of this commit:
cvs rdiff -u -r1.13.10.1 -r1.13.10.2 src/sys/fs/puffs/puffs_node.c
cvs rdiff -u -r1.70.20.2 -r1.70.20.3 src/sys/fs/puffs/puffs_sys.h
cvs rdiff -u -r1.129.4.9 -r1.129.4.10 src/sys/fs/puffs/puffs_vnops.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/fs/puffs/puffs_node.c
diff -u src/sys/fs/puffs/puffs_node.c:1.13.10.1 src/sys/fs/puffs/puffs_node.c:1.13.10.2
--- src/sys/fs/puffs/puffs_node.c:1.13.10.1 Sat Oct 3 23:11:27 2009
+++ src/sys/fs/puffs/puffs_node.c Sat Sep 17 18:53:30 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: puffs_node.c,v 1.13.10.1 2009/10/03 23:11:27 snj Exp $ */
+/* $NetBSD: puffs_node.c,v 1.13.10.2 2011/09/17 18:53:30 bouyer 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.13.10.1 2009/10/03 23:11:27 snj Exp $");
+__KERNEL_RCSID(0, "$NetBSD: puffs_node.c,v 1.13.10.2 2011/09/17 18:53:30 bouyer Exp $");
#include <sys/param.h>
#include <sys/hash.h>
@@ -155,6 +155,7 @@
}
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 @@
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 {
Index: src/sys/fs/puffs/puffs_sys.h
diff -u src/sys/fs/puffs/puffs_sys.h:1.70.20.2 src/sys/fs/puffs/puffs_sys.h:1.70.20.3
--- src/sys/fs/puffs/puffs_sys.h:1.70.20.2 Sat Jun 18 16:19:39 2011
+++ src/sys/fs/puffs/puffs_sys.h Sat Sep 17 18:53:29 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: puffs_sys.h,v 1.70.20.2 2011/06/18 16:19:39 bouyer Exp $ */
+/* $NetBSD: puffs_sys.h,v 1.70.20.3 2011/09/17 18:53:29 bouyer Exp $ */
/*
* Copyright (c) 2005, 2006 Antti Kantee. All Rights Reserved.
@@ -205,6 +205,8 @@
voff_t pn_serversize;
struct lockf * pn_lockf;
+ kmutex_t pn_sizemtx; /* size modification mutex */
+
LIST_ENTRY(puffs_node) pn_hashent;
};
Index: src/sys/fs/puffs/puffs_vnops.c
diff -u src/sys/fs/puffs/puffs_vnops.c:1.129.4.9 src/sys/fs/puffs/puffs_vnops.c:1.129.4.10
--- src/sys/fs/puffs/puffs_vnops.c:1.129.4.9 Sun Jul 17 15:36:03 2011
+++ src/sys/fs/puffs/puffs_vnops.c Sat Sep 17 18:53:30 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: puffs_vnops.c,v 1.129.4.9 2011/07/17 15:36:03 riz Exp $ */
+/* $NetBSD: puffs_vnops.c,v 1.129.4.10 2011/09/17 18:53:30 bouyer 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.129.4.9 2011/07/17 15:36:03 riz Exp $");
+__KERNEL_RCSID(0, "$NetBSD: puffs_vnops.c,v 1.129.4.10 2011/09/17 18:53:30 bouyer Exp $");
#include <sys/param.h>
#include <sys/fstrans.h>
@@ -856,6 +856,18 @@
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;
@@ -906,6 +918,9 @@
out:
puffs_releasenode(pn);
PUFFS_MSG_RELEASE(getattr);
+
+ mutex_exit(&pn->pn_sizemtx);
+
return error;
}
@@ -919,6 +934,8 @@
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
@@ -989,8 +1006,14 @@
struct vattr *a_vap;
kauth_cred_t a_cred;
} */ *ap = v;
+ struct puffs_node *pn = ap->a_vp->v_data;
+ int error;
+
+ mutex_enter(&pn->pn_sizemtx);
+ error = dosetattr(ap->a_vp, ap->a_vap, ap->a_cred, SETATTR_CHSIZE);
+ mutex_exit(&pn->pn_sizemtx);
- return dosetattr(ap->a_vp, ap->a_vap, ap->a_cred, SETATTR_CHSIZE);
+ return error;
}
static __inline int
@@ -1040,6 +1063,7 @@
int error;
pnode = vp->v_data;
+ mutex_enter(&pnode->pn_sizemtx);
if (doinact(pmp, pnode->pn_stat & PNODE_DOINACT)) {
flushvncache(vp, 0, 0, false);
@@ -1059,6 +1083,7 @@
*/
*ap->a_recycle = ((pnode->pn_stat & PNODE_NOREFS) != 0);
+ mutex_exit(&pnode->pn_sizemtx);
VOP_UNLOCK(vp, 0);
return 0;
@@ -1342,13 +1367,15 @@
} */ *ap = v;
PUFFS_MSG_VARS(vn, fsync);
struct vnode *vp = ap->a_vp;
+ 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
@@ -1356,8 +1383,9 @@
* has references neither in the kernel or the fs server.
* Otherwise we continue to issue fsync() forward.
*/
+ error = 0;
if (!EXISTSOP(pmp, FSYNC))
- return 0;
+ goto out;
dofaf = (ap->a_flags & FSYNC_WAIT) == 0 || ap->a_flags == FSYNC_LAZY;
/*
@@ -1390,6 +1418,8 @@
error = checkerr(pmp, error, __func__);
+out:
+ mutex_exit(&pn->pn_sizemtx);
return error;
}
@@ -1932,6 +1962,7 @@
} */ *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;
@@ -1943,6 +1974,8 @@
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;
if (UBC_WANT_UNMAP(vp))
@@ -2066,6 +2099,7 @@
puffs_msgmem_release(park_write);
}
+ mutex_exit(&pn->pn_sizemtx);
return error;
}
