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

Christian Brauner
2025-11-11T09:13:19Z

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:

diff --git a/fs/namespace.c b/fs/namespace.c
index 74a162a5703a..76f6e868f352 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -134,16 +134,15 @@ __cacheline_aligned_in_smp DEFINE_SEQLOCK(mount_lock);

static void mnt_ns_release(struct mnt_namespace *ns)
{
- if (IS_ERR_OR_NULL(ns))
- return;
/* keep alive for {list,stat}mount() */
- if (refcount_dec_and_test(&ns->passive)) {
+ if (ns && refcount_dec_and_test(&ns->passive)) {
fsnotify_mntns_delete(ns);
put_user_ns(ns->user_ns);
kfree(ns);
}
}
-DEFINE_FREE(mnt_ns_release, struct mnt_namespace *, if (_T) mnt_ns_release(_T))
+DEFINE_FREE(mnt_ns_release, struct mnt_namespace *,
+ if (!IS_ERR(_T)) mnt_ns_release(_T))

static void mnt_ns_release_rcu(struct rcu_head *rcu)
{
@@ -5750,10 +5749,7 @@ static struct mnt_namespace *grab_requested_mnt_ns(const struct mnt_id_req *kreq

if (kreq->mnt_ns_id) {
mnt_ns = lookup_mnt_ns(kreq->mnt_ns_id);
- return mnt_ns ? : ERR_PTR(-ENOENT);
- }
-
- if (kreq->spare) {
+ } else if (kreq->spare) {
struct ns_common *ns;

CLASS(fd, f)(kreq->spare);
@@ -5771,6 +5767,8 @@ static struct mnt_namespace *grab_requested_mnt_ns(const struct mnt_id_req *kreq
} else {
mnt_ns = current->nsproxy->mnt_ns;
}
+ if (!mnt_ns)
+ return ERR_PTR(-ENOENT);

refcount_inc(&mnt_ns->passive);
return mnt_ns;

> fs/namespace.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index d82910f33dc4..9124465dca55 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -144,8 +144,10 @@ static inline struct mnt_namespace *node_to_mnt_ns(const struct rb_node *node)
>
> static void mnt_ns_release(struct mnt_namespace *ns)
> {
> + if (IS_ERR_OR_NULL(ns))
> + return;
> /* keep alive for {list,stat}mount() */
> - if (ns && refcount_dec_and_test(&ns->passive)) {
> + if (refcount_dec_and_test(&ns->passive)) {
> fsnotify_mntns_delete(ns);
> put_user_ns(ns->user_ns);
> kfree(ns);
> @@ -5756,8 +5758,10 @@ static struct mnt_namespace *grab_requested_mnt_ns(const struct mnt_id_req *kreq
> if (kreq->mnt_ns_id && kreq->spare)
> return ERR_PTR(-EINVAL);
>
> - if (kreq->mnt_ns_id)
> - return lookup_mnt_ns(kreq->mnt_ns_id);
> + if (kreq->mnt_ns_id) {
> + mnt_ns = lookup_mnt_ns(kreq->mnt_ns_id);
> + return mnt_ns ? : ERR_PTR(-ENOENT);
> + }
>
> if (kreq->spare) {
> struct ns_common *ns;
> @@ -5801,8 +5805,8 @@ SYSCALL_DEFINE4(statmount, const struct mnt_id_req __user *, req,
> return ret;
>
> ns = grab_requested_mnt_ns(&kreq);
> - if (!ns)
> - return -ENOENT;
> + if (IS_ERR(ns))
> + return PTR_ERR(ns);
>
> if (kreq.mnt_ns_id && (ns != current->nsproxy->mnt_ns) &&
> !ns_capable_noaudit(ns->user_ns, CAP_SYS_ADMIN))
> @@ -5912,8 +5916,8 @@ static void __free_klistmount_free(const struct klistmount *kls)
> static inline int prepare_klistmount(struct klistmount *kls, struct mnt_id_req *kreq,
> size_t nr_mnt_ids)
> {
> -
> u64 last_mnt_id = kreq->param;
> + struct mnt_namespace *ns;
>
> /* The first valid unique mount id is MNT_UNIQUE_ID_OFFSET + 1. */
> if (last_mnt_id != 0 && last_mnt_id <= MNT_UNIQUE_ID_OFFSET)
> @@ -5927,9 +5931,10 @@ static inline int prepare_klistmount(struct klistmount *kls, struct mnt_id_req *
> if (!kls->kmnt_ids)
> return -ENOMEM;
>
> - kls->ns = grab_requested_mnt_ns(kreq);
> - if (!kls->ns)
> - return -ENOENT;
> + ns = grab_requested_mnt_ns(kreq);
> + if (IS_ERR(ns))
> + return PTR_ERR(ns);
> + kls->ns = ns;
>
> kls->mnt_parent_id = kreq->mnt_id;
> return 0;
> --
> 2.51.2.1041.gc1ab5b90ca-goog
>