[src/trunk]: src/sys/fs/union VOP_GETATTR() needs a shared lock at least.

[hannken <hannken%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/ce79b5d0aeb9
branches:  trunk
changeset: 770467:ce79b5d0aeb9
user:      hannken <hannken%NetBSD.org@localhost>
date:      Tue Oct 18 09:22:53 2011 +0000

description:
VOP_GETATTR() needs a shared lock at least.

diffstat:

 sys/fs/union/union_subr.c  |  24 +++++++++++++-----------
 sys/fs/union/union_vnops.c |  16 ++++++----------
 2 files changed, 19 insertions(+), 21 deletions(-)

diffs (103 lines):

diff -r a76f85a24d16 -r ce79b5d0aeb9 sys/fs/union/union_subr.c
--- a/sys/fs/union/union_subr.c Tue Oct 18 05:16:02 2011 +0000
+++ b/sys/fs/union/union_subr.c Tue Oct 18 09:22:53 2011 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: union_subr.c,v 1.50 2011/08/23 07:39:37 hannken Exp $  */
+/*     $NetBSD: union_subr.c,v 1.51 2011/10/18 09:22:53 hannken Exp $  */
 
 /*
  * Copyright (c) 1994
@@ -72,7 +72,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: union_subr.c,v 1.50 2011/08/23 07:39:37 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: union_subr.c,v 1.51 2011/10/18 09:22:53 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -472,9 +472,13 @@
        if (uppervp != NULLVP)
                if (VOP_GETATTR(uppervp, &va, FSCRED) == 0)
                        uppersz = va.va_size;
-       if (lowervp != NULLVP)
-               if (VOP_GETATTR(lowervp, &va, FSCRED) == 0)
+       if (lowervp != NULLVP) {
+               vn_lock(lowervp, LK_SHARED | LK_RETRY);
+               error = VOP_GETATTR(lowervp, &va, FSCRED);
+               VOP_UNLOCK(lowervp);
+               if (error == 0)
                        lowersz = va.va_size;
+       }
        hash = UNION_HASH(uppervp, lowervp);
 
        /*
@@ -1213,18 +1217,16 @@
        if (vp->v_op != union_vnodeop_p)
                return (0);
 
-       if ((lvp = union_dircache(vp, l)) == NULLVP)
-               return (0);
-
        /*
         * If the directory is opaque,
         * then don't show lower entries
         */
        error = VOP_GETATTR(vp, &va, fp->f_cred);
-       if (error || (va.va_flags & OPAQUE)) {
-               vput(lvp);
-               return (error);
-       }
+       if (error || (va.va_flags & OPAQUE))
+               return error;
+
+       if ((lvp = union_dircache(vp, l)) == NULLVP)
+               return (0);
 
        error = VOP_OPEN(lvp, FREAD, fp->f_cred);
        if (error) {
diff -r a76f85a24d16 -r ce79b5d0aeb9 sys/fs/union/union_vnops.c
--- a/sys/fs/union/union_vnops.c        Tue Oct 18 05:16:02 2011 +0000
+++ b/sys/fs/union/union_vnops.c        Tue Oct 18 09:22:53 2011 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: union_vnops.c,v 1.46 2011/08/23 07:39:37 hannken Exp $ */
+/*     $NetBSD: union_vnops.c,v 1.47 2011/10/18 09:22:53 hannken Exp $ */
 
 /*
  * Copyright (c) 1992, 1993, 1994, 1995
@@ -72,7 +72,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: union_vnops.c,v 1.46 2011/08/23 07:39:37 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: union_vnops.c,v 1.47 2011/10/18 09:22:53 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -830,14 +830,6 @@
 
        vp = un->un_uppervp;
        if (vp != NULLVP) {
-               /*
-                * It's not clear whether VOP_GETATTR is to be
-                * called with the vnode locked or not.  stat() calls
-                * it with (vp) locked, and fstat calls it with
-                * (vp) unlocked.
-                * In the mean time, compensate here by checking
-                * the union_node's lock flag.
-                */
                if (un->un_flags & UN_LOCKED)
                        FIXUP(un);
 
@@ -858,7 +850,11 @@
        }
 
        if (vp != NULLVP) {
+               if (vp == un->un_lowervp)
+                       vn_lock(vp, LK_SHARED | LK_RETRY);
                error = VOP_GETATTR(vp, vap, ap->a_cred);
+               if (vp == un->un_lowervp)
+                       VOP_UNLOCK(vp);
                if (error)
                        return (error);
                union_newsize(ap->a_vp, VNOVAL, vap->va_size);