Re: [PATCH 0/8] Support foreign mount namespace with statmount/listmount

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2024-06-24 at 11:49 -0400, Josef Bacik wrote:
> Hello,
> 
> Currently the only way to iterate over mount entries in mount namespaces that
> aren't your own is to trawl through /proc in order to find /proc/$PID/mountinfo
> for the mount namespace that you want.  This is hugely inefficient, so extend
> both statmount() and listmount() to allow specifying a mount namespace id in
> order to get to mounts in other mount namespaces.
> 
> There are a few components to this
> 
> 1. Having a global index of the mount namespace based on the ->seq value in the
>    mount namespace.  This gives us a unique identifier that isn't re-used.
> 2. Support looking up mount namespaces based on that unique identifier, and
>    validating the user has permission to access the given mount namespace.
> 3. Provide a new ioctl() on nsfs in order to extract the unique identifier we
>    can use for statmount() and listmount().
> 
> The code is relatively straightforward, and there is a selftest provided to
> validate everything works properly.
> 
> This is based on vfs.all as of last week, so must be applied onto a tree that
> has Christians error handling rework in this area.  If you wish you can pull the
> tree directly here
> 
> https://github.com/josefbacik/linux/tree/listmount.combined
> 
> Christian and I collaborated on this series, which is why there's patches from
> both of us in this series.
> 
> Josef
> 
> Christian Brauner (4):
>   fs: relax permissions for listmount()
>   fs: relax permissions for statmount()
>   fs: Allow listmount() in foreign mount namespace
>   fs: Allow statmount() in foreign mount namespace
> 
> Josef Bacik (4):
>   fs: keep an index of current mount namespaces
>   fs: export the mount ns id via statmount
>   fs: add an ioctl to get the mnt ns id from nsfs
>   selftests: add a test for the foreign mnt ns extensions
> 
>  fs/mount.h                                    |   2 +
>  fs/namespace.c                                | 240 ++++++++++--
>  fs/nsfs.c                                     |  14 +
>  include/uapi/linux/mount.h                    |   6 +-
>  include/uapi/linux/nsfs.h                     |   2 +
>  .../selftests/filesystems/statmount/Makefile  |   2 +-
>  .../filesystems/statmount/statmount.h         |  46 +++
>  .../filesystems/statmount/statmount_test.c    |  53 +--
>  .../filesystems/statmount/statmount_test_ns.c | 360 ++++++++++++++++++
>  9 files changed, 659 insertions(+), 66 deletions(-)
>  create mode 100644 tools/testing/selftests/filesystems/statmount/statmount.h
>  create mode 100644 tools/testing/selftests/filesystems/statmount/statmount_test_ns.c
> 


Nice work! I had a minor question about the locking, but this all looks
pretty straightfoward.

As a side question. Is there any progress on adding proper glibc
bindings for the new syscalls? We'll want to make sure they incorporate
this change, if that's being done.

Extending listmount() and statmount() via struct mnt_id_req turns out
to be pretty painless. Kudos to whoever designed that part of the
original interfaces!

Reviewed-by: Jeff Layton <jlayton@kernel.org>