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

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

[Michael S. Tsirkin]

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.  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.

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

Fixes: 2170a1f09148 ("net: no longer reset transport_header in __netif_receive_skb_core()")
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
---
 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)) {
+		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

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

[Eric Dumazet]

Le sam. 24 oct. 2026 à 14:56, Michael S. Tsirkin <mst@redhat.com> a écrit :
>
> 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.

>  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.

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.

>
> 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.

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

> Fixes: 2170a1f09148 ("net: no longer reset transport_header in __netif_receive_skb_core()")
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Co-Authored-By: Claude Code <noreply@anthropic.com>
> ---
>  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.

> +               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
>

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

[Michael S. Tsirkin]

On Sun, Oct 25, 2026 at 09:18:14AM +0100, Eric Dumazet wrote:
> Le sam. 24 oct. 2026 à 14:56, Michael S. Tsirkin <mst@redhat.com> a écrit :
> >
> > 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.

> 
> >  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?


> >
> > 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
> 
> > Fixes: 2170a1f09148 ("net: no longer reset transport_header in __netif_receive_skb_core()")
> > Cc: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Co-Authored-By: Claude Code <noreply@anthropic.com>
> > ---
> >  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.
> 
> > +               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
> >

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete because the length helpers that read the
  same fields were not changed.
- [High] The new guards in skb_transport_header() and
  skb_inner_transport_header() only fire when the field equals exactly…
- [Low] skb_inner_transport_header() checks inner_transport_header
  against ~0U.

--

skbuff: keep accesses through unset transport header in bounds

This commit changes skb_transport_header() and skb_inner_transport_header()
so that they warn under CONFIG_DEBUG_NET when the transport header offset
holds the ~0 unset value. In that case they fall back to the network header
position. The aim is to keep accesses through an unset transport header
inside the packet instead of at skb->head + 0xffff.

> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6787fcf6ac3e4..e0e2f925f7299 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3002,6 +3002,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)) {

[Severity: Low]
This isn't a bug, but is ~0U the right unset value to test for the inner
transport header? The rest of the inner header code uses 0 to mean unset:

include/linux/skbuff.h:skb_inner_network_header_was_set() {
	return skb->inner_network_header > 0;
}

net/core/skbuff.c:skb_carve_reset_headers() {
	...
	/* Inner offsets have no "unset" marker, zero them so that
	 * skb_inner_network_header_was_set() becomes false and no
	 * consumer mistakes them for a real (and long gone) header.
	 */
	skb->inner_mac_header = 0;
	skb->inner_network_header = 0;
	skb->inner_transport_header = 0;
	...
}

The ~0U case can still happen, because skb_reset_inner_headers() copies an
unset outer transport_header into inner_transport_header. An inner
transport header that was never set (0) is not caught by this check,
though. It still returns skb->head. That pointer is in bounds, and the
behaviour is the same as before the patch.

> +		DEBUG_NET_WARN_ON_ONCE(1);
> +		return skb->head + skb->inner_network_header;
> +	}
> +
>  	return skb->head + skb->inner_transport_header;
>  }
>  
> @@ -3081,7 +3087,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))) {

[Severity: High]
Does the unset value always survive until this check? The new test only
matches 0xffff exactly. skb_headers_offset_update() shifts both transport
offsets unconditionally, while it does guard mac_header:

net/core/skbuff.c:skb_headers_offset_update() {
	...
	skb->transport_header += off;
	skb->network_header   += off;
	if (skb_mac_header_was_set(skb))
		skb->mac_header += off;
	skb->inner_transport_header += off;
	...
}

pskb_expand_head() calls it with a positive nhead, so 0xffff wraps to
nhead - 1. skb_copy_expand() calls it with newheadroom - oldheadroom. That
value is negative when the new headroom is smaller, which gives 0xffff - k.

In both cases skb_transport_header_was_set() returns true, so no warning
fires. In the second case skb_transport_header() still returns a pointer
about 64K past skb->head.

One RX path that looks like it can reach this is vl600_rx_fixup() in
drivers/net/usb/lg-vl600.c:

	s->current_rx_buf = skb_copy_expand(skb, 0,
			le32_to_cpup(&frame->len), GFP_ATOMIC);

That buffer is later passed up with usbnet_skb_return(). Because was_set()
now returns true, __netif_receive_skb_core() also skips its reset:

	if (!skb_transport_header_was_set(skb))
		skb_reset_transport_header(skb);

Should skb_headers_offset_update() leave unset transport offsets alone, as
it already does for mac_header?

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

[Severity: High]
Should the length helpers that read the same fields use the same fallback?
skb_transport_offset() picks it up through skb_transport_header(), but
skb_network_header_len() and skb_inner_network_header_len() still use the
raw offsets:

include/linux/skbuff.h:
static inline u32 skb_network_header_len(const struct sk_buff *skb)
{
	DEBUG_NET_WARN_ON_ONCE(!skb_transport_header_was_set(skb));
	return skb->transport_header - skb->network_header;
}

static inline u32 skb_inner_network_header_len(const struct sk_buff *skb)
{
	return skb->inner_transport_header - skb->inner_network_header;
}

With an unset header, both still return about 0xffff minus the network
offset. skb_inner_network_header_len() has no check at all. Callers that
use these values for pulls, copies or offset arithmetic would still get a
length of about 64K.

Code that reads skb->transport_header directly, such as
virtio_net_hdr_tnl_from_skb(), also bypasses the fallback.

Does this match the commit message's "accesses stay within the packet"?

There is also a new mismatch. Before this patch,
skb_transport_header() - skb_network_header() always equalled
skb_network_header_len(). With an unset header they now differ.
ipv6_mc_may_pull() mixes the two:

include/net/addrconf.h:ipv6_mc_may_pull() {
	if (skb_transport_offset(skb) + ipv6_transport_len(skb) < len)
		return false;

	return pskb_may_pull(skb, len);
}

include/linux/ipv6.h:ipv6_transport_len() {
	return ntohs(ipv6_hdr(skb)->payload_len) + sizeof(struct ipv6hdr) -
	       skb_network_header_len(skb);
}

Before the patch the raw offsets cancelled out. Now the first term is
clamped to the network offset, but ipv6_transport_len() still subtracts the
raw ~64K value.

Can the unsigned sum wrap here and skip the advertised length check,
leaving pskb_may_pull() as the only bound?

> +
>  	return skb->head + skb->transport_header;
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/0bdcbe2ab5ffc86ba7190fe25476bcebcb967310.1792844270.git.mst%40redhat.com