NetBSD-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: mount_union(8) vs. open(O_RDWR)



The following seems to work, for the simplest cases at least.

I would appreciate if someone who knows this code could have a look to
see if I've missed any steps, or botched any locks, and to answer the
embedded questions.

--- union_vnops.c.~1.74.~	2021-03-07 17:23:02.000000000 -0800
+++ union_vnops.c	2021-12-04 00:36:56.318634542 -0800
@@ -622,6 +622,12 @@
 				un->un_uppervp->v_writecount++;
 				mutex_exit(un->un_uppervp->v_interlock);
 			}
+#ifdef UNION_DIAGNOSTIC
+			else {
+				printf("union_open: failed to open upper file for write %s: %d\n",
+				       un->un_path, error);
+			}
+#endif
 			return (error);
 		}

@@ -636,6 +642,12 @@
 		error = VOP_OPEN(tvp, mode, cred);
 		VOP_UNLOCK(tvp);

+#ifdef UNION_DIAGNOSTIC
+		if (error != 0) {
+			printf("union_open: failed to open lower file %s: %d\n",
+			       un->un_path, error);
+		}
+#endif
 		return (error);
 	}
 	/*
@@ -651,6 +663,12 @@
 		tvp->v_writecount++;
 		mutex_exit(tvp->v_interlock);
 	}
+#ifdef UNION_DIAGNOSTIC
+	if (error != 0) {
+		printf("union_open: failed to open upper file %s: %d\n",
+		       un->un_path, error);
+	}
+#endif

 	return (error);
 }
@@ -718,17 +736,63 @@
 	struct union_mount *um = MOUNTTOUNIONMOUNT(vp->v_mount);

 	/*
-	 * Disallow write attempts on read-only file systems;
-	 * unless the file is a socket, fifo, or a block or
-	 * character device resident on the file system.
+	 * Disallow write attempts on read-only file systems; unless the file is
+	 * readable by the user, or is a socket, fifo, or a block or character
+	 * device resident on the file system.
 	 */
-	if (ap->a_accmode & VWRITE) {
+	if (ap->a_accmode & VWRITE && un->un_uppervp == NULLVP) {
 		switch (vp->v_type) {
 		case VDIR:
 		case VLNK:
 		case VREG:
-			if (vp->v_mount->mnt_flag & MNT_RDONLY)
+			/*
+			 * XXX ToDo:  First check for read (and write?) access
+			 * on the underlying file system object (un->un_lowervp)
+			 *
+			 * If un->un_lowervp is readable then copy it up and
+			 * finally check the upper node (i.e. continue to the
+			 * next if).
+			 *
+			 * Does this work for VLNK and VDIR too?
+			 */
+			if (un->un_lowervp != NULLVP) {
+				struct vop_access_args ra;
+				int mode = ap->a_accmode;
+
+				ra = *ap;
+				ra.a_vp = un->un_lowervp;
+				ra.a_accmode &= !VWRITE;
+				ra.a_accmode |= VREAD;
+				vn_lock(un->un_lowervp, LK_EXCLUSIVE | LK_RETRY); /* xxx ? */
+				error = VCALL(un->un_lowervp, VOFFSET(vop_access), &ra);
+				VOP_UNLOCK(un->un_lowervp); /* xxx ? */
+				if (error != 0) {
+#ifdef UNION_DIAGNOSTIC
+					printf("union_access: read access on lvp denied for %s: %d\n",
+					       un->un_path, error);
+#endif
+					return (error);
+				}
+				/*
+				 * XXX in theory we only need to make the vnode,
+				 * not copy the file, but there's no logic
+				 * elsewhere to complete the copy later if/when
+				 * an open is attempted.
+				 */
+				error = union_copyup(un, ((mode & O_TRUNC) == 0), ap->a_cred, curlwp);
+				if (error != 0) {
+#ifdef UNION_DIAGNOSTIC
+					printf("union_access: failed copyup for %s: %d\n",
+					       un->un_path, error);
+#endif
+					return (error);
+				}
+			} else if (vp->v_mount->mnt_flag & MNT_RDONLY) {
+#ifdef UNION_DIAGNOSTIC
+				printf("union_access: EROFS for %s\n", un->un_path);
+#endif
 				return (EROFS);
+			}
 			break;
 		case VBAD:
 		case VBLK:
@@ -744,7 +808,14 @@

 	if ((vp = un->un_uppervp) != NULLVP) {
 		ap->a_vp = vp;
-		return (VCALL(vp, VOFFSET(vop_access), ap));
+		error = VCALL(vp, VOFFSET(vop_access), ap);
+#ifdef UNION_DIAGNOSTIC
+		if (error) {
+			printf("union_access: failed upper access check for %s: %d\n",
+			       un->un_path, error);
+		}
+#endif
+		return (error);
 	}

 	if ((vp = un->un_lowervp) != NULLVP) {
@@ -758,8 +829,13 @@
 			}
 		}
 		VOP_UNLOCK(vp);
-		if (error)
+		if (error) {
+#ifdef UNION_DIAGNOSTIC
+			printf("union_access: failed lower access check for %s: %d\n",
+			       un->un_path, error);
+#endif
 			return (error);
+		}
 	}

 	return (error);
@@ -1231,7 +1307,7 @@
 				 */
 				vp  = NULLVP;
 				if (dun->un_uppervp == NULLVP)
-					 panic("union: null upperdvp?");
+					 panic("union: null uppervp?");
 				error = relookup(ap->a_dvp, &vp, ap->a_cnp, 0);
 				if (error) {
 					VOP_UNLOCK(ap->a_vp);
@@ -1242,6 +1318,10 @@
 					 * The name we want to create has
 					 * mysteriously appeared (a race?)
 					 */
+#ifdef UNION_DIAGNOSTIC
+					printf("union_link: %s: already exists?\n",
+					       un->un_path);
+#endif
 					error = EEXIST;
 					VOP_UNLOCK(ap->a_vp);
 					vput(vp);



[   2.3112534] root file system type: cd9660
[   2.3262533] kern.module.path=/stand/amd64/9.99.81/modules
[   2.3362553] warning: no /dev/console
[   2.3362553] warning: no /dev/constty
Created tmpfs /dev (2064384 byte, 4000 inodes)
[   4.2142574] union_mount(mp = 0xffff888007b3e000)
[   4.2142574] union_mount: from <above>:/tmp/uvar, on /var
[   4.2142574] union_statvfs(mp = 0xffff888007b3e000, lvp = 0xffff888008cfcd40, uvp = 0xffff888008cfcac0)
[   4.3342534] union_newsize: wtmpx: lower size now 0
[   4.3352568] union: copied up wtmpx
[   4.3352568] union_newsize: wtmpx: upper size now 520
[   4.3352568] union_newsize: wtmp: lower size now 0
[   4.3352568] union: copied up wtmp
[   4.3402531] union_newsize: wtmp: upper size now 0
[   4.3452562] union_newsize: wtmp: upper size now 40
[   4.3502564] union_newsize: utmpx: lower size now 0
[   4.3502564] union: copied up utmpx
[   4.3502564] union_newsize: utmpx: upper size now 0
[   4.3602577] union_newsize: utmpx: upper size now 520
[   4.3602577] union_newsize: utmpx: upper size now 1040
[   4.3602577] union_newsize: utmpx: upper size now 1560
[   4.3742532] union_newsize: utmpx: upper size now 2080
init: kernel security level changed from 0 to 1
[   4.4112513] union_newsize: utmpx: upper size now 2600


--
					Greg A. Woods <gwoods%acm.org@localhost>

Kelowna, BC     +1 250 762-7675           RoboHack <woods%robohack.ca@localhost>
Planix, Inc. <woods%planix.com@localhost>     Avoncote Farms <woods%avoncote.ca@localhost>

Attachment: pgpm29y2TWtWp.pgp
Description: OpenPGP Digital Signature



Home | Main Index | Thread Index | Old Index