Re: [PATCH] RDS: Fix memory leak in rds_rdma_extra_size()

[Allison Henderson <achender@kernel.org>]

On Sun, 2026-04-12 at 20:44 +0800, Xiaobo Liu wrote:
> Free iov->iov when copy_from_user() or page count validation fails in rds_rdma_extra_size().
> 
> This preserves the existing success path and avoids leaking the allocated iovec array on error.

Hi Xiaobo,

Thanks for catching this.  The fix itself looks correct, but it will need your
Signed-off-by line.  Also be sure to note the target tree and subsystem in the subject
line like this "[PATCH net v2] net/rds: Fix memory leak in rds_rdma_extra_size()", and
make sure the commit message wraps at about 72 characters.  Other than that I think
the patch looks good.

Thank you!
Allison

> ---
>  net/rds/rdma.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/net/rds/rdma.c b/net/rds/rdma.c
> index aa6465dc7..91a20c1e2 100644
> --- a/net/rds/rdma.c
> +++ b/net/rds/rdma.c
> @@ -560,6 +560,7 @@ int rds_rdma_extra_size(struct rds_rdma_args *args,
>  	struct rds_iovec *vec;
>  	struct rds_iovec __user *local_vec;
>  	int tot_pages = 0;
> +	int ret = 0;
>  	unsigned int nr_pages;
>  	unsigned int i;
>  
> @@ -578,16 +579,20 @@ int rds_rdma_extra_size(struct rds_rdma_args *args,
>  	vec = &iov->iov[0];
>  
>  	if (copy_from_user(vec, local_vec, args->nr_local *
> -			   sizeof(struct rds_iovec)))
> -		return -EFAULT;
> +			   sizeof(struct rds_iovec))) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
>  	iov->len = args->nr_local;
>  
>  	/* figure out the number of pages in the vector */
>  	for (i = 0; i < args->nr_local; i++, vec++) {
>  
>  		nr_pages = rds_pages_in_vec(vec);
> -		if (nr_pages == 0)
> -			return -EINVAL;
> +		if (nr_pages == 0) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
>  
>  		tot_pages += nr_pages;
>  
> @@ -595,11 +600,20 @@ int rds_rdma_extra_size(struct rds_rdma_args *args,
>  		 * nr_pages for one entry is limited to (UINT_MAX>>PAGE_SHIFT)+1,
>  		 * so tot_pages cannot overflow without first going negative.
>  		 */
> -		if (tot_pages < 0)
> -			return -EINVAL;
> +		if (tot_pages < 0) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
>  	}
>  
> -	return tot_pages * sizeof(struct scatterlist);
> +	ret = tot_pages * sizeof(struct scatterlist);
> +
> +out:
> +	if (ret < 0) {
> +		kfree(iov->iov);
> +		iov->iov = NULL;
> +	}
> +	return ret;
>  }
>  
>  /*