Re: [RFC][PATCH] btrfs_get_tree_subvol(): switch from fc_mount() to vfs_create_mount()

Al Viro
2025-05-06T18:16:06Z

On Tue, May 06, 2025 at 07:54:32PM +0200, Klara Modin wrote:

> > > Though now I hit another issue which I don't know if it's related or
> > > not. I'm using an overlay mount with squashfs as lower and btrfs as
> > > upper. The mount fails with invalid argument and I see this in the log:
> > >
> > > overlayfs: failed to clone upperpath
> >
> > Seeing that you already have a kernel with that thing reverted, could
> > you check if the problem exists there?
>
> Yeah, it works fine with the revert instead.

Interesting... That message means that you've got clone_private_mount()
returning an error; the thing is, mount passed to it has come from
pathname lookup - it is *not* the mount created by that fc_mount() of
vfs_create_mount() in the modified code. That one gets passed to
mount_subvol() and consumed there (by mount_subtree()). All that is returned
is root dentry; the mount passed to clone_private_mount() is created
from scratch using dentry left by btrfs_get_tree_subvol() in its fc->root -
see
dentry = mount_subvol(ctx->subvol_name, ctx->subvol_objectid, mnt);
ctx->subvol_name = NULL;
if (IS_ERR(dentry))
return PTR_ERR(dentry);

fc->root = dentry;
return 0;
in the end of btrfs_get_tree_subvol().

What's more, on the overlayfs side we managed to get to
upper_mnt = clone_private_mount(upperpath);
err = PTR_ERR(upper_mnt);
if (IS_ERR(upper_mnt)) {
pr_err("failed to clone upperpath\n");
goto out;
so the upper path had been resolved...

OK, let's try to see what clone_private_mount() is unhappy about...
Could you try the following on top of -next + braino fix and see
what shows up? Another interesting thing, assuming you can get
to shell after overlayfs mount failure, would be /proc/self/mountinfo
contents and stat(1) output for upper path of your overlayfs mount...

diff --git a/fs/namespace.c b/fs/namespace.c
index eb990e9a668a..f7ce53f437dd 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2480,31 +2480,45 @@ struct vfsmount *clone_private_mount(const struct path *path)

guard(rwsem_read)(&namespace_sem);

- if (IS_MNT_UNBINDABLE(old_mnt))
+ if (IS_MNT_UNBINDABLE(old_mnt)) {
+ pr_err("unbindable");
return ERR_PTR(-EINVAL);
+ }

if (mnt_has_parent(old_mnt)) {
- if (!check_mnt(old_mnt))
+ if (!check_mnt(old_mnt)) {
+ pr_err("mounted, but not in our namespace");
return ERR_PTR(-EINVAL);
+ }
} else {
- if (!is_mounted(&old_mnt->mnt))
+ if (!is_mounted(&old_mnt->mnt)) {
+ pr_err("not mounted");
return ERR_PTR(-EINVAL);
+ }

/* Make sure this isn't something purely kernel internal. */
- if (!is_anon_ns(old_mnt->mnt_ns))
+ if (!is_anon_ns(old_mnt->mnt_ns)) {
+ pr_err("kern_mount?");
return ERR_PTR(-EINVAL);
+ }

/* Make sure we don't create mount namespace loops. */
- if (!check_for_nsfs_mounts(old_mnt))
+ if (!check_for_nsfs_mounts(old_mnt)) {
+ pr_err("shite with nsfs");
return ERR_PTR(-EINVAL);
+ }
}

- if (has_locked_children(old_mnt, path->dentry))
+ if (has_locked_children(old_mnt, path->dentry)) {
+ pr_err("has locked children");
return ERR_PTR(-EINVAL);
+ }

new_mnt = clone_mnt(old_mnt, path->dentry, CL_PRIVATE);
- if (IS_ERR(new_mnt))
+ if (IS_ERR(new_mnt)) {
+ pr_err("clone_mnt failed (%ld)", PTR_ERR(new_mnt));
return ERR_PTR(-EINVAL);
+ }

/* Longterm mount to be removed by kern_unmount*() */
new_mnt->mnt_ns = MNT_NS_INTERNAL;