On Mon, Sep 28, 2026 at 9:22 AM Disha Goel <[email protected]> wrote:
>
> On 25/09/26 9:27 pm, Amir Goldstein wrote:
> > On Thu, Sep 24, 2026 at 1:29 PM Disha Goel <[email protected]> wrote:
> >>
> >> When overlayfs layers are passed as detached mount file descriptors
> >> (opened with open_tree(OPEN_TREE_CLONE)), ovl_parse_layer() uses d_path()
> >> to record the layer's path for display in mountinfo.
> >>
> >> d_path() walks up the mount tree to the process's filesystem root. For
> >> detached mounts this walk stops at the anonymous namespace root instead
> >> of the real system root, producing a short incorrect path like "/l1"
> >> instead of the full absolute path.
> >>
> >> Export mnt_is_anon() from fs/namespace.c to detect detached mounts. In
> >> ovl_parse_layer(), use dentry_path_raw() for detached mounts instead of
> >> d_path(). dentry_path_raw() walks the dentry chain independent of mount
> >> namespace and returns the correct fs-relative path. The path is
> >> display-only; the actual layer_path used for mounting is always correct.
> >>
> >> Also fix the set_layers_via_detached_mount_fds selftest: the mountinfo
> >> check strings were copy-pasted from the regular (non-detached) test and
> >> expected absolute /tmp/ paths, causing the test to always fail.
> >>
> >> Fixes: a08557d19ef4 ("ovl: specify layers via file descriptors")
> >> Cc: [email protected]
> >> Signed-off-by: Disha Goel <[email protected]>
> >
> > Disha,
> >
> > Thanks for the report!
> >
> >> ---
> >> fs/namespace.c | 9 ++++++++
> >> fs/overlayfs/params.c | 14 ++++++++++-
> >> include/linux/mount.h | 1 +
> >> .../overlayfs/set_layers_via_fds.c | 23 +++++++++++--------
> >> 4 files changed, 37 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/fs/namespace.c b/fs/namespace.c
> >> index ae5dc64f8b45..9feefa0a517d 100644
> >> --- a/fs/namespace.c
> >> +++ b/fs/namespace.c
> >> @@ -363,6 +363,15 @@ bool __mnt_is_readonly(const struct vfsmount *mnt)
> >> }
> >> EXPORT_SYMBOL_GPL(__mnt_is_readonly);
> >>
> >> +bool mnt_is_anon(struct vfsmount *mnt)
> >> +{
> >> + struct mount *m = real_mount(mnt);
> >> + struct mnt_namespace *ns = READ_ONCE(m->mnt_ns);
> >> +
> >> + return !IS_ERR_OR_NULL(ns) && is_anon_ns(ns);
> >> +}
> >> +EXPORT_SYMBOL_GPL(mnt_is_anon);
> >> +
> >> static inline void mnt_inc_writers(struct mount *mnt)
> >> {
> >> #ifdef CONFIG_SMP
> >> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> >> index c93fcaa45d4a..2758ff8426b7 100644
> >> --- a/fs/overlayfs/params.c
> >> +++ b/fs/overlayfs/params.c
> >> @@ -477,7 +477,19 @@ static int ovl_parse_layer(struct fs_context *fc,
> >> struct fs_parameter *param,
> >> layer_path = param->file->f_path;
> >> path_get(&layer_path);
> >>
> >> - layer_name = d_path(&layer_path, buf, PATH_MAX);
> >> + /*
> >> + * For detached mounts (open_tree(OPEN_TREE_CLONE)),
> >> d_path()
> >> + * resolves against the anonymous namespace root and
> >> returns a
> >> + * short fs-relative path rather than a full system path.
> >> Use
> >> + * dentry_path_raw() instead, which gives the path
> >> relative to
> >> + * the filesystem root regardless of mount namespace. The
> >> name
> >> + * is display-only; layer_path itself is always correct.
> >> + */
> >> + if (mnt_is_anon(layer_path.mnt))
> >> + layer_name = dentry_path_raw(layer_path.dentry,
> >> + buf, PATH_MAX);
> >> + else
> >> + layer_name = d_path(&layer_path, buf, PATH_MAX);
> >> if (IS_ERR(layer_name))
> >> return PTR_ERR(layer_name);
> >>
> >> diff --git a/include/linux/mount.h b/include/linux/mount.h
> >> index acfe7ef86a1b..51e9f228c9d1 100644
> >> --- a/include/linux/mount.h
> >> +++ b/include/linux/mount.h
> >> @@ -78,6 +78,7 @@ extern void mnt_make_shortterm(struct vfsmount *mnt);
> >> extern struct vfsmount *mnt_clone_internal(const struct path *path);
> >> extern bool __mnt_is_readonly(const struct vfsmount *mnt);
> >> extern bool mnt_may_suid(struct vfsmount *mnt);
> >> +extern bool mnt_is_anon(struct vfsmount *mnt);
> >>
> >> extern struct vfsmount *clone_private_mount(const struct path *path);
> >> int mnt_get_write_access(struct vfsmount *mnt);
> >> diff --git
> >> a/tools/testing/selftests/filesystems/overlayfs/set_layers_via_fds.c
> >> b/tools/testing/selftests/filesystems/overlayfs/set_layers_via_fds.c
> >> index 7a293544233d..12d930fe46be 100644
> >> --- a/tools/testing/selftests/filesystems/overlayfs/set_layers_via_fds.c
> >> +++ b/tools/testing/selftests/filesystems/overlayfs/set_layers_via_fds.c
> >> @@ -686,23 +686,28 @@ TEST_F(set_layers_via_fds,
> >> set_layers_via_detached_mount_fds)
> >> while (getline(&line, &len, f_mountinfo) != -1) {
> >> char *haystack = line;
> >>
> >> - if (strstr(haystack, "workdir=/tmp/w"))
> >> + /*
> >> + * Detached mount FDs are resolved via dentry_path_raw(),
> >> + * which gives a path relative to the underlying fs root
> >> + * (e.g. "/u/upper", "/l1") rather than a full system path.
> >> + */
> >> + if (strstr(haystack, "upperdir=/u/upper"))
> >> layers_found[0] = true;
> >> - if (strstr(haystack, "upperdir=/tmp/u"))
> >> + if (strstr(haystack, "workdir=/u/work"))
> >> layers_found[1] = true;
> >> - if (strstr(haystack, "lowerdir+=/tmp/l1"))
> >> + if (strstr(haystack, "lowerdir+=/l1"))
> >> layers_found[2] = true;
> >> - if (strstr(haystack, "lowerdir+=/tmp/l2"))
> >> + if (strstr(haystack, "lowerdir+=/l2"))
> >> layers_found[3] = true;
> >> - if (strstr(haystack, "lowerdir+=/tmp/l3"))
> >> + if (strstr(haystack, "lowerdir+=/l3"))
> >> layers_found[4] = true;
> >> - if (strstr(haystack, "lowerdir+=/tmp/l4"))
> >> + if (strstr(haystack, "lowerdir+=/l4"))
> >> layers_found[5] = true;
> >> - if (strstr(haystack, "datadir+=/tmp/d1"))
> >> + if (strstr(haystack, "datadir+=/d1"))
> >> layers_found[6] = true;
> >> - if (strstr(haystack, "datadir+=/tmp/d2"))
> >> + if (strstr(haystack, "datadir+=/d2"))
> >> layers_found[7] = true;
> >> - if (strstr(haystack, "datadir+=/tmp/d3"))
> >> + if (strstr(haystack, "datadir+=/d3"))
> >> layers_found[8] = true;
> >> }
> >> free(line);
> >> --
> >> 2.45.1
> >>
> >
> > Christian,
> >
> > I confirm that the test is failing on upstream:
> >
> > # Starting 1 tests from 1 test cases.
> > # RUN set_layers_via_fds.set_layers_via_detached_mount_fds ...
> > # set_layers_via_fds.c:717:set_layers_via_detached_mount_fds:Expected
> > layers_found[i] (0) == true (1)
> > # set_layers_via_fds.c:39:set_layers_via_detached_mount_fds:Expected
> > rmdir("/set_layers_via_fds") (-1) == 0 (0)
> > # set_layers_via_detached_mount_fds: Test terminated by assertion
> > # FAIL set_layers_via_fds.set_layers_via_detached_mount_fds
> > not ok 1 set_layers_via_fds.set_layers_via_detached_mount_fds
> > # FAILED: 0 / 1 tests passed.
> >
> > I could not find a point of regression.
> > Could it be that the test was merged failing?
> > That would be strange.
> >
> > This is how mountinfo of detached layers look like on upstream:
> > / /set_layers_via_fds rw,relatime - overlay none
> > rw,lowerdir+=/,lowerdir+=/,lowerdir+=/,lowerdir+=/,datadir+=/,datadir+=/,datadir+=/,upperdir=/upper,workdir=/work,uuid=on,metacopy=on
> >
>
> Hi Amir,
>
> Thanks for looking into this!
>
> On my ppc64le system (7.3.0-rc4) I saw dentry_path_raw() giving per-
> directory names like lowerdir+=/l1, upperdir=/u/upper etc., which is why
> I went with the kernel fix. But your observation of lowerdir+=/ for all
> layers is a valid outcome too depending on how d_path() terminates.
>
> You are right that the test strings (/tmp/l1 etc.) never matched
> detached mount output.
>
> > Whether the suggested solution is what we want for it, I am not sure.
> > I also don't recall if we discussed this at the time and whether
> > there were any decisions about how to present this mountinfo.
> >
> > Maybe this mountinfo is fine and then we only need to fix the test.
>
> Happy to send a v2 with just the selftest fix if that is the preferred
> direction.
>
Hi Disha,
Sorry for the delay, you caught us just before traveling to Plumbers.
I have applied a patch to my tree to fix just the test paths expectation.
w.r.t the regression of the override_cred feature and test case
set_override_creds_nomknod due to commit
c28f922c9dcee ("clone_private_mnt(): make sure that caller has
CAP_SYS_ADMIN in the right userns")
I am still not sure what's the best way to handle this yet.
Will wait for Christian to chime in, but I don't expect that to happen
during the upcoming merge window.
Thanks,
Amir.
