On Tue, Oct 27, 2020 at 11:21 AM Cong Wang <xiyou.wangc...@gmail.com> wrote:
>
> On Mon, Oct 26, 2020 at 3:46 AM Tung Nguyen
> <tung.q.ngu...@dektech.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.
More importantly, prior to Xin Long's change of skb_unshare(),
skb_unclone() was used, which does not touch the skb refcnt either.
So, why does it rely on skb_unshare() to release this refcnt now?
Thanks.
