net/rds/rdma.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-)
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.
Signed-off-by: Xiaobo Liu <cppcoffee@gmail.com>
---
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;
}
/*
--
2.34.1
On 4/13/26 9:00 AM, Xiaobo Liu wrote:
> @@ -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;
Is this really needed?!? AFAICS rds_rdma_extra_size() is invoked only
via: rds_sendmsg() -> rds_rm_size() -> rds_rdma_extra_size(), and the
rds_sendmsg() error path already frees any non NULL iov.
/P
On Mon, 2026-04-13 at 15:00 +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.
>
> Signed-off-by: Xiaobo Liu <cppcoffee@gmail.com>
I think this looks good now. Thanks Xiaobo.
Reviewed-by: Allison Henderson <achender@kernel.org>
> ---
> 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;
> }
>
> /*
© 2016 - 2026 Red Hat, Inc.