Re: [PATCH] fs/namespace: correctly handle errors returned by grab_requested_mnt_ns

Christian Brauner
2025-11-12T09:42:38Z

On Tue, Nov 11, 2025 at 08:20:46AM -0800, Andrei Vagin wrote:
> )
>
> On Tue, Nov 11, 2025 at 1:13 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Tue, Nov 11, 2025 at 06:28:15AM +0000, Andrei Vagin wrote:
> > > grab_requested_mnt_ns was changed to return error codes on failure, but
> > > its callers were not updated to check for error pointers, still checking
> > > only for a NULL return value.
> > >
> > > This commit updates the callers to use IS_ERR() or IS_ERR_OR_NULL() and
> > > PTR_ERR() to correctly check for and propagate errors.
> > >
> > > Fixes: 7b9d14af8777 ("fs: allow mount namespace fd")
> > > Cc: Christian Brauner <brauner@kernel.org>
> > > Signed-off-by: Andrei Vagin <avagin@google.com>
> > > ---
> >
> > Thanks. I've folded the following diff into the patch to be more in line
> > with our usual error handling:
>
> The diff looks good, thanks. I have another question regarding
> 7b9d14af8777 ("fs: allow mount namespace fd"). My understanding is that
> the intention was to allow using mount namespace file descriptors
> (req->spare) for the statmount and listmounts syscalls. If this is
> correct and I haven't missed anything, we need to make one more change:
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 9124465dca556..3250cadde6fc4 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -5738,7 +5738,7 @@ static int copy_mnt_id_req(const struct
> mnt_id_req __user *req,
> ret = copy_struct_from_user(kreq, sizeof(*kreq), req, usize);
> if (ret)
> return ret;
> - if (kreq->spare != 0)
> + if (kreq->spare != 0 && kreq->mnt_ns_id != 0)
> return -EINVAL;

Yeah, that's right. I'm going to rename the field to mnt_ns_fd as
well... Thanks!

diff --git a/fs/namespace.c b/fs/namespace.c
index 76f6e868f352..2bad25709b2c 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -5727,7 +5727,7 @@ static int copy_mnt_id_req(const struct mnt_id_req __user *req,
ret = copy_struct_from_user(kreq, sizeof(*kreq), req, usize);
if (ret)
return ret;
- if (kreq->spare != 0)
+ if (kreq->mnt_ns_fd != 0 && kreq->mnt_ns_id)
return -EINVAL;
/* The first valid unique mount id is MNT_UNIQUE_ID_OFFSET + 1. */
if (kreq->mnt_id <= MNT_UNIQUE_ID_OFFSET)
@@ -5744,15 +5744,12 @@ static struct mnt_namespace *grab_requested_mnt_ns(const struct mnt_id_req *kreq
{
struct mnt_namespace *mnt_ns;

- if (kreq->mnt_ns_id && kreq->spare)
- return ERR_PTR(-EINVAL);
-
if (kreq->mnt_ns_id) {
mnt_ns = lookup_mnt_ns(kreq->mnt_ns_id);
- } else if (kreq->spare) {
+ } else if (kreq->mnt_ns_fd) {
struct ns_common *ns;

- CLASS(fd, f)(kreq->spare);
+ CLASS(fd, f)(kreq->mnt_ns_fd);
if (fd_empty(f))
return ERR_PTR(-EBADF);

diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h
index 7fa67c2031a5..5d3f8c9e3a62 100644
--- a/include/uapi/linux/mount.h
+++ b/include/uapi/linux/mount.h
@@ -197,7 +197,7 @@ struct statmount {
*/
struct mnt_id_req {
__u32 size;
- __u32 spare;
+ __u32 mnt_ns_fd;
__u64 mnt_id;
__u64 param;
__u64 mnt_ns_id;