On Thu, Jul 29, 2021 at 10:43:23AM +1000, NeilBrown wrote:
> On Wed, 28 Jul 2021, Christian Brauner wrote:
> >
> > Hey Neil,
> >
> > Sorry if this is a stupid question but wouldn't you want to copy the
> > mount properties from path->mnt here? Couldn't you otherwise use this to
> > e.g. suddenly expose a dentry on a read-only mount as read-write?
>
> There are no stupid questions, and this is a particularly non-stupid
> one!
>
> I hadn't considered that, but having examined the code I see that it
> is already handled.
> The vfsmount that d_automount returns is passed to finish_automount(),
> which hands it to do_add_mount() together with the mnt_flags for the
> parent vfsmount (plus MNT_SHRINKABLE).
> do_add_mount() sets up the mnt_flags of the new vfsmount.
> In fact, the d_automount interface has no control of these flags at all.
> Whatever it sets will be over-written by do_add_mount.
Ah, interesting thank you very much, Neil. I seemed to have overlooked
this yesterday.
If btrfs makes use of automounts the way you envisioned to expose
subvolumes and also will support idmapped mounts (see [1]) we need to
teach do_add_mount() to also take the idmapped mount into account. So
you'd need something like (entirely untested):
diff --git a/fs/namespace.c b/fs/namespace.c
index ab4174a3c802..921f6396c36d 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2811,6 +2811,11 @@ static int do_add_mount(struct mount *newmnt, struct mountpoint *mp,
return -EINVAL;
newmnt->mnt.mnt_flags = mnt_flags;
+
+ newmnt->mnt.mnt_userns = path->mnt;
+ if (newmnt->mnt.mnt_userns != &init_user_ns)
+ newmnt->mnt.mnt_userns = get_user_ns(newmnt->mnt.mnt_userns);
+
return graft_tree(newmnt, parent, mp);
}
[1]: https://lore.kernel.org/linux-btrfs/20210727104900.829215-1-brauner@xxxxxxxxxx/T/#mca601363b435e81c89d8ca4f09134faa5c227e6d
