Subject: Unionfs write problem with RO bottom layer.
To: None <tech-kern@netbsd.org>
From: Thor Lancelot Simon <tls@rek.tjls.com>
List: tech-kern
Date: 11/27/2007 20:08:49
I discovered today that unionfs can no longer truncate or append to
files present in a read-only bottom layer, because of the VOP_ACCESS
calls in sys_open().  This is because union_access() gets called with
a vnode which has no union upper layer vnode associated with it, and,
unlike union_open, which can simply cope with this by creating the
new upper layer node immediately, does not correctly test to see if
it would be possible to write the upper layer node.

To reproduce:

1) Mount /l0 read-write
2) touch /l0/foo
3) Remount /l0 read-only
4) Mount /l1 read-write
4) Union mount /l1 over /l0: mount_union /l1 /l0
5) cat > /l0/foo or cat >> /l0/foo (either will fail with EROFS).

The same exercise works correctly (creates /l1/foo with new contents,
leaves /l0/foo unmodified underneath) if /l0 is mounted read-write.  Also,
if /l0/foo does not exist, /l1/foo (and the node in the union namespace)
is created correctly even if /l0 is mounted RO, I beleive because in the
O_CREAT case the VOP_ACCESS() checks in sys_open() are skipped. 

Below is a patch that attempts to fix this.  It does not work, probably
because the second hunk is wrong (I think the first hunk is still
necessary).  I don't know how to fix it better but it's definitely broken;
these are both cases in which the mount_union manual page says that a new
file will be created in the upper layer.

I considered adding an new "docopy" value to union_copyup() to cause it
to hold the vnode locked and remove the new upper-layer node after
confirming that it can create it.  Unfortunately this would, I think,
cause real problems with NFS as an upper layer.

I'm not sure when the VOP_ACCESS calls were added to sys_open before
the actual VOP_OPEN is tried but this appears to have been broken since
at least 3.0.  I think access() on unionfs with a RO bottom layer has
_always_ been broken.

Index: union_vnops.c
===================================================================
RCS file: /workspaces/cvsroot/nbsrc/sys/fs/union/union_vnops.c,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 union_vnops.c
--- union_vnops.c	18 Oct 2007 20:25:21 -0000	1.1.1.2
+++ union_vnops.c	27 Nov 2007 23:39:33 -0000
@@ -755,7 +755,8 @@
 		case VDIR:
 		case VLNK:
 		case VREG:
-			if (vp->v_mount->mnt_flag & MNT_RDONLY)
+			if (um->um_uppervp->v_mount->mnt_flag & MNT_RDONLY)
+			/* if (vp->v_mount->mnt_flag & MNT_RDONLY) */
 				return (EROFS);
 			break;
 		case VBAD:
@@ -778,6 +779,23 @@
 
 	if ((vp = un->un_lowervp) != NULLVP) {
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
+
+		if (ap->a_mode & VWRITE) {
+			struct union_node *un2 = un;
+			while(un2->un_dirvp != um->um_uppervp) {
+				if ((vp = un2->un_uppervp) != NULLVP) {
+					FIXUP(un2);
+					ap->a_vp = vp;
+					error = VCALL(vp,
+					              VOFFSET(vop_access), ap);
+					VOP_UNLOCK(vp, 0);
+					return (error);
+				}
+				un2 = VTOUNION(un->un_dirvp);
+			}
+		}
+					
+				
 		ap->a_vp = vp;
 		error = VCALL(vp, VOFFSET(vop_access), ap);
 		if (error == 0) {