d_path() results in presence of detached mounts

d_path() results in presence of detached mounts

[Jan Kara]

Hello!

Recently I've got a question from a user about the following:

# unshare --mount swapon /dev/sda3
# cat /proc/swaps
Filename                                Type            Size            Used            Priority
/sda3                                   partition       2098152         0               -2

Now everything works as expected here AFAICS. When namespace gets created
/dev mount is cloned into it. When swapon exits, the namespace is
destroyed and /dev mount clone is detached (no parent, namespace NULL).
Hence when d_path() crawls the path it stops at the mountpoint root and
exits. There's not much we can do about this but when discussing the
situation internally, Michal proposed that d_path() could append something
like "(detached)" to the path string - similarly to "(deleted)". That could
somewhat reduce the confusion about such paths? What do people think about
this?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: d_path() results in presence of detached mounts

[Christian Brauner]

On Mon, Apr 07, 2025 at 06:00:07PM +0200, Jan Kara wrote:
> Hello!
> 
> Recently I've got a question from a user about the following:
> 
> # unshare --mount swapon /dev/sda3
> # cat /proc/swaps
> Filename                                Type            Size            Used            Priority
> /sda3                                   partition       2098152         0               -2
> 
> Now everything works as expected here AFAICS. When namespace gets created
> /dev mount is cloned into it. When swapon exits, the namespace is
> destroyed and /dev mount clone is detached (no parent, namespace NULL).
> Hence when d_path() crawls the path it stops at the mountpoint root and
> exits. There's not much we can do about this but when discussing the
> situation internally, Michal proposed that d_path() could append something
> like "(detached)" to the path string - similarly to "(deleted)". That could
> somewhat reduce the confusion about such paths? What do people think about
> this?

You can get into this situation in plenty of other ways. For example by
using detached mount via open_tree(OPEN_TREE_CLONE) as layers in
overlayfs. Or simply:

        int fd;
        char dpath[PATH_MAX];

        fd = open_tree(-EBADF, "/var/lib/fwupd", OPEN_TREE_CLONE);
        dup2(fd, 500);
        close(fd);
        readlink("/proc/self/fd/500", dpath, sizeof(dpath));
        printf("dpath = %s\n", dpath);

Showing "(detached)" will be ambiguous just like "(deleted)" is. If that
doesn't matter and it's clearly documented then it's probably fine. But
note that this will also affect /proc/<pid>/fd/ as can be seen from the
above example.

int main(int argc, char *argv[])
{
        int fd;
        char dpath[PATH_MAX];
        char *dirs[] = { "/ONE", "TWO", "THREE", "FOUR", NULL };

        for (char **dir = dirs; *dir; dir++) {
                mkdir(*dir, 0777);
                chdir(*dir);
        }

        chdir("/");
        fd = open_tree(-EBADF, "/ONE/TWO/THREE/FOUR", OPEN_TREE_CLONE);
        if (fd < 0) {
                perror("open_tree");
                _exit(1);
        }

        rmdir("/ONE/TWO/THREE/FOUR");

        if (dup2(fd, 500) < 0) {
                perror("dup2");
                _exit(1);
        }
        close(fd);

        readlink("/proc/self/fd/500", dpath, sizeof(dpath));
        if (strcmp("/ (detached) (deleted)", dpath) != 0) {
                printf("wrong dpath = %s\n", dpath);
                _exit(1);
        }
        printf("dpath = %s\n", dpath);
        _exit(0);
}

user1@localhost:~/data/scripts$ sudo ./open_tree_detached
dpath = / (detached) (deleted)

Seems good to me.

Should be as simple as:

diff --git a/fs/d_path.c b/fs/d_path.c
index 5f4da5c8d5db..58874a107634 100644
--- a/fs/d_path.c
+++ b/fs/d_path.c
@@ -246,6 +246,12 @@ static void get_fs_root_rcu(struct fs_struct *fs, struct path *root)
        } while (read_seqcount_retry(&fs->seq, seq));
 }

+static inline bool is_detached(struct mount *mnt)
+{
+       struct mnt_namespace *mnt_ns = READ_ONCE(mnt->mnt_ns);
+       return unlikely(!mnt_ns || is_anon_ns(mnt_ns));
+}
+
 /**
  * d_path - return the path of a dentry
  * @path: path to report
@@ -284,7 +290,11 @@ char *d_path(const struct path *path, char *buf, int buflen)

        rcu_read_lock();
        get_fs_root_rcu(current->fs, &root);
-       if (unlikely(d_unlinked(path->dentry)))
+       if (unlikely(is_detached(real_mount(path->mnt))) && d_unlinked(path->dentry))
+               prepend(&b, " (detached) (deleted)", 22);
+       else if (unlikely(is_detached(real_mount(path->mnt))))
+               prepend(&b, " (detached)", 11);
+       else if (unlikely(d_unlinked(path->dentry)))
                prepend(&b, " (deleted)", 11);
        else
                prepend_char(&b, 0);

This is racy. Iow, after the first check it's still possible that it's
both detached and deleted. But I don't think that matters. (deleted)
must stay at the end because there's userspace out there that expects
(deleted) to be at the end.

Re: d_path() results in presence of detached mounts

[Christian Brauner]

On Tue, Apr 08, 2025 at 10:55:07AM +0200, Christian Brauner wrote:
> On Mon, Apr 07, 2025 at 06:00:07PM +0200, Jan Kara wrote:
> > Hello!
> > 
> > Recently I've got a question from a user about the following:
> > 
> > # unshare --mount swapon /dev/sda3
> > # cat /proc/swaps
> > Filename                                Type            Size            Used            Priority
> > /sda3                                   partition       2098152         0               -2
> > 
> > Now everything works as expected here AFAICS. When namespace gets created
> > /dev mount is cloned into it. When swapon exits, the namespace is
> > destroyed and /dev mount clone is detached (no parent, namespace NULL).

That's not the issue you're seeing here though

> > Hence when d_path() crawls the path it stops at the mountpoint root and
> > exits. There's not much we can do about this but when discussing the
> > situation internally, Michal proposed that d_path() could append something
> > like "(detached)" to the path string - similarly to "(deleted)". That could
> > somewhat reduce the confusion about such paths? What do people think about
> > this?
> 
> You can get into this situation in plenty of other ways. For example by
> using detached mount via open_tree(OPEN_TREE_CLONE) as layers in
> overlayfs. Or simply:
> 
>         int fd;
>         char dpath[PATH_MAX];
> 
>         fd = open_tree(-EBADF, "/var/lib/fwupd", OPEN_TREE_CLONE);
>         dup2(fd, 500);
>         close(fd);
>         readlink("/proc/self/fd/500", dpath, sizeof(dpath));
>         printf("dpath = %s\n", dpath);
> 
> Showing "(detached)" will be ambiguous just like "(deleted)" is. If that
> doesn't matter and it's clearly documented then it's probably fine. But
> note that this will also affect /proc/<pid>/fd/ as can be seen from the
> above example.

The other downside is that it will still be quite opaque because the
user will have to be aware of the concept of a detached mount. So it's
mostly useful for administrators tbh.

In general, I think it needs to be made clear to userspace that paths
shown in such tables are open()-able in the best case and decorative or
even misleading in the worst case.

The swap case is particularly ugly because it's a kernel-internal open
so in essence you might be located in a namespace where you can't even
even possibly reach that path.

In the

	unshare --mount swapon /dev/sda3
	exit

example you're lucky. If you're on the host then you can reasonably
guess and go "Oh, this is probably a fscked path we could look at /dev"
and parse through /dev/ to find "sda3" but that's a risky game even in
that case.

If you're in a container however that /proc/swaps output is completely
useless as there surely (unless you have a very broken container) won't
be a devtmpfs mount in there at all so that device isn't reachable at
all. Same for files.

In short, that output is purely diagnostic at best.

Re: d_path() results in presence of detached mounts

[Jan Kara]

On Tue 08-04-25 13:39:28, Christian Brauner wrote:
> On Tue, Apr 08, 2025 at 10:55:07AM +0200, Christian Brauner wrote:
> > On Mon, Apr 07, 2025 at 06:00:07PM +0200, Jan Kara wrote:
> > > Hello!
> > > 
> > > Recently I've got a question from a user about the following:
> > > 
> > > # unshare --mount swapon /dev/sda3
> > > # cat /proc/swaps
> > > Filename                                Type            Size            Used            Priority
> > > /sda3                                   partition       2098152         0               -2
> > > 
> > > Now everything works as expected here AFAICS. When namespace gets created
> > > /dev mount is cloned into it. When swapon exits, the namespace is
> > > destroyed and /dev mount clone is detached (no parent, namespace NULL).
> 
> That's not the issue you're seeing here though
> 
> > > Hence when d_path() crawls the path it stops at the mountpoint root and
> > > exits. There's not much we can do about this but when discussing the
> > > situation internally, Michal proposed that d_path() could append something
> > > like "(detached)" to the path string - similarly to "(deleted)". That could
> > > somewhat reduce the confusion about such paths? What do people think about
> > > this?
> > 
> > You can get into this situation in plenty of other ways. For example by
> > using detached mount via open_tree(OPEN_TREE_CLONE) as layers in
> > overlayfs. Or simply:
> > 
> >         int fd;
> >         char dpath[PATH_MAX];
> > 
> >         fd = open_tree(-EBADF, "/var/lib/fwupd", OPEN_TREE_CLONE);
> >         dup2(fd, 500);
> >         close(fd);
> >         readlink("/proc/self/fd/500", dpath, sizeof(dpath));
> >         printf("dpath = %s\n", dpath);
> > 
> > Showing "(detached)" will be ambiguous just like "(deleted)" is. If that
> > doesn't matter and it's clearly documented then it's probably fine. But
> > note that this will also affect /proc/<pid>/fd/ as can be seen from the
> > above example.
> 
> The other downside is that it will still be quite opaque because the
> user will have to be aware of the concept of a detached mount. So it's
> mostly useful for administrators tbh.

Thanks for the insights!

> In general, I think it needs to be made clear to userspace that paths
> shown in such tables are open()-able in the best case and decorative or
> even misleading in the worst case.

Yes, I know this and I was just wondering if we can at least somehow
visibly indicate the path shown is likely unusable. If you think it would
do more harm than good, I'm fine with that answer, I just thought I'll
ask...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: d_path() results in presence of detached mounts

[Christian Brauner]

On Tue, Apr 08, 2025 at 02:04:48PM +0200, Jan Kara wrote:
> On Tue 08-04-25 13:39:28, Christian Brauner wrote:
> > On Tue, Apr 08, 2025 at 10:55:07AM +0200, Christian Brauner wrote:
> > > On Mon, Apr 07, 2025 at 06:00:07PM +0200, Jan Kara wrote:
> > > > Hello!
> > > > 
> > > > Recently I've got a question from a user about the following:
> > > > 
> > > > # unshare --mount swapon /dev/sda3
> > > > # cat /proc/swaps
> > > > Filename                                Type            Size            Used            Priority
> > > > /sda3                                   partition       2098152         0               -2
> > > > 
> > > > Now everything works as expected here AFAICS. When namespace gets created
> > > > /dev mount is cloned into it. When swapon exits, the namespace is
> > > > destroyed and /dev mount clone is detached (no parent, namespace NULL).
> > 
> > That's not the issue you're seeing here though

Uh, sorry about this weird stray sentence. Not sure how that ended up in
here. I think that was supposed to be in another mail.

> > 
> > > > Hence when d_path() crawls the path it stops at the mountpoint root and
> > > > exits. There's not much we can do about this but when discussing the
> > > > situation internally, Michal proposed that d_path() could append something
> > > > like "(detached)" to the path string - similarly to "(deleted)". That could
> > > > somewhat reduce the confusion about such paths? What do people think about
> > > > this?
> > > 
> > > You can get into this situation in plenty of other ways. For example by
> > > using detached mount via open_tree(OPEN_TREE_CLONE) as layers in
> > > overlayfs. Or simply:
> > > 
> > >         int fd;
> > >         char dpath[PATH_MAX];
> > > 
> > >         fd = open_tree(-EBADF, "/var/lib/fwupd", OPEN_TREE_CLONE);
> > >         dup2(fd, 500);
> > >         close(fd);
> > >         readlink("/proc/self/fd/500", dpath, sizeof(dpath));
> > >         printf("dpath = %s\n", dpath);
> > > 
> > > Showing "(detached)" will be ambiguous just like "(deleted)" is. If that
> > > doesn't matter and it's clearly documented then it's probably fine. But
> > > note that this will also affect /proc/<pid>/fd/ as can be seen from the
> > > above example.
> > 
> > The other downside is that it will still be quite opaque because the
> > user will have to be aware of the concept of a detached mount. So it's
> > mostly useful for administrators tbh.
> 
> Thanks for the insights!
> 
> > In general, I think it needs to be made clear to userspace that paths
> > shown in such tables are open()-able in the best case and decorative or
> > even misleading in the worst case.
> 
> Yes, I know this and I was just wondering if we can at least somehow
> visibly indicate the path shown is likely unusable. If you think it would
> do more harm than good, I'm fine with that answer, I just thought I'll
> ask...

Oh yes, absolutely. It's good to bring this up. I just wonder whether
we could do better. Karel suggested using a stx_attribute and I had
pondered a statmount() extension but both have some issues though let me
pitch them nonetheless somewhat independent of the issue.

We could add a statmount() extension to mark detached mounts as being
detached. That however will only work for anonymous detached mounts,
i.e., mounts that have been created using OPEN_TREE_CLONE or fsmount().
Since statmount() uses the mount namespace rbtree to lookup mounts any
unmounted mount with mnt->mnt_ns == NULL cannot be found anymore. So
it's - currently at least - not useful to handle the /proc/swaps case.

A statx() extension would be more useful because it would work
independent of whether this is a detached mount or not. So we would
start reporting STATX_MNT_DETACHED in stx_attributes which allows
userspace to figure out that the thing was unmounted. That might be
genuinely useful.

But both solution wouldn't help with the /proc/swaps scenario because
it's not guaranteed that the file can be opened because it might be
outside the caller's mount namespace (or even deleted).