* [PATCH net v2 1/2] net/ipv4: reserve exthdrlen as headroom instead of counting it in skb->len
[not found] <ipv4.6d58afc075934322860394a26ceaa2e8.0@tencent.com>
@ 2026-11-05 7:34 ` =?gb18030?B?dGVuY2VudG9zX3NlY3VyZShUZW5jZW50T1Owssiruau5stPKz+Qp?=
2026-11-05 7:34 ` [PATCH net v2 2/2] net/ipv4: reject appends with negative fragment copy =?gb18030?B?dGVuY2VudG9zX3NlY3VyZShUZW5jZW50T1Owssiruau5stPKz+Qp?=
1 sibling, 0 replies; 4+ messages in thread
From: =?gb18030?B?dGVuY2VudG9zX3NlY3VyZShUZW5jZW50T1Owssiruau5stPKz+Qp?= @ 2026-11-05 7:34 UTC (permalink / raw)
To: netdev
Cc: David Ahern, Ido Schimmel, Paolo Abeni, Jakub Kicinski,
Willem de Bruijn, Willem de Bruijn, Steffen Klassert,
steffen.klassert, Eric Dumazet
When sending through an xfrm route (rt->dst.header_len > 0),
__ip_append_data() counts the bundle header space ("exthdrlen")
into skb->len of the first fragment skb: the skb setup does
skb_reserve(skb, hh_len);
data = skb_put(skb, fraglen + exthdrlen - pagedlen);
skb_set_network_header(skb, exthdrlen);
so a fully filled first skb has skb->len == maxfraglen + exthdrlen.
Since commit 353e5c9abd90 ("ipv4: Fix IPsec slowpath fragmentation
problem"), the next iteration's fraggap = skb_prev->len - maxfraglen
therefore inherits the whole bundle header length instead of the
intended alignment slop.
fraggap is relayed into the next skb by skb_copy_and_csum_bits().
The MSG_MORE (alloclen = mtu) and linear (alloclen = fraglen) branches
rely on datalen accounting for the moved tail. If datalen is capped
below transhdrlen + fraggap, the copy can extend beyond skb->end and
corrupt skb_shared_info. The paged-branch accounting introduced by
commit eca856950f7c ("ipv4: account for fraggap on the paged
allocation path") also relies on datalen being large enough for the
transport header and moved tail.
Fix the root cause by reserving exthdrlen as headroom, like IPv6 has
done since commit 1f85851e17b6 ("ipv6: fix incorrent ipv6 ipsec
packet fragment") in 2012:
skb_reserve(skb, hh_len + exthdrlen);
data = skb_put(skb, fraglen - pagedlen);
skb_reset_network_header(skb);
The corresponding IPv6 variable is dst_exthdrlen; IPv6 exthdrlen
instead holds the IPv6 extension-header length.
With this, exthdrlen no longer contributes to skb->len or to the next
iteration's fraggap. When fraggap is nonzero, it is bounded by
mtu - maxfraglen <= 7, the alignment slop that the relay was designed for.
For the same inputs to the skb setup block, the allocation request and
tail offset are unchanged (alloc_extra already carries exthdrlen).
The network and transport header offsets from skb->head are unchanged
as well. Excluding exthdrlen from skb->len intentionally changes the
subsequent fragmentation calculations. When exthdrlen is zero, these
skb setup changes have no effect.
Fixes: 353e5c9abd90 ("ipv4: Fix IPsec slowpath fragmentation problem")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Anakin Zhang <tencentos_secure@tencent.com>
---
v2:
- Correct the too-strong skb->len/fraggap wording.
- Clarify the IPv6 dst_exthdrlen terminology.
- Describe the skb_shared_info corruption without claiming a write to
an adjacent slab object; qualify the local layout comparison.
- Add Willem's Reviewed-by; no code changes.
net/ipv4/ip_output.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index eaa6fab..b84d56c 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1150,16 +1150,16 @@ alloc_new_skb:
*/
skb->ip_summed = csummode;
skb->csum = 0;
- skb_reserve(skb, hh_len);
+ skb_reserve(skb, hh_len + exthdrlen);
/*
* Find where to start putting bytes.
*/
- data = skb_put(skb, fraglen + exthdrlen - pagedlen);
- skb_set_network_header(skb, exthdrlen);
+ data = skb_put(skb, fraglen - pagedlen);
+ skb_reset_network_header(skb);
skb->transport_header = (skb->network_header +
fragheaderlen);
- data += fragheaderlen + exthdrlen;
+ data += fragheaderlen;
if (fraggap) {
skb->csum = skb_copy_and_csum_bits(
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH net v2 2/2] net/ipv4: reject appends with negative fragment copy
[not found] <ipv4.6d58afc075934322860394a26ceaa2e8.0@tencent.com>
2026-11-05 7:34 ` [PATCH net v2 1/2] net/ipv4: reserve exthdrlen as headroom instead of counting it in skb->len =?gb18030?B?dGVuY2VudG9zX3NlY3VyZShUZW5jZW50T1Owssiruau5stPKz+Qp?=
@ 2026-11-05 7:34 ` =?gb18030?B?dGVuY2VudG9zX3NlY3VyZShUZW5jZW50T1Owssiruau5stPKz+Qp?=
2026-11-06 2:51 ` Willem de Bruijn
2026-11-10 21:46 ` Jakub Kicinski
1 sibling, 2 replies; 4+ messages in thread
From: =?gb18030?B?dGVuY2VudG9zX3NlY3VyZShUZW5jZW50T1Owssiruau5stPKz+Qp?= @ 2026-11-05 7:34 UTC (permalink / raw)
To: netdev
Cc: David Ahern, Ido Schimmel, Paolo Abeni, Jakub Kicinski,
Willem de Bruijn, Willem de Bruijn, Steffen Klassert,
steffen.klassert, Eric Dumazet
__ip_append_data() caps datalen at maxfraglen - fragheaderlen when more
fragments are needed, and later computes
copy = datalen - transhdrlen - fraggap - pagedlen;
If transhdrlen + fraggap exceeds datalen, copy goes negative in the
non-paged allocation branches, where pagedlen is zero, invalidating
the offset/length accounting. In the paged allocation branch,
pagedlen = datalen - transhdrlen - fraggap wraps unsigned instead.
The subsequent copy calculation cancels to zero, so checking copy
alone does not catch this invalid fragment geometry.
This is reachable independently of the xfrm fraggap bug fixed by the
preceding patch. icmp_timestamp() passes a 20-byte transport header.
With a small MTU and IPv4 options, the capped datalen can be smaller
than that header even when fraggap is zero. The negative copy then
propagates a negative offset into icmp_glue_bits(), and a timestamp
reply with corrupted timestamp fields can be emitted.
For example, with MTU 68 and 40 bytes of IPv4 options, fragheaderlen
is 60, maxfraglen is 68, datalen is capped at 8, and fraggap is zero,
so the 20-byte transport header cannot fit.
Reject this geometry before allocation with -EINVAL, covering all three
alloclen branches and matching the existing __ip6_append_data() guard.
A negative copy cannot produce a valid append, so this does not reject a
send that could otherwise construct a correct packet. All callers flush
the cork on append error, and returning -EINVAL in mid-cork state already
happens in this function (MSG_ZEROCOPY ubuf mismatch).
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Anakin Zhang <tencentos_secure@tencent.com>
---
v2:
- Document the independently reachable ICMP Timestamp corruption path.
- Distinguish negative copy in non-paged allocation branches from
unsigned pagedlen wrap in the paged allocation branch.
- Update the subject and Fixes tag accordingly.
- Drop the unrelated xfrm Reported-by tag; no code changes.
net/ipv4/ip_output.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index b84d56c..5db8b06 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1105,6 +1105,11 @@ alloc_new_skb:
fraglen = datalen + fragheaderlen;
pagedlen = 0;
+ if (unlikely(transhdrlen + fraggap > datalen)) {
+ err = -EINVAL;
+ goto error;
+ }
+
alloc_extra = hh_len + 15;
alloc_extra += exthdrlen;
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread