1. Patchew
  2. linux
  3. View series

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

Andrei Vagin posted 1 patch 3 hours ago 
fs/namespace.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
[PATCH] fs/namespace: correctly handle errors returned by grab_requested_mnt_ns
Posted by Andrei Vagin 3 hours ago 
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>
---
 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
Re: [PATCH] fs/namespace: correctly handle errors returned by grab_requested_mnt_ns
Posted by Christian Brauner 44 minutes ago 
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
>
Re: [PATCH] fs/namespace: correctly handle errors returned by grab_requested_mnt_ns
Posted by Jan Kara an hour ago 
On Tue 11-11-25 06:28:15, 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>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  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
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.