Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/ufs/ufs Provide correct locking for ufs_wapbl_rename. No...
details: https://anonhg.NetBSD.org/src/rev/59c5b101af3d
branches: trunk
changeset: 767384:59c5b101af3d
user: dholland <dholland%NetBSD.org@localhost>
date: Sun Jul 17 22:07:59 2011 +0000
description:
Provide correct locking for ufs_wapbl_rename. Note that this does not
fix the non-wapbl rename; that will be coming soon. This patch also
leaves a lot of the older locking-related code around in #if 0 blocks,
and there's a lot of leftover redundant logic. All that will be going
away later.
Relates to at least these PRs:
PR kern/24887
PR kern/41417
PR kern/42093
PR kern/43626
and possibly others.
diffstat:
sys/ufs/ufs/ufs_extern.h | 4 +-
sys/ufs/ufs/ufs_lookup.c | 127 ++++++++++++++-
sys/ufs/ufs/ufs_wapbl.c | 399 ++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 485 insertions(+), 45 deletions(-)
diffs (truncated from 750 to 300 lines):
diff -r bc7146526707 -r 59c5b101af3d sys/ufs/ufs/ufs_extern.h
--- a/sys/ufs/ufs/ufs_extern.h Sun Jul 17 22:02:26 2011 +0000
+++ b/sys/ufs/ufs/ufs_extern.h Sun Jul 17 22:07:59 2011 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ufs_extern.h,v 1.65 2011/07/12 16:59:49 dholland Exp $ */
+/* $NetBSD: ufs_extern.h,v 1.66 2011/07/17 22:07:59 dholland Exp $ */
/*-
* Copyright (c) 1991, 1993, 1994
@@ -135,6 +135,8 @@
struct inode *, ino_t, int, int, int);
int ufs_dirempty(struct inode *, ino_t, kauth_cred_t);
int ufs_checkpath(struct inode *, struct inode *, kauth_cred_t);
+int ufs_parentcheck(struct vnode *, struct vnode *, kauth_cred_t,
+ int *, struct vnode **);
int ufs_blkatoff(struct vnode *, off_t, char **, struct buf **, bool);
/* ufs_quota.c */
diff -r bc7146526707 -r 59c5b101af3d sys/ufs/ufs/ufs_lookup.c
--- a/sys/ufs/ufs/ufs_lookup.c Sun Jul 17 22:02:26 2011 +0000
+++ b/sys/ufs/ufs/ufs_lookup.c Sun Jul 17 22:07:59 2011 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ufs_lookup.c,v 1.110 2011/07/14 16:27:11 dholland Exp $ */
+/* $NetBSD: ufs_lookup.c,v 1.111 2011/07/17 22:07:59 dholland Exp $ */
/*
* Copyright (c) 1989, 1993
@@ -37,7 +37,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ufs_lookup.c,v 1.110 2011/07/14 16:27:11 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_lookup.c,v 1.111 2011/07/17 22:07:59 dholland Exp $");
#ifdef _KERNEL_OPT
#include "opt_ffs.h"
@@ -1308,6 +1308,129 @@
return (error);
}
+/*
+ * Extract the inode number of ".." from a directory.
+ * Helper for ufs_parentcheck.
+ */
+static int
+ufs_readdotdot(struct vnode *vp, int needswap, kauth_cred_t cred, ino_t *result)
+{
+ struct dirtemplate dirbuf;
+ int namlen, error;
+
+ error = vn_rdwr(UIO_READ, vp, &dirbuf,
+ sizeof (struct dirtemplate), (off_t)0, UIO_SYSSPACE,
+ IO_NODELOCKED, cred, NULL, NULL);
+ if (error) {
+ return error;
+ }
+
+#if (BYTE_ORDER == LITTLE_ENDIAN)
+ if (FSFMT(vp) && needswap == 0)
+ namlen = dirbuf.dotdot_type;
+ else
+ namlen = dirbuf.dotdot_namlen;
+#else
+ if (FSFMT(vp) && needswap != 0)
+ namlen = dirbuf.dotdot_type;
+ else
+ namlen = dirbuf.dotdot_namlen;
+#endif
+ if (namlen != 2 ||
+ dirbuf.dotdot_name[0] != '.' ||
+ dirbuf.dotdot_name[1] != '.') {
+ printf("ufs_readdotdot: directory %llu contains "
+ "garbage instead of ..\n",
+ (unsigned long long) VTOI(vp)->i_number);
+ return ENOTDIR;
+ }
+ *result = ufs_rw32(dirbuf.dotdot_ino, needswap);
+ return 0;
+}
+
+/*
+ * Check if LOWER is a descendent of UPPER. If we find UPPER, return
+ * nonzero in FOUND and return a reference to the immediate descendent
+ * of UPPER in UPPERCHILD. If we don't find UPPER (that is, if we
+ * reach the volume root and that isn't UPPER), return zero in FOUND
+ * and null in UPPERCHILD.
+ *
+ * Neither UPPER nor LOWER should be locked.
+ *
+ * On error (such as a permissions error checking up the directory
+ * tree) fail entirely.
+ *
+ * Note that UPPER and LOWER must be on the same volume, and because
+ * we inspect only that volume NEEDSWAP can be constant.
+ */
+int
+ufs_parentcheck(struct vnode *upper, struct vnode *lower, kauth_cred_t cred,
+ int *found_ret, struct vnode **upperchild_ret)
+{
+ const int needswap = UFS_MPNEEDSWAP(VTOI(lower)->i_ump);
+ ino_t upper_ino, found_ino;
+ struct vnode *current, *next;
+ int error;
+
+ if (upper == lower) {
+ vref(upper);
+ *found_ret = 1;
+ *upperchild_ret = upper;
+ return 0;
+ }
+ if (VTOI(lower)->i_number == ROOTINO) {
+ *found_ret = 0;
+ *upperchild_ret = NULL;
+ return 0;
+ }
+
+ upper_ino = VTOI(upper)->i_number;
+
+ current = lower;
+ vref(current);
+ vn_lock(current, LK_EXCLUSIVE | LK_RETRY);
+
+ for (;;) {
+ error = ufs_readdotdot(current, needswap, cred, &found_ino);
+ if (error) {
+ vput(current);
+ return error;
+ }
+ if (found_ino == upper_ino) {
+ VOP_UNLOCK(current);
+ *found_ret = 1;
+ *upperchild_ret = current;
+ return 0;
+ }
+ if (found_ino == ROOTINO) {
+ vput(current);
+ *found_ret = 0;
+ *upperchild_ret = NULL;
+ return 0;
+ }
+ VOP_UNLOCK(current);
+ error = VFS_VGET(current->v_mount, found_ino, &next);
+ if (error) {
+ vrele(current);
+ return error;
+ }
+ KASSERT(VOP_ISLOCKED(next));
+ if (next->v_type != VDIR) {
+ printf("ufs_parentcheck: inode %llu reached via .. of "
+ "inode %llu is not a directory\n",
+ (unsigned long long)VTOI(next)->i_number,
+ (unsigned long long)VTOI(current)->i_number);
+ vput(next);
+ vrele(current);
+ return ENOTDIR;
+ }
+ vrele(current);
+ current = next;
+ }
+
+ return 0;
+}
+
#define UFS_DIRRABLKS 0
int ufs_dirrablks = UFS_DIRRABLKS;
diff -r bc7146526707 -r 59c5b101af3d sys/ufs/ufs/ufs_wapbl.c
--- a/sys/ufs/ufs/ufs_wapbl.c Sun Jul 17 22:02:26 2011 +0000
+++ b/sys/ufs/ufs/ufs_wapbl.c Sun Jul 17 22:07:59 2011 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ufs_wapbl.c,v 1.16 2011/07/14 16:27:43 dholland Exp $ */
+/* $NetBSD: ufs_wapbl.c,v 1.17 2011/07/17 22:07:59 dholland Exp $ */
/*-
* Copyright (c) 2003,2006,2008 The NetBSD Foundation, Inc.
@@ -66,7 +66,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ufs_wapbl.c,v 1.16 2011/07/14 16:27:43 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_wapbl.c,v 1.17 2011/07/17 22:07:59 dholland Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -148,6 +148,138 @@
}
/*
+ * Wrapper for relookup that also updates the supplemental results.
+ */
+static int
+do_relookup(struct vnode *dvp, struct ufs_lookup_results *ulr,
+ struct vnode **vp, struct componentname *cnp)
+{
+ int error;
+
+ error = relookup(dvp, vp, cnp, 0);
+ if (error) {
+ return error;
+ }
+ /* update the supplemental reasults */
+ *ulr = VTOI(dvp)->i_crap;
+ UFS_CHECK_CRAPCOUNTER(VTOI(dvp));
+ return 0;
+}
+
+/*
+ * Lock and relookup a sequence of two directories and two children.
+ *
+ */
+static int
+lock_vnode_sequence(struct vnode *d1, struct ufs_lookup_results *ulr1,
+ struct vnode **v1_ret, struct componentname *cn1,
+ int v1_missing_ok,
+ int overlap_error,
+ struct vnode *d2, struct ufs_lookup_results *ulr2,
+ struct vnode **v2_ret, struct componentname *cn2,
+ int v2_missing_ok)
+{
+ struct vnode *v1, *v2;
+ int error;
+
+ KASSERT(d1 != d2);
+
+ vn_lock(d1, LK_EXCLUSIVE | LK_RETRY);
+ if (VTOI(d1)->i_size == 0) {
+ /* d1 has been rmdir'd */
+ VOP_UNLOCK(d1);
+ return ENOENT;
+ }
+ error = do_relookup(d1, ulr1, &v1, cn1);
+ if (v1_missing_ok) {
+ if (error == ENOENT) {
+ /*
+ * Note: currently if the name doesn't exist,
+ * relookup succeeds (it intercepts the
+ * EJUSTRETURN from VOP_LOOKUP) and sets tvp
+ * to NULL. Therefore, we will never get
+ * ENOENT and this branch is not needed.
+ * However, in a saner future the EJUSTRETURN
+ * garbage will go away, so let's DTRT.
+ */
+ v1 = NULL;
+ error = 0;
+ }
+ } else {
+ if (error == 0 && v1 == NULL) {
+ /* This is what relookup sets if v1 disappeared. */
+ error = ENOENT;
+ }
+ }
+ if (error) {
+ VOP_UNLOCK(d1);
+ return error;
+ }
+ if (v1 && v1 == d2) {
+ VOP_UNLOCK(d1);
+ VOP_UNLOCK(v1);
+ vrele(v1);
+ return overlap_error;
+ }
+
+ /*
+ * The right way to do this is to do lookups without locking
+ * the results, and lock the results afterwards; then at the
+ * end we can avoid trying to lock v2 if v2 == v1.
+ *
+ * However, for the reasons described in the fdvp == tdvp case
+ * in rename below, we can't do that safely. So, in the case
+ * where v1 is not a directory, unlock it and lock it again
+ * afterwards. This is safe in locking order because a
+ * non-directory can't be above anything else in the tree. If
+ * v1 *is* a directory, that's not true, but then because d1
+ * != d2, v1 != v2.
+ */
+ if (v1 && v1->v_type != VDIR) {
+ VOP_UNLOCK(v1);
+ }
+ vn_lock(d2, LK_EXCLUSIVE | LK_RETRY);
+ if (VTOI(d2)->i_size == 0) {
+ /* d2 has been rmdir'd */
+ VOP_UNLOCK(d2);
+ if (v1 && v1->v_type == VDIR) {
+ VOP_UNLOCK(v1);
+ }
+ VOP_UNLOCK(d1);
+ vrele(v1);
+ return ENOENT;
+ }
+ error = do_relookup(d2, ulr2, &v2, cn2);
+ if (v2_missing_ok) {
+ if (error == ENOENT) {
+ /* as above */
+ v2 = NULL;
+ error = 0;
+ }
+ } else {
+ if (error == 0 && v2 == NULL) {
+ /* This is what relookup sets if v2 disappeared. */
+ error = ENOENT;
Home |
Main Index |
Thread Index |
Old Index