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

[Allison Henderson]

On Thu, 2026-04-16 at 18:00 +0800, Xiaobo Liu wrote:
> The internal addition of kfree and setting the pointer to NULL in
> rds_rdma_extra_size makes the function more self‑consistent and secure.
> After applying this patch, kfree(NULL) in rds_sendmsg is also safe and will
> not cause a double‑free.

Hi Xiaobo,                                                                               
                  
Paolo makes a good point that I had missed in that rds_sendmsg owns the
cleanup. So even though iov->iov isn't freed here, it isn't leaked
either. Self-consistency is fair as a style point, but it's not
strong enough to justify the change on its own since it isn't a bug
fix. That said, thank you for taking the time to look at this area;
we appreciate the effort to help track down and fix bugs.

Thanks,         
Allison

> 
> On 4/16/2616:20 Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
> > 
> > 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