[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251020075441.127980-1-daniel@iogearbox.net>
Date: Mon, 20 Oct 2025 09:54:41 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: martin.lau@...ux.dev
Cc: kuba@...nel.org,
bpf@...r.kernel.org,
netdev@...r.kernel.org,
Yinhao Hu <dddddd@...t.edu.cn>,
Kaiyan Mei <M202472210@...t.edu.cn>,
Dongliang Mu <dzm91@...t.edu.cn>,
Willem de Bruijn <willemb@...gle.com>,
Stanislav Fomichev <sdf@...ichev.me>
Subject: [PATCH bpf] bpf: Do not let BPF test infra emit invalid GSO types to stack
Yinhao et al. reported that their fuzzer tool was able to trigger a
skb_warn_bad_offload() from netif_skb_features() -> gso_features_check().
When a BPF program - triggered via BPF test infra - pushes the packet
to the loopback device via bpf_clone_redirect() then mentioned offload
warning can be seen. GSO-related features are then rightfully disabled.
We get into this situation due to convert___skb_to_skb() setting
gso_segs and gso_size but not gso_type. Technically, it makes sense
that this warning triggers since the GSO properties are malformed due
to the gso_type. Potentially, the gso_type could be marked non-trustworthy
through setting it at least to SKB_GSO_DODGY without any other specific
assumptions, but that also feels wrong given we should not go further
into the GSO engine in the first place.
The checks were added in 121d57af308d ("gso: validate gso_type in GSO
handlers") because there were malicious (syzbot) senders that combine
a protocol with a non-matching gso_type. If we would want to drop such
packets, gso_features_check() currently only returns feature flags via
netif_skb_features(), so one location for potentially dropping such skbs
could be validate_xmit_unreadable_skb(), but then otoh it would be
an additional check in the fast-path for a very corner case. Given
bpf_clone_redirect() is the only place where BPF test infra could emit
such packets, lets reject them right there.
Fixes: 850a88cc4096 ("bpf: Expose __sk_buff wire_len/gso_segs to BPF_PROG_TEST_RUN")
Fixes: cf62089b0edd ("bpf: Add gso_size to __sk_buff")
Reported-by: Yinhao Hu <dddddd@...t.edu.cn>
Reported-by: Kaiyan Mei <M202472210@...t.edu.cn>
Reported-by: Dongliang Mu <dzm91@...t.edu.cn>
Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
Cc: Willem de Bruijn <willemb@...gle.com>
Cc: Stanislav Fomichev <sdf@...ichev.me>
---
[ bpf-next would be fine as well imho since its mainly about muting
the skb_warn_bad_offload warning. The Fixes tags are mainly for
reference / historic context. ]
net/bpf/test_run.c | 5 +++++
net/core/filter.c | 7 +++++++
2 files changed, 12 insertions(+)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 1782e83de2cb..983b9ee1164b 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -950,6 +950,11 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
if (__skb->gso_segs > GSO_MAX_SEGS)
return -EINVAL;
+
+ /* Currently GSO type is zero/unset. If this gets extended with
+ * a small list of accepted GSO types in future, the filter for
+ * an unset GSO type in bpf_clone_redirect() can be lifted.
+ */
skb_shinfo(skb)->gso_segs = __skb->gso_segs;
skb_shinfo(skb)->gso_size = __skb->gso_size;
skb_shinfo(skb)->hwtstamps.hwtstamp = __skb->hwtstamp;
diff --git a/net/core/filter.c b/net/core/filter.c
index 76628df1fc82..9d67a34a6650 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2458,6 +2458,13 @@ BPF_CALL_3(bpf_clone_redirect, struct sk_buff *, skb, u32, ifindex, u64, flags)
if (unlikely(flags & (~(BPF_F_INGRESS) | BPF_F_REDIRECT_INTERNAL)))
return -EINVAL;
+ /* BPF test infra's convert___skb_to_skb() can create type-less
+ * GSO packets. gso_features_check() will detect this as a bad
+ * offload. However, lets not leak them out in the first place.
+ */
+ if (unlikely(skb_is_gso(skb) && !skb_shinfo(skb)->gso_type))
+ return -EBADMSG;
+
dev = dev_get_by_index_rcu(dev_net(skb->dev), ifindex);
if (unlikely(!dev))
return -EINVAL;
--
2.43.0
Powered by blists - more mailing lists