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