From: Cong Wang <xiyou.wangcong@gmail.com>
To: Tung Nguyen <tung.q.nguyen@dektech.com.au>
Cc: David Miller <davem@davemloft.net>,
Linux Kernel Network Developers <netdev@vger.kernel.org>,
tipc-discussion@lists.sourceforge.net
Subject: Re: [tipc-discussion] [net v2 1/1] tipc: fix memory leak caused by tipc_buf_append()
Date: Tue, 27 Oct 2020 11:21:05 -0700 [thread overview]
Message-ID: <CAM_iQpXnsiGP_x-D5YEWbVmqzP2ZhRdtG1ReDQq2wr6YUs2J0w@mail.gmail.com> (raw)
In-Reply-To: <20201026104333.13008-1-tung.q.nguyen@dektech.com.au>
On Mon, Oct 26, 2020 at 3:46 AM Tung Nguyen
<tung.q.nguyen@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.
Can you be more specific here?
Thanks.
next prev parent reply other threads:[~2020-10-27 18:25 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-26 10:43 [tipc-discussion] [net v2 1/1] tipc: fix memory leak caused by tipc_buf_append() Tung Nguyen
2020-10-26 18:54 ` Jakub Kicinski
2020-10-27 18:21 ` Cong Wang [this message]
2020-10-27 18:49 ` Cong Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAM_iQpXnsiGP_x-D5YEWbVmqzP2ZhRdtG1ReDQq2wr6YUs2J0w@mail.gmail.com \
--to=xiyou.wangcong@gmail.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=tipc-discussion@lists.sourceforge.net \
--cc=tung.q.nguyen@dektech.com.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).