Re: [PATCH 2/2] ovl: fix wrong layer paths in mountinfo for detached mount FDs

[Amir Goldstein]

On Mon, Sep 28, 2026 at 9:22 AM Disha Goel <disgoel@xxxxxxxxxxxxx> wrote:
>
> On 25/09/26 9:27 pm, Amir Goldstein wrote:
> > On Thu, Sep 24, 2026 at 1:29 PM Disha Goel <disgoel@xxxxxxxxxxxxx> 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: stable@xxxxxxxxxxxxxxx
> >> Signed-off-by: Disha Goel <disgoel@xxxxxxxxxxxxx>
> >
> > 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.