* 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