linux-kernel - Re: [PATCH] fs/namespace: correctly handle errors returned by grab_requested_mnt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251112-gelitten-sanduhr-17a9d8b97ec8@brauner>
Date: Wed, 12 Nov 2025 10:42:34 +0100
From: Christian Brauner <brauner@...nel.org>
To: Andrei Vagin <avagin@...gle.com>
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>
Subject: Re: [PATCH] fs/namespace: correctly handle errors returned by
 grab_requested_mnt_ns

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@...nel.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@...nel.org>
> > > Signed-off-by: Andrei Vagin <avagin@...gle.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;