* [PATCH net v2] net: skbuff: fix pull-bound underflow in skb_checksum_setup_ipv6()
@ 2026-09-19 13:36 Shihuang Liu
2026-09-22 12:37 ` netdev-bot+sashiko
2026-09-24 0:50 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Shihuang Liu @ 2026-09-19 13:36 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, kuba, pabeni, horms, Shihuang Liu
skb_maybe_pull_tail() subtracts skb_headlen(skb) from the unsigned max
argument and passes the result to __pskb_pull_tail() as a signed int. The
function does not ensure that max is at least skb_headlen(skb).
This can happen while parsing IPv6 extension headers when an skb already
has a linear area larger than MAX_IPV6_HDR_LEN. Once the parser needs data
beyond the linear area, max - skb_headlen(skb) wraps and is converted to a
negative delta. __pskb_pull_tail() then passes that negative length to
skb_copy_bits(), where it can become a very large copy length.
Pass the requested length itself as the pull bound at the three
extension-header call sites, so the delta can no longer go negative.
Fixes: 1431fb31ecba ("xen-netback: fix fragment detection in checksum setup")
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Shihuang Liu <shlomojune6@gmail.com>
---
Changes in v2:
- fix the three extension-header call sites in
skb_checksum_setup_ipv6() to pass the requested length as the pull
bound, keeping the helper's fast path untouched.
v1:
https://lore.kernel.org/r/20260823142620.126424-1-shlomojune6@gmail.com/
net/core/skbuff.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index cc3b4b70288b4..a2d7b2fbc006d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5977,7 +5977,8 @@ static int skb_checksum_setup_ipv6(struct sk_buff *skb, bool recalculate)
err = skb_maybe_pull_tail(skb,
off +
sizeof(struct ipv6_opt_hdr),
- MAX_IPV6_HDR_LEN);
+ off +
+ sizeof(struct ipv6_opt_hdr));
if (err < 0)
goto out;
@@ -5992,7 +5993,8 @@ static int skb_checksum_setup_ipv6(struct sk_buff *skb, bool recalculate)
err = skb_maybe_pull_tail(skb,
off +
sizeof(struct ip_auth_hdr),
- MAX_IPV6_HDR_LEN);
+ off +
+ sizeof(struct ip_auth_hdr));
if (err < 0)
goto out;
@@ -6007,7 +6009,8 @@ static int skb_checksum_setup_ipv6(struct sk_buff *skb, bool recalculate)
err = skb_maybe_pull_tail(skb,
off +
sizeof(struct frag_hdr),
- MAX_IPV6_HDR_LEN);
+ off +
+ sizeof(struct frag_hdr));
if (err < 0)
goto out;
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net v2] net: skbuff: fix pull-bound underflow in skb_checksum_setup_ipv6()
2026-09-19 13:36 [PATCH net v2] net: skbuff: fix pull-bound underflow in skb_checksum_setup_ipv6() Shihuang Liu
@ 2026-09-22 12:37 ` netdev-bot+sashiko
2026-09-24 0:50 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-09-22 12:37 UTC (permalink / raw)
To: shlomojune6; +Cc: netdev, davem, edumazet, kuba, pabeni, horms
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 1 · Low: 0
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] net/core/skbuff.c, skb_checksum_setup_ipv6(): replacing
MAX_IPV6_HDR_LEN with the per-header requested length (max == len) at…
- [Medium] Because the patch lets skb_checksum_setup_ipv6() walk an
arbitrarily long extension-header chain in a non-linear skb, `off` can…
--
net: skbuff: fix pull-bound underflow in skb_checksum_setup_ipv6()
The patch changes the three IPv6 extension-header call sites in
skb_checksum_setup_ipv6() so that the max argument passed to
skb_maybe_pull_tail() is the requested length instead of
MAX_IPV6_HDR_LEN, which prevents max - skb_headlen(skb) from wrapping
negative.
v1: https://lore.kernel.org/r/20260823142620.126424-1-shlomojune6@gmail.com/
A couple of questions about the behaviour change this introduces.
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 609f2c7f4a47..b4edbd06655e 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -5977,7 +5977,8 @@ static int skb_checksum_setup_ipv6(struct sk_buff *skb, bool recalculate)
> err = skb_maybe_pull_tail(skb,
> off +
> sizeof(struct ipv6_opt_hdr),
> - MAX_IPV6_HDR_LEN);
> + off +
> + sizeof(struct ipv6_opt_hdr));
> if (err < 0)
> goto out;
[Severity: High]
Does passing max == len here also remove the 256-byte parse bound for
non-linear skbs?
Before the change, on an skb whose linear area is smaller than
MAX_IPV6_HDR_LEN, once off + sizeof(hdr) grew past 256 the helper
clamped max:
net/core/skbuff.c:skb_maybe_pull_tail() {
if (max > skb->len)
max = skb->len;
if (__pskb_pull_tail(skb, max - skb_headlen(skb)) == NULL)
return -ENOMEM;
if (skb_headlen(skb) < len)
return -EPROTO;
}
so skb_headlen(skb) stayed at 256, the len test failed, and the frame
was dropped with -EPROTO after exactly one bounded pull.
With max == len the same frame is now accepted and the walker keeps
pulling instead:
while (off <= len && !done) {
where len = sizeof(struct ipv6hdr) + ntohs(ipv6_hdr(skb)->payload_len),
and off advances by as little as 8 bytes per iteration because
ipv6_optlen(hp) and ipv6_authlen(hp) are derived from the peer-supplied
hp->hdrlen. For a 64 KiB frame built out of ~8000 eight-byte DSTOPTS or
HOPOPTS headers, is that not up to ~8K __pskb_pull_tail() calls per
packet, each walking the frag array, with repeated
pskb_expand_head(skb, 0, eat + 128, GFP_ATOMIC) reallocations growing
the head towards ~64 KiB in softirq context?
The path looks reachable from untrusted input: xen-netback
checksum_setup() -> skb_checksum_setup() -> skb_checksum_setup_ipv6(),
where xenvif_tx_build_gops() builds exactly this shape of skb:
drivers/net/xen-netback/netback.c:xenvif_tx_build_gops() {
data_len = (txreq.size > XEN_NETBACK_TX_COPY_LEN) ?
XEN_NETBACK_TX_COPY_LEN : txreq.size;
}
i.e. a 128-byte linear head with the remainder in frags. xen-netfront
handle_incoming_queue() -> checksum_setup() has the same shape with
pull_to capped at RX_COPY_THRESHOLD (256).
Would keeping the bound and rejecting the chain instead fix the
underflow without changing what the parser accepts, for example by
returning -EPROTO when skb_headlen(skb) >= max, or by leaving
max = MAX_IPV6_HDR_LEN and refusing chains that reach beyond it?
Two related comments now disagree with the code. MAX_IPV6_HDR_LEN still
says:
/* This value should be large enough to cover a tagged ethernet header plus
* an IPv6 header, all options, and a maximal TCP or UDP header.
*/
#define MAX_IPV6_HDR_LEN 256
but after this change the constant only bounds the 40-byte base-header
pull, and no code enforces the "all options" part. Likewise
skb_maybe_pull_tail() still says:
/* If we need to pullup then pullup to the max, so we
* won't need to do it again.
*/
which no longer holds for these three callers, since each iteration
pulls only the next header.
The v2 changelog says the change keeps "the helper's fast path
untouched", and the patch carries a Fixes: tag so it is stable
material. Could the changelog spell out that extension-header chains
longer than 256 bytes in paged data change from dropped to accepted?
> @@ -5992,7 +5993,8 @@ static int skb_checksum_setup_ipv6(struct sk_buff *skb, bool recalculate)
> err = skb_maybe_pull_tail(skb,
> off +
> sizeof(struct ip_auth_hdr),
> - MAX_IPV6_HDR_LEN);
> + off +
> + sizeof(struct ip_auth_hdr));
> if (err < 0)
> goto out;
>
> @@ -6007,7 +6009,8 @@ static int skb_checksum_setup_ipv6(struct sk_buff *skb, bool recalculate)
> err = skb_maybe_pull_tail(skb,
> off +
> sizeof(struct frag_hdr),
> - MAX_IPV6_HDR_LEN);
> + off +
> + sizeof(struct frag_hdr));
> if (err < 0)
> goto out;
[Severity: Medium]
Since the walk is no longer capped at 256 bytes, can off now exceed
65535 before the transport header is reached, and get truncated on the
way into skb_partial_csum_set()?
skb_checksum_setup_ip() keeps off as unsigned int:
net/core/skbuff.c:skb_checksum_setup_ip() {
case IPPROTO_TCP:
err = skb_maybe_pull_tail(skb, off + sizeof(struct tcphdr),
off + MAX_TCP_HDR_LEN);
if (!err && !skb_partial_csum_set(skb, off,
offsetof(struct tcphdr,
check)))
err = -EPROTO;
}
but the callee takes u16:
net/core/skbuff.c:skb_partial_csum_set() {
bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off)
{
u32 csum_end = (u32)start + (u32)off + sizeof(__sum16);
u32 csum_start = skb_headroom(skb) + (u32)start;
if (unlikely(csum_start >= U16_MAX || csum_end > skb_headlen(skb))) {
}
With off == 65536 the argument arrives as start == 0, the range check
cannot see the original value, and skb->csum_start and
skb->transport_header end up pointing at the IPv6 header. With
recalculate == true the caller then writes through
&tcp_hdr(skb)->check, i.e. two bytes at network_header + 16, over the
IPv6 source address; with recalculate == false the skb goes up the
stack with CHECKSUM_PARTIAL metadata describing the IP header as the
transport header.
Reaching off >= 65536 also requires the pull in skb_checksum_setup_ip()
to succeed, so skb->len must be at least off + 20. That looks possible
on the netfront RX side, where xennet_get_responses() allows up to
XEN_NETIF_NR_SLOTS_MIN slots:
drivers/net/xen-netfront.c:xennet_get_responses() {
int max = XEN_NETIF_NR_SLOTS_MIN + (rx->status <= RX_COPY_THRESHOLD);
}
and xennet_fill_frags() allows up to MAX_SKB_FRAGS frags, giving an
skb->len around 69632. On the xen-netback side the guest cannot get
there because xen_netif_tx_request.size is u16 and
xenvif_count_requests() rejects totals above 65535. Pre-patch this was
unreachable from either driver because MAX_IPV6_HDR_LEN stopped the
walk at 256. Should skb_checksum_setup_ip() reject off > U16_MAX
explicitly?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260919133604.50948-1-shlomojune6%40gmail.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net v2] net: skbuff: fix pull-bound underflow in skb_checksum_setup_ipv6()
2026-09-19 13:36 [PATCH net v2] net: skbuff: fix pull-bound underflow in skb_checksum_setup_ipv6() Shihuang Liu
2026-09-22 12:37 ` netdev-bot+sashiko
@ 2026-09-24 0:50 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-24 0:50 UTC (permalink / raw)
To: Shihuang Liu; +Cc: netdev, davem, edumazet, kuba, pabeni, horms
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Sat, 19 Sep 2026 21:36:04 +0800 you wrote:
> skb_maybe_pull_tail() subtracts skb_headlen(skb) from the unsigned max
> argument and passes the result to __pskb_pull_tail() as a signed int. The
> function does not ensure that max is at least skb_headlen(skb).
>
> This can happen while parsing IPv6 extension headers when an skb already
> has a linear area larger than MAX_IPV6_HDR_LEN. Once the parser needs data
> beyond the linear area, max - skb_headlen(skb) wraps and is converted to a
> negative delta. __pskb_pull_tail() then passes that negative length to
> skb_copy_bits(), where it can become a very large copy length.
>
> [...]
Here is the summary with links:
- [net,v2] net: skbuff: fix pull-bound underflow in skb_checksum_setup_ipv6()
https://git.kernel.org/netdev/net/c/3b4e0b0c008a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-24 0:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-19 13:36 [PATCH net v2] net: skbuff: fix pull-bound underflow in skb_checksum_setup_ipv6() Shihuang Liu
2026-09-22 12:37 ` netdev-bot+sashiko
2026-09-24 0:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox