Module Name: src
Committed By: manu
Date: Mon Aug 29 04:12:46 UTC 2011
Modified Files:
src/sys/fs/puffs: puffs_node.c puffs_sys.h puffs_vnops.c
Log Message:
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.19 -r1.20 src/sys/fs/puffs/puffs_node.c
cvs rdiff -u -r1.77 -r1.78 src/sys/fs/puffs/puffs_sys.h
cvs rdiff -u -r1.154 -r1.155 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.19 src/sys/fs/puffs/puffs_node.c:1.20
--- src/sys/fs/puffs/puffs_node.c:1.19 Thu Jun 30 20:09:41 2011
+++ src/sys/fs/puffs/puffs_node.c Mon Aug 29 04:12:45 2011
@@ -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 @@
}
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.77 src/sys/fs/puffs/puffs_sys.h:1.78
--- src/sys/fs/puffs/puffs_sys.h:1.77 Tue Jan 11 14:04:54 2011
+++ src/sys/fs/puffs/puffs_sys.h Mon Aug 29 04:12:45 2011
@@ -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 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.154 src/sys/fs/puffs/puffs_vnops.c:1.155
--- src/sys/fs/puffs/puffs_vnops.c:1.154 Mon Jul 4 08:07:30 2011
+++ src/sys/fs/puffs/puffs_vnops.c Mon Aug 29 04:12:45 2011
@@ -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 @@
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 @@
out:
puffs_releasenode(pn);
PUFFS_MSG_RELEASE(getattr);
+
+ mutex_exit(&pn->pn_sizemtx);
+
return error;
}
@@ -902,6 +917,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
@@ -972,8 +989,14 @@
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 @@
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 @@
*ap->a_recycle = true;
}
+ mutex_exit(&pnode->pn_sizemtx);
VOP_UNLOCK(vp);
return 0;
@@ -1328,14 +1353,15 @@
} */ *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 @@
* 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 @@
error = checkerr(pmp, error, __func__);
+out:
+ mutex_exit(&pn->pn_sizemtx);
return error;
}
@@ -1902,6 +1931,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;
@@ -1913,6 +1943,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 | UBC_UNMAP_FLAG(vp);
@@ -2034,6 +2066,7 @@
puffs_msgmem_release(park_write);
}
+ mutex_exit(&pn->pn_sizemtx);
return error;
}
