At Thu, 2 Dec 2021 17:23:37 +0100, "J. Hannken-Illjes" <hannken%mailbox.org@localhost> wrote:
Subject: Re: mount_union(8) vs. open(O_RDWR)
>
> This behaviour comes from sys/fs/union/union_vnops.c::union_access():
>
> /*
> * Check access permission on the union vnode.
> * The access check being enforced is to check
> * against both the underlying vnode, and any
> * copied vnode. This ensures that no additional
> * file permissions are given away simply because
> * the user caused an implicit file copy.
> */
> int
> union_access(void *v)
>
> As the underlying node is on a ro-mounted file system this will
> always fail with EROFS if the underlying node exists.
Ah, yes, of course -- I forgot to grep for EROFS and missed that.
I also had trouble searching, e.g. the mailing list archives, with
google -- it (google) has become increasingly useless for such tasks and
it would be very nice if there were a local search feature on
mail-index.
With more careful searching I now find discussions going back literally
decades which directly and indirectly circled around this same issue and
related issues.
> Maybe we should check for read access on the underlying file system,
> copy up and then check the upper node.
I haven't worked on vnode-ops code for a long while so I might be off on
the wrong foot or missing a step (and have left some other open
questions in the comments), but perhaps something like this (i.e. with
the #if 0 removed):
--- union_vnops.c.~1.74.~ 2021-03-07 17:23:02.000000000 -0800
+++ union_vnops.c 2021-12-03 12:26:03.148904408 -0800
@@ -718,15 +718,51 @@
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) {
switch (vp->v_type) {
case VDIR:
case VLNK:
case VREG:
+ /*
+ * 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 0
+ if (un->un_lowervp != NULLVP) {
+ struct vop_access_args ra;
+ int mode = ap->a_mode;
+
+ ra = *ap;
+ ra.a_vp = un->un_lowervp;
+ 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) {
+ return (EROFS); /* xxx or just (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) {
+ return (error);
+ }
+ } else
+#endif
if (vp->v_mount->mnt_flag & MNT_RDONLY)
return (EROFS);
break;
Also there's still no support for whiteouts of directories (though that
shouldn't cause too much problem for my current needs).
BTW, poking around in the un-finished import of FreeBSD's "unionfs"
shows it would need similar changes. Indeed the FreeBSD native version
seems to still require similar changes. Also it seems as though it
would now be easier to restart the import with fresh FreeBSD sources.
Such a re-import would still seem to be highly useful to more easily
acquire the new features in FreeBSD's unionfs (which may include
whiteout support for directories?), with the caveat that their manual
page still warns of the seriously experimental nature of their code.
I've yet to try their code.
--
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:
pgpt7J9ZlgkeR.pgp
Description: OpenPGP Digital Signature