[PATCH] xfrm: validate ihl in xfrm4_transport_output()

[PATCH] xfrm: validate ihl in xfrm4_transport_output()

[Qihang]

xfrm4_transport_output() reads the IPv4 header length (ihl) from the
packet and uses it for __skb_pull() and memmove() without checking it
against skb->len, so a frame with ihl * 4 > skb->len underflows skb->len
to a huge value.

Packets injected into an xfrm interface (e.g. AF_PACKET on an xfrmi
device) reach xfrm output without the pskb_may_pull(ihl * 4) check that
ip_rcv_core() applies to received traffic.  The underflowed length then
flows into esp_output() and the crypto scatterlist setup, where every
length in the underflow window ends in a fatal fault (BUG_ON in
__skb_to_sgvec(), or a wild scatterlist access in the crypto layer).
This is a deterministic local denial of service reachable by an
unprivileged user through a user and network namespace.

xfrm6_transport_output() bounds the header length via
xfrm6_hdr_offset() and the IPv4 tunnel/BEET paths use the constant
XFRM_MODE_SKB_CB(skb)->ihl; only IPv4 transport uses the live,
unvalidated value.  Reject the packet when ihl exceeds skb->len.

Fixes: b59f45d0b2878 ("[IPSEC] xfrm: Abstract out encapsulation modes")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
 net/xfrm/xfrm_output.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -66,6 +66,9 @@ static int xfrm4_transport_output(struct xfrm_state *x, struct sk_buff *skb)
 	struct iphdr *iph = ip_hdr(skb);
 	int ihl = iph->ihl * 4;
 
+	if (ihl > skb->len)
+		return -EINVAL;
+
 	if (!skb->inner_protocol)
 		skb_set_inner_transport_header(skb,
 					       skb_transport_offset(skb));
-- 
2.46.0

Re: [PATCH] xfrm: validate ihl in xfrm4_transport_output()

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 (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/xfrm/xfrm_output.c:69-70, xfrm4_transport_output(): the new
  guard validates the attacker-controlled IPv4 header length against…

Pre-existing issues:
- [Medium] net/xfrm/xfrm_output.c:66-81, xfrm4_transport_output(): only
  an upper bound on ihl is added, so iph->ihl values 0..4 (ihl = 0..16…

--

xfrm: validate ihl in xfrm4_transport_output()

This adds a bounds check on the IPv4 header length read from the packet in
xfrm4_transport_output(), rejecting the packet with -EINVAL when ihl exceeds
skb->len, before the header relocation done by __skb_pull() and memmove().
The stated goal is to close a local denial of service reachable by injecting
frames into an xfrm interface, for example with AF_PACKET on an xfrmi device.

> diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
> index e305ba32e356b..3abb21d113a45 100644
> --- a/net/xfrm/xfrm_output.c
> +++ b/net/xfrm/xfrm_output.c
> @@ -66,6 +66,9 @@ static int xfrm4_transport_output(struct xfrm_state *x, struct sk_buff *skb)
>  	struct iphdr *iph = ip_hdr(skb);
>  	int ihl = iph->ihl * 4;
>  
> +	if (ihl > skb->len)
> +		return -EINVAL;
> +

[Severity: High]
Is skb->len the right bound here?  skb->len covers the paged fragment data
too, but the two operations further down in xfrm4_transport_output() need
the linear head length:

net/xfrm/xfrm_output.c:xfrm4_transport_output()
	skb->transport_header = skb->network_header + ihl;
	__skb_pull(skb, ihl);
	memmove(skb_network_header(skb), iph, ihl);

include/linux/skbuff.h:__skb_pull()
	skb->len -= len;
	if (unlikely(skb->len < skb->data_len)) {
		...
		BUG();
	}

So for any skb where skb_headlen(skb) < ihl <= skb->len, the new check
passes and __skb_pull() still drives skb->len below skb->data_len and
BUG()s, and the memmove() source range still runs past skb->tail.

Can the injection vector named in the changelog produce exactly such an
skb?  In packet_snd() the linear length is attacker controlled:

net/packet/af_packet.c:packet_snd()
	linear = __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len);
	linear = max(linear, min_t(int, len, hard_header_len));

xfrmi_dev_setup() never calls ether_setup(), so hard_header_len is 0 and
linear stays at hdr_len, and packet_alloc_skb() keeps the rest paged:

net/packet/af_packet.c:packet_alloc_skb()
	if (prepad + len < PAGE_SIZE || !linear)
		linear = len;
	...
	skb_put(skb, linear);
	skb->data_len = len - linear;

With hdr_len = 1 and len larger than PAGE_SIZE, skb_headlen(skb) is 1 and
the single linear byte is the attacker's version/ihl byte.

Does anything linearize the skb before the pull?  xfrmi_dev_init() sets
NETIF_F_SG | NETIF_F_FRAGLIST, so skb_needs_linearize() is false, and
xfrm_output_one() only calls xfrm_skb_check_space() (pskb_expand_head(),
which does not linearize) before xfrm_outer_mode_output().

Would using the linear bound instead work, i.e.

	if (ihl > skb_headlen(skb))
		return -EINVAL;

or

	if (!pskb_may_pull(skb, ihl))
		return -EINVAL;

which also makes the memmove() source in-bounds?

[Severity: Medium]
This isn't a bug introduced by this patch, but since the check being added
here is the ihl validation, should it also enforce a lower bound?  Only the
upper bound is checked, so iph->ihl values 0..4 (0..16 bytes, less than
sizeof(struct iphdr)) are still accepted.

The changelog points at the validation in ip_rcv_core(), which rejects both
directions:

net/ipv4/ip_input.c:ip_rcv_core()
	if (iph->ihl < 5 || iph->version != 4)
		goto inhdr_error;

and sibling xfrm4 code already assumes ihl >= 5:

net/xfrm/xfrm_inout.h:xfrm4_extract_header()
	XFRM_MODE_SKB_CB(skb)->optlen = iph->ihl * 4 - sizeof(*iph);

Can a short ihl leak uninitialized headroom?  In
xfrm4_transport_output() mac_header is placed at a fixed offset while only
ihl bytes are relocated:

	skb->mac_header = skb->network_header +
			  offsetof(struct iphdr, protocol);
	...
	memmove(skb_network_header(skb), iph, ihl);

offsetof(struct iphdr, protocol) is 9, so for iph->ihl <= 2 the byte at
skb_mac_header() is never written, and esp_output() reads it:

net/ipv4/esp4.c:esp_output()
	esp.proto = *skb_mac_header(skb);
	*skb_mac_header(skb) = IPPROTO_ESP;

esp_output_head() then puts that value on the wire:

	esp_output_fill_trailer(tail, esp->tfclen, esp->plen, esp->proto);

For iph->ihl == 2 the emitted outer header bytes covering check/saddr/daddr
are similarly never-initialized headroom, apart from tot_len and check
being rewritten by __ip_local_out()/ip_send_check().

On reachability, the IP_HDRINCL raw path cannot deliver this because
raw_send_hdrinc() rejects it:

net/ipv4/raw.c:raw_send_hdrinc()
	if (iphlen > length || iphlen < sizeof(*iph))
		goto error_free;

and for the AF_PACKET/xfrmi path the flow dissector bails out on short ihl:

net/core/flow_dissector.c:__skb_flow_dissect()
		if (!iph || iph->ihl < 5) {
			fdret = FLOW_DISSECT_RET_OUT_BAD;

which leaves decode_session4() with an all-zero flowi4, so a wildcard
(0.0.0.0) transport-mode policy and SA would additionally have to be
selected.  I could not establish that such a state cannot be installed, so
would adding an ihl < sizeof(struct iphdr) rejection alongside the new
check be worthwhile?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917072226.80788-1-q.h.hack.winter%40gmail.com

Re: [PATCH] xfrm: validate ihl in xfrm4_transport_output()

[Qihang]

Hi Sashiko,

Thanks for the review.  Both points are valid; I will send a v2.

[High] pskb_may_pull() instead of skb->len

You are right that skb->len is too loose.  __skb_pull() only needs the
header to be in the linear area: it BUG()s when the pull drives skb->len
below skb->data_len, i.e. when ihl > skb_headlen(skb).  skb->len counts
the paged fragments too, so an AF_PACKET packet with a virtio net header
and hdr_len = 1 leaves skb_headlen() == 1 while skb->len can be large;
ihl values up to skb->len then pass the ihl > skb->len check and still
reach the BUG() (and the memmove() source runs past skb->tail).  v2 uses
pskb_may_pull(skb, ihl): it linearizes the header, which both satisfies
__skb_pull()'s precondition and keeps the memmove() source in bounds,
and it rejects ihl > skb->len.  ip_hdr() is re-read afterwards because
pskb_may_pull() may reallocate the skb head.

[Medium] lower bound on ihl

Agreed, thanks.  v2 also rejects ihl < 5, matching ip_rcv_core() and the
implicit assumption in xfrm4_extract_header() and the IPv4 BEET paths.
This keeps the mac_header protocol byte initialized before esp_output()
reads it, so the uninitialized-headroom path you described is closed as
well.

The check is now:

	if (iph->ihl < 5 || !pskb_may_pull(skb, ihl))
		return -EINVAL;

	iph = ip_hdr(skb);

v2 sent separately as [PATCH v2].

pw-bot: cr

[PATCH net v2] xfrm: validate ihl in xfrm4_transport_output()

[Qihang]

xfrm4_transport_output() reads the IPv4 header length (ihl) from the
packet and uses it for __skb_pull() and memmove() without validating it,
so a frame with ihl * 4 larger than the linear header underflows skb->len
to a huge value or trips BUG() in __skb_pull().

Packets injected into an xfrm interface (e.g. AF_PACKET on an xfrmi
device) reach xfrm output without the header validation that
ip_rcv_core() applies to received traffic.  The underflowed length then
flows into esp_output() and the crypto scatterlist setup, where every
length in the underflow window ends in a fatal fault (BUG_ON in
__skb_to_sgvec()).  This is a deterministic local denial of service
reachable by an unprivileged user through a user and network namespace.

Reject ihl < 5, matching ip_rcv_core(), so a complete IPv4 header is
present.  Then use pskb_may_pull() to make the header linear: this
rejects ihl > skb->len, linearizes a header that currently lives in
fragments, and guarantees __skb_pull()'s precondition that the pull must
not drive skb->len below skb->data_len, and it keeps the memmove() source
in bounds.  ip_hdr() is re-read afterwards because pskb_may_pull() may
reallocate the skb head.  xfrm6_transport_output() already bounds its
header length through xfrm6_hdr_offset().

Fixes: b59f45d0b2878 ("[IPSEC] xfrm: Abstract out encapsulation modes")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
 net/xfrm/xfrm_output.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index e305ba3..86943e2 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -66,6 +66,11 @@ static int xfrm4_transport_output(struct xfrm_state *x, struct sk_buff *skb)
 	struct iphdr *iph = ip_hdr(skb);
 	int ihl = iph->ihl * 4;
 
+	if (iph->ihl < 5 || !pskb_may_pull(skb, ihl))
+		return -EINVAL;
+
+	iph = ip_hdr(skb);
+
 	if (!skb->inner_protocol)
 		skb_set_inner_transport_header(skb,
 					       skb_transport_offset(skb));
-- 
2.46.0

Re: [PATCH net v2] xfrm: validate ihl in xfrm4_transport_output()

[Herbert Xu]

On Wed, Sep 23, 2026 at 10:16:40AM +0800, Qihang wrote:
> xfrm4_transport_output() reads the IPv4 header length (ihl) from the
> packet and uses it for __skb_pull() and memmove() without validating it,
> so a frame with ihl * 4 larger than the linear header underflows skb->len
> to a huge value or trips BUG() in __skb_pull().
> 
> Packets injected into an xfrm interface (e.g. AF_PACKET on an xfrmi
> device) reach xfrm output without the header validation that
> ip_rcv_core() applies to received traffic.  The underflowed length then
> flows into esp_output() and the crypto scatterlist setup, where every
> length in the underflow window ends in a fatal fault (BUG_ON in
> __skb_to_sgvec()).  This is a deterministic local denial of service
> reachable by an unprivileged user through a user and network namespace.
> 
> Reject ihl < 5, matching ip_rcv_core(), so a complete IPv4 header is
> present.  Then use pskb_may_pull() to make the header linear: this
> rejects ihl > skb->len, linearizes a header that currently lives in
> fragments, and guarantees __skb_pull()'s precondition that the pull must
> not drive skb->len below skb->data_len, and it keeps the memmove() source
> in bounds.  ip_hdr() is re-read afterwards because pskb_may_pull() may
> reallocate the skb head.  xfrm6_transport_output() already bounds its
> header length through xfrm6_hdr_offset().
> 
> Fixes: b59f45d0b2878 ("[IPSEC] xfrm: Abstract out encapsulation modes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
> ---
>  net/xfrm/xfrm_output.c | 5 +++++
>  1 file changed, 5 insertions(+)

As I said when this first came up, our entire IPv4 stack
assumes that this is validated upon entry into the IP stack.

So taking a whack-a-mole approach inside the IPv4 stack is the
wrong thing to do.

Please fix those entry points into the stack instead by ensuring
that the IP header is valid.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH net v2] xfrm: validate ihl in xfrm4_transport_output()

[Qihang]

On Wed, Sep 23, 2026 at 12:24:09PM +1000, Herbert Xu wrote:
> As I said when this first came up, our entire IPv4 stack
> assumes that this is validated upon entry into the IP stack.
>
> So taking a whack-a-mole approach inside the IPv4 stack is the
> wrong thing to do.
>
> Please fix those entry points into the stack instead by ensuring
> that the IP header is valid.

Understood, thanks.  v3 (posted as a new thread) drops the
xfrm4_transport_output() change and validates the header at the
xfrmi_xmit() entry instead, mirroring the pskb_inet_may_pull() call
that vti_tunnel_xmit() already has:

    <20260923032456.45814-1-q.h.hack.winter@gmail.com>

Thanks,
Qihang