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

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

Hi Christian,

Looking for your suggestion on this patch series before we come to a
conclusion or send a v2.

On 28/09/26 12:52 pm, Disha Goel wrote:
> 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