[BUG] net: rds: rds_send_probe memory leak

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?

Thanks,
Fatih

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

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:

quoted

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

Thanks,
Fatih

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:

quoted

On Sun, Mar 14, 2021 at 11:23:10AM +0300, Fatih Yildirim wrote:

quoted

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

On Sun, 2021-03-14 at 13:44 +0100, Greg KH wrote:

On Sun, Mar 14, 2021 at 03:19:05PM +0300, Fatih Yildirim wrote:

quoted

On Sun, 2021-03-14 at 09:36 +0100, Greg KH wrote:

quoted

On Sun, Mar 14, 2021 at 11:23:10AM +0300, Fatih Yildirim wrote:

quoted

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

Hi Greg,

I also would like to come with a patch to resolve the issue as well.
But couldn't figure out so far. I just would like to have a review or a
suggestion from an expert in order to move forward.
Anyway, I'm still working on it and hope to find a solution.
Will appreciate any comment, suggestion on the issue.

Thanks,
Fatih