[PATCH bpf] bpf: Fix non-linear SRH access in bpf_update_srh_state()

[PATCH bpf] bpf: Fix non-linear SRH access in bpf_update_srh_state()

[Cen Zhang (Microsoft Security FORGE Labs)]

bpf_update_srh_state() locates an SRH with ipv6_find_hdr() and caches
skb->data + srhoff in the per-CPU SEG6 BPF state. This assumes that the
returned offset is within the skb linear head.

That assumption is wrong because ipv6_find_hdr() uses skb_header_pointer()
and can locate an SRH in non-linear data. The direct srh->hdrlen read and
the cached SRH pointer can therefore access memory outside the linear area.

BUG: KASAN: slab-use-after-free in bpf_update_srh_state+0x1bc/0x200
 net/core/filter.c:7027 bpf_update_srh_state()
 bpf_lwt_seg6_action()
 input_action_end_bpf()
 seg6_local_input()
 ipv6_rthdr_rcv()

Fix this by using seg6_get_srh(), which pulls and validates the complete
SRH and reloads its pointer afterwards. Pulling can reallocate skb->head,
so refresh the BPF data pointers inside bpf_update_srh_state() immediately
after the call.

For End.DT6, make the inner IPv6 base header linear before removing the
outer headers. Pulling only the outer headers can leave the inner header in
non-linear data, while ipv6_find_hdr() and the nexthop lookup access it
directly. Clear the cached SRH pointer and refresh the BPF data pointers if
the pull fails.

End.B6 and End.B6.Encap can modify or reallocate the skb before a later
nexthop lookup returns an error. Rebuild the SRH state regardless of the
action result instead of assuming that an error leaves the skb unchanged.

Fixes: 486cdf21583e ("bpf: add End.DT6 action to bpf_lwt_seg6_action helper")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>
Assisted-by: Copilot:gpt-5.6-sol
---
Please queue this fix for stable kernels.

Testing:
- Static checks only: checkpatch.pl, diff --check and patch replay.
- No targeted selftest was added. BPF LWT test-run does not support
  non-linear skbs, and the existing SEG6 netns test does not create a
  split inner IPv6 header for End.DT6.

 net/core/filter.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 61940e753552..1dd3a685d1b8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7018,15 +7018,16 @@ static void bpf_update_srh_state(struct sk_buff *skb)
 {
 	struct seg6_bpf_srh_state *srh_state =
 		this_cpu_ptr(&seg6_bpf_srh_states);
-	int srhoff = 0;
+	struct ipv6_sr_hdr *srh;
 
-	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0) {
-		srh_state->srh = NULL;
-	} else {
-		srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
-		srh_state->hdrlen = srh_state->srh->hdrlen << 3;
-		srh_state->valid = true;
-	}
+	srh = seg6_get_srh(skb, 0);
+	bpf_compute_data_pointers(skb);
+	srh_state->srh = srh;
+	if (!srh)
+		return;
+
+	srh_state->hdrlen = srh->hdrlen << 3;
+	srh_state->valid = true;
 }
 
 BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
@@ -7059,15 +7060,18 @@ BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
 
 		if (ipv6_find_hdr(skb, &hdroff, IPPROTO_IPV6, NULL, NULL) < 0)
 			return -EBADMSG;
-		if (!pskb_pull(skb, hdroff))
+		if (!pskb_may_pull(skb, hdroff + sizeof(struct ipv6hdr))) {
+			srh_state->srh = NULL;
+			bpf_compute_data_pointers(skb);
 			return -EBADMSG;
+		}
+		__skb_pull(skb, hdroff);
 
 		skb_postpull_rcsum(skb, skb_network_header(skb), hdroff);
 		skb_reset_network_header(skb);
 		skb_reset_transport_header(skb);
 		skb->encapsulation = 0;
 
-		bpf_compute_data_pointers(skb);
 		bpf_update_srh_state(skb);
 		return seg6_lookup_nexthop(skb, NULL, *(int *)param);
 	case SEG6_LOCAL_ACTION_END_B6:
@@ -7075,8 +7079,7 @@ BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
 			return -EBADMSG;
 		err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6_INLINE,
 					  param, param_len);
-		if (!err)
-			bpf_update_srh_state(skb);
+		bpf_update_srh_state(skb);
 
 		return err;
 	case SEG6_LOCAL_ACTION_END_B6_ENCAP:
@@ -7084,8 +7087,7 @@ BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
 			return -EBADMSG;
 		err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6,
 					  param, param_len);
-		if (!err)
-			bpf_update_srh_state(skb);
+		bpf_update_srh_state(skb);
 
 		return err;
 	default:

base-commit: 28d75dd3eb60812b3a87cbdf0d52c42f51b28a78
-- 
2.55.0

Re: [PATCH bpf] bpf: Fix non-linear SRH access in bpf_update_srh_state()

[sashiko-bot]

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

Pre-existing issues:
- [High] The per-CPU `srh_state->srh` pointer caching mechanism leads to use-after-free when BPF helpers or internal functions reallocate the SKB head.
--

commit 0d38e8ff1c91b9fb7ce2f48763adb2a41f8f00a2
Author: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>
Subject: bpf: Fix non-linear SRH access in bpf_update_srh_state()

The patch fixes a slab-use-after-free issue in bpf_update_srh_state() by using
seg6_get_srh() to correctly pull and validate the SRH instead of assuming it's
in the linear head. It also refreshes BPF data pointers when pulling headers.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 61940e7535523..1dd3a685d1b89 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c

[ ... ]

> @@ -7059,15 +7060,18 @@ BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
[ ... ]
>  		bpf_update_srh_state(skb);
>  		return seg6_lookup_nexthop(skb, NULL, *(int *)param);
>  	case SEG6_LOCAL_ACTION_END_B6:
> -		if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
> -			return -EBADMSG;
                ^^^^

[Severity: High]
This is a pre-existing issue, but does seg6_bpf_has_valid_srh() dereference
srh_state->srh when a prior generic BPF helper has already reallocated the
SKB head?

If a BPF program calls a generic helper like bpf_skb_store_bytes() on a
cloned SKB before this action, pskb_expand_head() might reallocate the head
and free the original memory. Since bpf_skb_store_bytes() does not clear
srh_state->srh, the check in seg6_bpf_has_valid_srh(skb) would trigger a
use-after-free when accessing the dangling pointer.

>  		err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6_INLINE,
>  					  param, param_len);

The same caching mechanism leads to pre-existing use-after-free issues in
other SEG6 helpers when dereferencing the cached SRH pointer or when
ipv6_find_hdr() fails.

For example, in bpf_lwt_seg6_store_bytes():

net/core/filter.c:bpf_lwt_seg6_store_bytes()
{
	struct ipv6_sr_hdr *srh = srh_state->srh;
...
	srh_tlvs = (void *)((char *)srh + ((srh->first_segment + 1) << 4));

[Severity: High]
This isn't a bug introduced by this patch, but does this blindly dereference
srh when it might be a dangling pointer if bpf_skb_pull_data() or another
helper previously reallocated the head?

Later in the same function:

	if (unlikely(bpf_try_make_writable(skb, offset + len)))
		return -EFAULT;
	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
		return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but if ipv6_find_hdr() fails after
bpf_try_make_writable() has successfully reallocated the head, does this
function return without clearing srh_state->srh, leaving it dangling for
subsequent helpers?

Similarly, in bpf_lwt_seg6_adjust_srh():

net/core/filter.c:bpf_lwt_seg6_adjust_srh()
{
	struct ipv6_sr_hdr *srh = srh_state->srh;
...
	srh_tlvs = (void *)((unsigned char *)srh + sizeof(*srh) +
			((srh->first_segment + 1) << 4));

[Severity: High]
This is a pre-existing issue, but is this another instance where the
potentially freed SRH pointer is blindly dereferenced?

And later in the same function:

	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
		return -EINVAL;

[Severity: High]
This is a pre-existing issue, but could an early return after skb_cow_head()
(which might reallocate the SKB head) leave srh_state->srh pointing to
freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901183151.16648-1-cenzhang@linux.microsoft.com?part=1