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

[Jan Kara]

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@xxxxxxxxxx>
> Signed-off-by: Andrei Vagin <avagin@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								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@xxxxxxxx>
SUSE Labs, CR