[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAM_iQpXnsiGP_x-D5YEWbVmqzP2ZhRdtG1ReDQq2wr6YUs2J0w@mail.gmail.com>
Date: Tue, 27 Oct 2020 11:21:05 -0700
From: Cong Wang <xiyou.wangcong@...il.com>
To: Tung Nguyen <tung.q.nguyen@...tech.com.au>
Cc: David Miller <davem@...emloft.net>,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
tipc-discussion@...ts.sourceforge.net
Subject: Re: [tipc-discussion] [net v2 1/1] tipc: fix memory leak caused by tipc_buf_append()
On Mon, Oct 26, 2020 at 3:46 AM Tung Nguyen
<tung.q.nguyen@...tech.com.au> wrote:
>
> Commit ed42989eab57 ("fix the skb_unshare() in tipc_buf_append()")
> replaced skb_unshare() with skb_copy() to not reduce the data reference
> counter of the original skb intentionally. This is not the correct
> way to handle the cloned skb because it causes memory leak in 2
> following cases:
> 1/ Sending multicast messages via broadcast link
> The original skb list is cloned to the local skb list for local
> destination. After that, the data reference counter of each skb
> in the original list has the value of 2. This causes each skb not
> to be freed after receiving ACK:
Interesting, I can not immediately see how tipc_link_advance_transmq()
clones the skb. You point out how it is freed but not cloned.
It looks really odd to see the skb is held by some caller, then expected
to be released by the unshare in tipc_buf_append(). IMHO, the refcnt
should be released where it is held.
Can you be more specific here?
Thanks.
Powered by blists - more mailing lists