[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <662cfd6db06df_28b9852949a@willemb.c.googlers.com.notmuch>
Date: Sat, 27 Apr 2024 09:28:13 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Daniel Borkmann <daniel@...earbox.net>,
Lena Wang (王娜) <Lena.Wang@...iatek.com>,
"maze@...gle.com" <maze@...gle.com>,
"willemdebruijn.kernel@...il.com" <willemdebruijn.kernel@...il.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"bpf@...r.kernel.org" <bpf@...r.kernel.org>,
"steffen.klassert@...unet.com" <steffen.klassert@...unet.com>,
"kuba@...nel.org" <kuba@...nel.org>,
Shiming Cheng (成诗明) <Shiming.Cheng@...iatek.com>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"yan@...udflare.com" <yan@...udflare.com>
Subject: Re: [PATCH net] udp: fix segmentation crash for GRO packet without
fraglist
Daniel Borkmann wrote:
> On 4/26/24 11:52 AM, Lena Wang (王娜) wrote:
> [...]
> >>> From 301da5c9d65652bac6091d4cd64b751b3338f8bb Mon Sep 17 00:00:00
> >> 2001
> >>> From: Shiming Cheng <shiming.cheng@...iatek.com>
> >>> Date: Wed, 24 Apr 2024 13:42:35 +0800
> >>> Subject: [PATCH net] net: prevent BPF pulling SKB_GSO_FRAGLIST skb
> >>>
> >>> A SKB_GSO_FRAGLIST skb can't be pulled data
> >>> from its fraglist as it may result an invalid
> >>> segmentation or kernel exception.
> >>>
> >>> For such structured skb we limit the BPF pulling
> >>> data length smaller than skb_headlen() and return
> >>> error if exceeding.
> >>>
> >>> Fixes: 3a1296a38d0c ("net: Support GRO/GSO fraglist chaining.")
> >>> Signed-off-by: Shiming Cheng <shiming.cheng@...iatek.com>
> >>> Signed-off-by: Lena Wang <lena.wang@...iatek.com>
> >>> ---
> >>> net/core/filter.c | 5 +++++
> >>> 1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/net/core/filter.c b/net/core/filter.c
> >>> index 8adf95765cdd..8ed4d5d87167 100644
> >>> --- a/net/core/filter.c
> >>> +++ b/net/core/filter.c
> >>> @@ -1662,6 +1662,11 @@ static DEFINE_PER_CPU(struct bpf_scratchpad,
> >>> bpf_sp);
> >>> static inline int __bpf_try_make_writable(struct sk_buff *skb,
> >>> unsigned int write_len)
> >>> {
> >>> +if (skb_is_gso(skb) &&
> >>> + (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) &&
> >>> + write_len > skb_headlen(skb)) {
> >>> +return -ENOMEM;
> >>> +}
> >>> return skb_ensure_writable(skb, write_len);
>
> Dumb question, but should this guard be more generically part of skb_ensure_writable()
> internals, presumably that would be inside pskb_may_pull_reason(), or only if we ever
> see more code instances similar to this?
Good point. Most callers of skb_ensure_writable correctly pull only
headers, so wouldn't cause this problem. But it also adds coverage to
things like tc pedit.
Powered by blists - more mailing lists