From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: <ptyadav@amazon.de>
Cc: <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
<kuniyu@amazon.com>, <linux-kernel@vger.kernel.org>,
<netdev@vger.kernel.org>, <nmanthey@amazon.de>,
<pabeni@redhat.com>, <willemb@google.com>
Subject: Re: [PATCH net] net: fix skb leak in __skb_tstamp_tx()
Date: Mon, 22 May 2023 08:45:54 -0700 [thread overview]
Message-ID: <20230522154554.44836-1-kuniyu@amazon.com> (raw)
In-Reply-To: <20230522153020.32422-1-ptyadav@amazon.de>
From: Pratyush Yadav <ptyadav@amazon.de>
Date: Mon, 22 May 2023 17:30:20 +0200
> Commit 50749f2dd685 ("tcp/udp: Fix memleaks of sk and zerocopy skbs with
> TX timestamp.") added a call to skb_orphan_frags_rx() to fix leaks with
> zerocopy skbs. But it ended up adding a leak of its own. When
> skb_orphan_frags_rx() fails, the function just returns, leaking the skb
> it just cloned. Free it before returning.
>
> This bug was discovered and resolved using Coverity Static Analysis
> Security Testing (SAST) by Synopsys, Inc.
>
> Fixes: 50749f2dd685 ("tcp/udp: Fix memleaks of sk and zerocopy skbs with TX timestamp.")
> Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Good catch, thanks!
> ---
>
> I do not know this code very well, this was caught by our static
> analysis tool. I did not try specifically reproducing the leak but I did
> do a boot test by adding this patch on 6.4-rc3 and the kernel boots
> fine.
>
> net/core/skbuff.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 515ec5cdc79c..cea28d30abb5 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -5224,8 +5224,10 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb,
> } else {
> skb = skb_clone(orig_skb, GFP_ATOMIC);
>
> - if (skb_orphan_frags_rx(skb, GFP_ATOMIC))
> + if (skb_orphan_frags_rx(skb, GFP_ATOMIC)) {
> + kfree_skb(skb);
> return;
> + }
> }
> if (!skb)
> return;
> --
> 2.39.2
next prev parent reply other threads:[~2023-05-22 15:46 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-22 15:30 [PATCH net] net: fix skb leak in __skb_tstamp_tx() Pratyush Yadav
2023-05-22 15:45 ` Kuniyuki Iwashima [this message]
2023-05-22 16:11 ` Willem de Bruijn
2023-05-22 16:55 ` SeongJae Park
2023-05-22 17:03 ` Pratyush Yadav
2023-05-22 17:08 ` Greg KH
2023-05-22 17:04 ` Kuniyuki Iwashima
2023-05-22 17:18 ` SeongJae Park
2023-05-22 17:23 ` SeongJae Park
2023-05-22 17:33 ` SeongJae Park
2023-05-22 18:57 ` Eric Dumazet
2023-05-24 4:20 ` patchwork-bot+netdevbpf
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=20230522154554.44836-1-kuniyu@amazon.com \
--to=kuniyu@amazon.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nmanthey@amazon.de \
--cc=pabeni@redhat.com \
--cc=ptyadav@amazon.de \
--cc=willemb@google.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.