lkml.org 
[lkml]   [2022]   [Aug]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] can: j1939: fix memory leak of skbs
From
Hi Oleksij,

On 29.07.2022 07:22, Oleksij Rempel wrote:
> Initial issue can be reproduced by using real (slow) CAN with j1939cat[1]
> tool. Both parts should be started to make sure the j1939_session_tx_dat() will
> actually start using the queue. After pushing about 100K of data, application
> will try to close the socket and exit. After socket is closed, all skb related
> to this socket will be freed and j1939_session_tx_dat() will use freed skbs.

Ok, the patch I suggested was a kind of a guess, now I understand that
it breaks important logic.

On 29.07.2022 07:22, Oleksij Rempel wrote:
> This skb_get() is counter part of skb_unref()
> j1939_session_skb_drop_old().

However, we have a case [1] where j1939_session_skb_queue() is called
but the corresponding j1939_session_skb_drop_old() is not called and it
causes a memory leak.

I tried to investigate it a little bit: the thing is that
j1939_session_skb_drop_old() can be called only when processing
J1939_ETP_CMD_CTS. On the contrary, as I can see,
j1939_session_skb_queue() can be called independently from
J1939_ETP_CMD_CTS so the two functions obviously do not correspond to
each other.

In reproducer case there is a J1939_ETP_CMD_RTS processing, then
we send some messages (where j1939_session_skb_queue() is called) and
after that J1939_ETP_CMD_ABORT is processed and we lose those skbs.

[1] https://forge.ispras.ru/issues/11743

\
 
 \ /
  Last update: 2022-08-05 10:57    [W:0.061 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site