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

[Andrei Vagin <avagin@google.com>]

)

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;
        /* The first valid unique mount id is MNT_UNIQUE_ID_OFFSET + 1. */
        if (kreq->mnt_id <= MNT_UNIQUE_ID_OFFSET)
@@ -5755,9 +5755,6 @@ 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);
                return mnt_ns ? : ERR_PTR(-ENOENT);

Thanks,
Andrei