Re: [PATCH net] skbuff: keep accesses through unset transport header in bounds

On Sun, Oct 25, 2026 at 09:18:14AM +0100, Eric Dumazet wrote:

Le sam. 24 oct. 2026 à 14:56, Michael S. Tsirkin [off-list ref] a écrit :

quoted

Commit 2170a1f09148 ("net: no longer reset transport_header in
__netif_receive_skb_core()") stopped the receive path from pretending
that an unset transport header starts at the network header - but only
on CONFIG_DEBUG_NET=y builds, the idea being that fuzzers and CI could
catch stacks reading a transport header where none was ever established.
And the plan is to drop this for release as well.

Indeed, under KASAN, an access through the unset offset produces a
slab-out-of-bounds splat, which is how these bugs are usually found.
But a regular debug build has no such safety net: the accesses land at
skb->head + 0xffff, some 64K past the packet data.

I don't think release builds are exposed to oob in this way, but I want
to make the stack a bit more robust if yes.

This is not a net candidate: there is no bug in release builds, since
__netif_receive_skb_core() still resets the transport header when
CONFIG_DEBUG_NET is not set. The Fixes: tag points to a commit doing
exactly what it was meant to do.

Good point.

quoted

 While I appreciate a nice
KASAN splat as much as the next developer, making even a debug kernel do
oob accesses was never the point of the exercise.

Make skb_transport_header(), and its inner variant, verify and WARN,
then fall back to the network header position when the transport header
was not set. This way, accesses stay within the packet.

This adds a test and a conditional branch in a very hot helper
(tcp_hdr(), udp_hdr(), skb_transport_offset()...), inlined in hundreds
of places, for all builds, to protect CONFIG_DEBUG_NET=y builds only.

Good point. I'll put this in ifdef CONFIG_DEBUG_NET then?

Falling back to the network header is also what 2170a1f09148 wanted
to stop doing: pretending transport and network headers are the same
is wrong, and silently hides the bug after the first WARN_ON_ONCE().
KASAN would no longer report anything.

But the bug already manifested with a warning, why do we want
also the kasan splat much later? And why do we want memory
silently corrupted on non KASAN builds with CONFIG_DEBUG_NET?

quoted

An extra benefit is that release and debug kernels now behave
identically if an unset transport header is ever reached,
avoids heisenbugs.

Not really, the fallback path is only reachable on CONFIG_DEBUG_NET=y
builds.

Yes, this is what I referred to as the heisenbug: release behaves one
way, debug another, so e.g. if there's a bug only with header set,
it will not reproduce. I guess it's not a concern then.

If you have an actual report (syzbot, KASAN, or a stack trace from a
debug build), please share it, and we will fix the caller that uses
an unset transport header. That is the whole point of the debug check.

pw-bot: rejected

quoted

Fixes: 2170a1f09148 ("net: no longer reset transport_header in __netif_receive_skb_core()")
Cc: Eric Dumazet <redacted>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Co-Authored-By: Claude Code [off-list ref]
---
 include/linux/skbuff.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a3ff380d43f0..cbfeb17d86f7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h

@@ -2993,6 +2993,12 @@ static inline void skb_reset_mac_len(struct sk_buff *skb)
 static inline unsigned char *skb_inner_transport_header(const struct sk_buff
                                                        *skb)
 {
+       if (unlikely(skb->inner_transport_header ==
+                    (typeof(skb->inner_transport_header))~0U)) {

Inner offsets do not use ~0 as an 'unset' marker, they are zeroed.

quoted

+               DEBUG_NET_WARN_ON_ONCE(1);
+               return skb->head + skb->inner_network_header;
+       }
+
        return skb->head + skb->inner_transport_header;
 }

@@ -3072,7 +3078,11 @@ static inline void skb_unset_transport_header(struct sk_buff *skb)

 static inline unsigned char *skb_transport_header(const struct sk_buff *skb)
 {
-       DEBUG_NET_WARN_ON_ONCE(!skb_transport_header_was_set(skb));
+       if (unlikely(!skb_transport_header_was_set(skb))) {
+               DEBUG_NET_WARN_ON_ONCE(1);
+               return skb->head + skb->network_header;
+       }
+
        return skb->head + skb->transport_header;
 }

--

MST