Hi Tung,

Okie, got it. I lost track when your patch was sent in August... ☹.
Anyway, I think you can consider my commit message which highlights this as a 
memory leak issue seriously.

BR/Tuong

-----Original Message-----
From: tung quang nguyen <tung.q.ngu...@dektech.com.au> 
Sent: Monday, November 25, 2019 3:23 PM
To: 'Tuong Lien' <tuong.t.l...@dektech.com.au>; 
tipc-discussion@lists.sourceforge.net; jon.ma...@ericsson.com; 
ma...@donjonn.com; ying....@windriver.com
Subject: RE: [tipc-discussion] [net] tipc: fix memory leak in socket streaming

Hi Tuong,

I fixed it in this commit "[tipc-discussion] [net v1 2/3] tipc: fix wrong
socket reference counter after tipc_sk_timeout() returns" but I have not
sent to David yet.
I intended to send it in a series of patch for fixing bugs at socket layer.

Thanks.

Best regards,
Tung Nguyen

-----Original Message-----
From: Tuong Lien <tuong.t.l...@dektech.com.au> 
Sent: Monday, November 25, 2019 3:13 PM
To: tipc-discussion@lists.sourceforge.net; jon.ma...@ericsson.com;
ma...@donjonn.com; ying....@windriver.com
Subject: [tipc-discussion] [net] tipc: fix memory leak in socket streaming

In case of stream sockets, a per-socket timer is set up for either the
SYN attempt or connection supervision mechanism. When the socket timer
expires, an appropriate action (i.e. sending SYN or PROBE message)
would be taken just in the case the socket is not being owned by user
(e.g. via the 'lock_sock()').

In the latter case, there is nothing but the timer is re-scheduled for
a short period of time (~ 50ms) to try again. The function just makes a
'return' immediately without decreasing the socket 'refcnt' which was
held in advance for the timer callback itself! The same happens if at
the next time, the socket is still busy...

As a result, the socket 'refcnt' is increased without decreasing, so
the sock object cannot be freed at all (due to 'refcnt' != 0) even when
the connection is closed and user releases all related resources.

The memory leak is hard to detect because the probe interval is set to
1 hour since the connection is established, but in the case of a SYN
attempt, that can be much more likely.

The commit fixes the bug by calling the 'sock_put()' in the case
mentioned above, then the socket 'refcnt' will be increased & decreased
correctly and the sock object can be released later.

Fixes: 0d5fcebf3c37 ("tipc: refactor tipc_sk_timeout() function")
Signed-off-by: Tuong Lien <tuong.t.l...@dektech.com.au>
---
 net/tipc/socket.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index a1c8d722ca20..d67c3747d2c3 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2757,7 +2757,7 @@ static void tipc_sk_timeout(struct timer_list *t)
        if (sock_owned_by_user(sk)) {
                sk_reset_timer(sk, &sk->sk_timer, jiffies + HZ / 20);
                bh_unlock_sock(sk);
-               return;
+               goto exit;
        }
 
        if (sk->sk_state == TIPC_ESTABLISHED)
@@ -2775,6 +2775,8 @@ static void tipc_sk_timeout(struct timer_list *t)
                tipc_dest_push(&tsk->cong_links, pnode, 0);
                tsk->cong_link_cnt = 1;
        }
+
+exit:
        sock_put(sk);
 }
 
-- 
2.13.7



_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion




_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Reply via email to