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

[Disha Goel <disgoel@linux.ibm.com>]

On 25/09/26 9:27 pm, Amir Goldstein wrote:
> On Thu, Sep 24, 2026 at 1:29 PM Disha Goel <disgoel@linux.ibm.com> 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@vger.kernel.org
>> Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
> 
> 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.

> Thanks,
> Amir.

-- 
Regards,
Disha