Re: [BUG] net: rds: rds_send_probe memory leak

[Greg KH <gregkh@linuxfoundation.org>]

On Sun, Mar 14, 2021 at 03:19:05PM +0300, Fatih Yildirim wrote:
> On Sun, 2021-03-14 at 09:36 +0100, Greg KH wrote:
> > On Sun, Mar 14, 2021 at 11:23:10AM +0300, Fatih Yildirim wrote:
> > > Hi Santosh,
> > > 
> > > I've been working on a memory leak bug reported by syzbot.
> > > https://syzkaller.appspot.com/bug?id=39b72114839a6dbd66c1d2104522698a813f9ae2
> > > 
> > > It seems that memory allocated in rds_send_probe function is not
> > > freed.
> > > 
> > > Let me share my observations.
> > > rds_message is allocated at the beginning of rds_send_probe
> > > function.
> > > Then it is added to cp_send_queue list of rds_conn_path and
> > > refcount
> > > is increased by one.
> > > Next, in rds_send_xmit function it is moved from cp_send_queue list
> > > to
> > > cp_retrans list, and again refcount is increased by one.
> > > Finally in rds_loop_xmit function refcount is increased by one.
> > > So, total refcount is 4.
> > > However, rds_message_put is called three times, in rds_send_probe,
> > > rds_send_remove_from_sock and rds_send_xmit functions. It seems
> > > that
> > > one more rds_message_put is needed.
> > > Would you please check and share your comments on this issue?
> > 
> > Do you have a proposed patch that syzbot can test to verify if this
> > is
> > correct or not?
> > 
> > thanks,
> > 
> > gre gk-h
> 
> Hi Greg,
> 
> Actually, using the .config and the C reproducer, syzbot reports the
> memory leak in rds_send_probe function. Also by enabling
> CONFIG_RDS_DEBUG=y, the debug messages indicates the similar as I
> mentioned above. To give an example, below is the RDS_DEBUG messages.
> Allocated address 000000008a7476e5 has initial ref_count 1. Then there
> are three rds_message_addref calls for the same address making the
> refcount 4, but only three rds_message_put calls which leave the
> address still allocated.
> 
> [   60.570681] rds_message_addref(): addref rm 000000008a7476e5 ref 1
> [   60.570707] rds_message_put(): put rm 000000008a7476e5 ref 2
> [   60.570845] rds_message_addref(): addref rm 000000008a7476e5 ref 1
> [   60.570870] rds_message_addref(): addref rm 000000008a7476e5 ref 2
> [   60.570960] rds_message_put(): put rm 000000008a7476e5 ref 3
> [   60.570995] rds_message_put(): put rm 000000008a7476e5 ref 2
> 

Ok, so the next step is to try your proposed change to see if it works
or not.  What prevents you from doign that?

No need to ask people if your analysis of an issue is true or not, no
maintainer or developer usually has the time to deal with that.  We much
rather would like to see patches of things you have tested to resolve
issues.

thanks,

greg k-h