[PATCH net] seg6: fix transport header for inner IPv4 with IP options

[PATCH net] seg6: fix transport header for inner IPv4 with IP options

[Andrea Mayer]

The End.DT4, End.DT46 (its IPv4 path only) and End.DX4 behaviors set the
transport header of the decapsulated IPv4 packet at a fixed 20 bytes, the
size of an IPv4 header without options. They do not validate the header
length either. With options, the transport header points at the first
option byte. The effects include:
- ping -R sends an echo request with a 60-byte IPv4 header to a local
  address through an End.DT4 or an End.DX4 SID. Since icmp_rcv()
  computes the ICMP checksum from the first option byte, the request
  fails the check and is counted in IcmpInCsumErrors.
- A UDP datagram can reach a port that an iptables rule drops. The rule
  checks the UDP header after the full IPv4 header, while the socket
  lookup reads it at the fixed 20-byte offset.
- With an inbound AH state, ah_input() reads the AH header at the fixed
  20-byte offset, but copies the IPv4 header over it with the length in
  the IHL field. A packet shorter than that length can make ah_input()
  write past the end of the data, into skb_shared_info when the skb has
  no tailroom.

Validate the inner IPv4 header length with the new helper
seg6_validate_iphdr_len() and set the transport header at that length. A
packet that is not IPv4, whose header length field is below 5, or whose
header is truncated, is dropped with a drop reason. The fix is limited to
the header length, which the transport header relies on. Other checks done
on IPv4 input, such as the header checksum, the total length and the IP
options, are not addressed here.

Found by code inspection while reviewing commit 44930446dde4 ("ipv6: seg6:
clear IPv4 control block on IPIP decapsulation"), as a pre-existing issue
that Sashiko also reported on v2 of that patch. Without this patch, a
bpftrace probe on ah_input() shows the reporter's reproducer packet
written into skb_shared_info. With it, that packet is dropped before
ah_input(), ping -R no longer fails the ICMP checksum, the UDP datagram
reaches its real port, and the SRv6 selftests still pass. These effects
reproduce on an unmodified kernel, without error injection.

Fixes: 891ef8dd2a8d ("ipv6: sr: implement additional seg6local actions")
Fixes: 664d6f86868b ("seg6: add support for the SRv6 End.DT4 behavior")
Reported-by: Hazley Samsudin <hazleycryilis@gmail.com>
Closes: https://lore.kernel.org/netdev/20260809221915.2ffc1ecd0f445fcb70270e31@uniroma2.it/
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
 net/ipv6/seg6_local.c | 56 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 53f7b2452575..c249f7751d41 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -1056,6 +1056,40 @@ static int input_action_end_dx6(struct sk_buff *skb,
 	return -EINVAL;
 }
 
+/* Validate the length of the IPv4 header at skb->data (which must point to
+ * the network header) and report that length, options included.
+ * Only the version, the header length and the presence of the whole header
+ * are checked.
+ * The length is written only when SKB_NOT_DROPPED_YET is returned.
+ *
+ * skb pointers may change: after this call, the caller must evaluate again
+ * any pointer into the packet data.
+ */
+static enum skb_drop_reason
+seg6_validate_iphdr_len(struct sk_buff *skb, int *hdrlen)
+{
+	enum skb_drop_reason reason;
+	const struct iphdr *iph;
+	int len;
+
+	reason = pskb_may_pull_reason(skb, sizeof(struct iphdr));
+	if (reason)
+		return reason;
+
+	iph = ip_hdr(skb);
+	if (iph->version != 4 || iph->ihl < 5)
+		return SKB_DROP_REASON_IP_INHDR;
+
+	len = ip_hdrlen(skb);
+
+	reason = pskb_may_pull_reason(skb, len);
+	if (reason)
+		return reason;
+
+	*hdrlen = len;
+	return SKB_NOT_DROPPED_YET;
+}
+
 static int input_action_end_dx4_finish(struct net *net, struct sock *sk,
 				       struct sk_buff *skb)
 {
@@ -1085,14 +1119,18 @@ static int input_action_end_dx4_finish(struct net *net, struct sock *sk,
 static int input_action_end_dx4(struct sk_buff *skb,
 				struct seg6_local_lwt *slwt)
 {
+	enum skb_drop_reason reason = SKB_DROP_REASON_NOT_SPECIFIED;
+	int hdrlen;
+
 	if (!decap_and_validate(skb, IPPROTO_IPIP))
 		goto drop;
 
-	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
+	reason = seg6_validate_iphdr_len(skb, &hdrlen);
+	if (reason)
 		goto drop;
 
 	skb->protocol = htons(ETH_P_IP);
-	skb_set_transport_header(skb, sizeof(struct iphdr));
+	skb_set_transport_header(skb, hdrlen);
 	nf_reset_ct(skb);
 
 	if (static_branch_unlikely(&nf_hooks_lwtunnel_enabled))
@@ -1102,7 +1140,7 @@ static int input_action_end_dx4(struct sk_buff *skb,
 
 	return input_action_end_dx4_finish(dev_net(skb->dev), NULL, skb);
 drop:
-	kfree_skb(skb);
+	kfree_skb_reason(skb, reason);
 	return -EINVAL;
 }
 
@@ -1234,6 +1272,7 @@ static struct net_device *end_dt_get_vrf_rcu(struct sk_buff *skb,
 static struct sk_buff *end_dt_vrf_core(struct sk_buff *skb,
 				       struct seg6_local_lwt *slwt, u16 family)
 {
+	enum skb_drop_reason reason = SKB_DROP_REASON_NOT_SPECIFIED;
 	struct seg6_end_dt_info *info = &slwt->dt_info;
 	struct net_device *vrf;
 	__be16 protocol;
@@ -1246,7 +1285,9 @@ static struct sk_buff *end_dt_vrf_core(struct sk_buff *skb,
 	switch (family) {
 	case AF_INET:
 		protocol = htons(ETH_P_IP);
-		hdrlen = sizeof(struct iphdr);
+		reason = seg6_validate_iphdr_len(skb, &hdrlen);
+		if (unlikely(reason))
+			goto drop;
 		break;
 	case AF_INET6:
 		protocol = htons(ETH_P_IPV6);
@@ -1260,6 +1301,7 @@ static struct sk_buff *end_dt_vrf_core(struct sk_buff *skb,
 
 	if (unlikely(info->family != AF_UNSPEC && info->family != family)) {
 		pr_warn_once("seg6local: SRv6 End.DT* family mismatch");
+		reason = SKB_DROP_REASON_NOT_SPECIFIED;
 		goto drop;
 	}
 
@@ -1273,7 +1315,7 @@ static struct sk_buff *end_dt_vrf_core(struct sk_buff *skb,
 	return end_dt_vrf_rcv(skb, family, vrf);
 
 drop:
-	kfree_skb(skb);
+	kfree_skb_reason(skb, reason);
 	return ERR_PTR(-EINVAL);
 }
 
@@ -1286,9 +1328,7 @@ static int input_action_end_dt4(struct sk_buff *skb,
 	if (!decap_and_validate(skb, IPPROTO_IPIP))
 		goto drop;
 
-	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
-		goto drop;
-
+	/* end_dt_vrf_core() validates the inner IPv4 header length */
 	skb = end_dt_vrf_core(skb, slwt, AF_INET);
 	if (!skb)
 		/* packet has been processed and consumed by the VRF */
-- 
2.43.0

Re: [PATCH net] seg6: fix transport header for inner IPv4 with IP options

[David Ahern]

On 10/19/26 8:33 PM, Andrea Mayer wrote:
> The End.DT4, End.DT46 (its IPv4 path only) and End.DX4 behaviors set the
> transport header of the decapsulated IPv4 packet at a fixed 20 bytes, the
> size of an IPv4 header without options. They do not validate the header
> length either. With options, the transport header points at the first
> option byte. The effects include:
> - ping -R sends an echo request with a 60-byte IPv4 header to a local
>   address through an End.DT4 or an End.DX4 SID. Since icmp_rcv()
>   computes the ICMP checksum from the first option byte, the request
>   fails the check and is counted in IcmpInCsumErrors.
> - A UDP datagram can reach a port that an iptables rule drops. The rule
>   checks the UDP header after the full IPv4 header, while the socket
>   lookup reads it at the fixed 20-byte offset.
> - With an inbound AH state, ah_input() reads the AH header at the fixed
>   20-byte offset, but copies the IPv4 header over it with the length in
>   the IHL field. A packet shorter than that length can make ah_input()
>   write past the end of the data, into skb_shared_info when the skb has
>   no tailroom.
> 
> Validate the inner IPv4 header length with the new helper
> seg6_validate_iphdr_len() and set the transport header at that length. A
> packet that is not IPv4, whose header length field is below 5, or whose
> header is truncated, is dropped with a drop reason. The fix is limited to
> the header length, which the transport header relies on. Other checks done
> on IPv4 input, such as the header checksum, the total length and the IP
> options, are not addressed here.
> 
> Found by code inspection while reviewing commit 44930446dde4 ("ipv6: seg6:
> clear IPv4 control block on IPIP decapsulation"), as a pre-existing issue
> that Sashiko also reported on v2 of that patch. Without this patch, a
> bpftrace probe on ah_input() shows the reporter's reproducer packet
> written into skb_shared_info. With it, that packet is dropped before
> ah_input(), ping -R no longer fails the ICMP checksum, the UDP datagram
> reaches its real port, and the SRv6 selftests still pass. These effects
> reproduce on an unmodified kernel, without error injection.
> 
> Fixes: 891ef8dd2a8d ("ipv6: sr: implement additional seg6local actions")
> Fixes: 664d6f86868b ("seg6: add support for the SRv6 End.DT4 behavior")
> Reported-by: Hazley Samsudin <hazleycryilis@gmail.com>
> Closes: https://lore.kernel.org/netdev/20260809221915.2ffc1ecd0f445fcb70270e31@uniroma2.it/
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
> ---
>  net/ipv6/seg6_local.c | 56 ++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 


LGTM.

Reviewed-by: David Ahern <dsahern@kernel.org>