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

[PATCH bpf v3] 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. The helper parameter may point into packet data, so copy the
table ID before pulling and potentially replacing skb->head. Clear the
cached SRH pointer and refresh the BPF data pointers if the pull fails.

End.B6 and End.B6.Encap can insert a new SRH and reallocate the skb before
a later HMAC calculation or nexthop lookup returns an error. Rebuild the
SRH state when the skb length changes, which indicates that the new SRH was
inserted. Failures before insertion leave the existing state unchanged.

Fixes: 486cdf21583e ("bpf: add End.DT6 action to bpf_lwt_seg6_action helper")
Reported-by: Xiang Mei <xmei5@asu.edu>
Link: https://lore.kernel.org/bpf/20260901183151.16648-1-cenzhang@linux.microsoft.com/
Suggested-by: Emil Tsalapatis <emil@etsalapatis.com>
Link: https://lore.kernel.org/bpf/CABFh=a5iLOEJdPhoaWUhLc0eEqAuhnd83_jJr9MVZZG6gSJAEw@mail.gmail.com/
Link: https://lore.kernel.org/bpf/CABFh=a4VyzxsQqsayWpTKjxY3HL2C072u=pbecEpNDbL9eBNKA@mail.gmail.com/
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>
Assisted-by: Copilot:gpt-5.6-sol
---
Changes in v3:
- Copy the End.DT6 table ID before pskb_may_pull() can replace skb->head
  and invalidate a packet-backed helper parameter.
- Rebase onto the current bpf master branch.

Changes in v2:
- Rebuild the SRH state after End.B6 and End.B6.Encap only when the skb
  length changes, avoiding selection of a spent Routing Header on errors
  before insertion.
- Rebase onto the current bpf master branch.

Please queue this fix for stable kernels.

Testing:
- Built net/core/filter.o with CONFIG_IPV6_SEG6_BPF=y.
- checkpatch.pl, diff --check and exact-base patch replay.
- No targeted runtime 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 | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 2a84f9d01131..06a748c07747 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7017,15 +7017,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,
@@ -7033,6 +7034,7 @@ BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
 {
 	struct seg6_bpf_srh_state *srh_state =
 		this_cpu_ptr(&seg6_bpf_srh_states);
+	unsigned int old_len;
 	int hdroff = 0;
 	int err;
 
@@ -7050,40 +7052,49 @@ BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
 		if (param_len != sizeof(int))
 			return -EINVAL;
 		return seg6_lookup_nexthop(skb, NULL, *(int *)param);
-	case SEG6_LOCAL_ACTION_END_DT6:
+	case SEG6_LOCAL_ACTION_END_DT6: {
+		int tbl_id;
+
 		if (!seg6_bpf_has_valid_srh(skb))
 			return -EBADMSG;
 		if (param_len != sizeof(int))
 			return -EINVAL;
+		tbl_id = *(int *)param;
 
 		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);
+		return seg6_lookup_nexthop(skb, NULL, tbl_id);
+	}
 	case SEG6_LOCAL_ACTION_END_B6:
 		if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
 			return -EBADMSG;
+		old_len = skb->len;
 		err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6_INLINE,
 					  param, param_len);
-		if (!err)
+		if (skb->len != old_len)
 			bpf_update_srh_state(skb);
 
 		return err;
 	case SEG6_LOCAL_ACTION_END_B6_ENCAP:
 		if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
 			return -EBADMSG;
+		old_len = skb->len;
 		err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6,
 					  param, param_len);
-		if (!err)
+		if (skb->len != old_len)
 			bpf_update_srh_state(skb);
 
 		return err;

base-commit: e4a62833adff6ef0fe7c0b90393204fe3c26b5c5
-- 
2.55.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free in bpf_lwt_seg6_action() during END_B6 and END_B6_ENCAP actions due to SKB reallocation.
- [High] Incomplete SRH linearization and potential dangling pointer in bpf_lwt_seg6_store_bytes().
- [High] Use-After-Free in bpf_lwt_seg6_store_bytes() via invalidated 'from' pointer.
- [High] Use-After-Free in bpf_skb_store_bytes() via invalidated 'from' pointer.
--

commit a33b7a64a031c2a48a228616879e253a6490be94
Author: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>

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

bpf_update_srh_state() assumes the SRH offset is within the skb linear
head, which can be incorrect if ipv6_find_hdr() locates it in non-linear
data. This patch replaces ipv6_find_hdr() with seg6_get_srh() to correctly
pull and validate the SRH. It also adjusts End.DT6, End.B6, and End.B6.Encap
actions to properly handle skb reallocation.

Link: https://lore.kernel.org/bpf/20260901183151.16648-1-cenzhang@linux.microsoft.com/
Link: https://lore.kernel.org/bpf/CABFh=a5iLOEJdPhoaWUhLc0eEqAuhnd83_jJr9MVZZG6gSJAEw@mail.gmail.com/
Link: https://lore.kernel.org/bpf/CABFh=a4VyzxsQqsayWpTKjxY3HL2C072u=pbecEpNDbL9eBNKA@mail.gmail.com/

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 2a84f9d011314..06a748c077479 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
[ ... ]
> @@ -7050,40 +7052,49 @@ BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
>  		if (param_len != sizeof(int))
>  			return -EINVAL;
>  		return seg6_lookup_nexthop(skb, NULL, *(int *)param);
> -	case SEG6_LOCAL_ACTION_END_DT6:
> +	case SEG6_LOCAL_ACTION_END_DT6: {
> +		int tbl_id;
> +
>  		if (!seg6_bpf_has_valid_srh(skb))
>  			return -EBADMSG;
>  		if (param_len != sizeof(int))
>  			return -EINVAL;
> +		tbl_id = *(int *)param;
>  
>  		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);
> +		return seg6_lookup_nexthop(skb, NULL, tbl_id);
> +	}
>  	case SEG6_LOCAL_ACTION_END_B6:
>  		if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
>  			return -EBADMSG;
> +		old_len = skb->len;
>  		err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6_INLINE,
>  					  param, param_len);

[Severity: High]
This is a pre-existing issue, but since param can point to packet data,
could passing it directly to bpf_push_seg6_encap() lead to a use-after-free?

In the END_DT6 case above, the code correctly copies param to tbl_id before
reallocating the skb head. For END_B6, bpf_push_seg6_encap() calls down to
__seg6_do_srh_inline(), which can trigger skb_cow_head() to reallocate the
skb, freeing the original packet data.

If param points to this freed packet data, would the subsequent memcpy() in
__seg6_do_srh_inline() read from freed memory?

net/ipv6/seg6_iptunnel.c:__seg6_do_srh_inline() {
    ...
    err = skb_cow_head(skb, hdrlen + dst_dev_overhead(cache_dst, skb));
    ...
    memcpy(isrh, osrh, hdrlen);  // osrh aliases the dangling param
    ...
}

> -		if (!err)
> +		if (skb->len != old_len)
>  			bpf_update_srh_state(skb);
>  
>  		return err;
>  	case SEG6_LOCAL_ACTION_END_B6_ENCAP:
>  		if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
>  			return -EBADMSG;
> +		old_len = skb->len;
>  		err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6,
>  					  param, param_len);

[Severity: High]
This is a pre-existing issue, but this appears to have the same vulnerability
as END_B6. Does this need to copy the param data before calling
bpf_push_seg6_encap() to prevent a use-after-free if skb_cow_head()
reallocates the skb?


I also noticed a few other pre-existing issues in this file related to
pointers being invalidated by skb head reallocation:

[Severity: High]
This isn't a bug introduced by this patch, but does bpf_lwt_seg6_store_bytes()
have a similar use-after-free if the from argument points to packet data?

net/core/filter.c:bpf_lwt_seg6_store_bytes() {
    ...
    if (unlikely(bpf_try_make_writable(skb, offset + len)))
        return -EFAULT;
    ...
    memcpy(skb->data + offset, from, len);
}

If bpf_try_make_writable() reallocates the skb head, the packet memory
from points to could be freed before the memcpy() dereferences it.


[Severity: High]
This is a pre-existing issue, but bpf_lwt_seg6_store_bytes() also appears to
leave the cached SRH pointer dangling if ipv6_find_hdr() fails after an skb
reallocation:

net/core/filter.c:bpf_lwt_seg6_store_bytes() {
    ...
    if (unlikely(bpf_try_make_writable(skb, offset + len)))
        return -EFAULT;

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

    srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
    ...
}

If bpf_try_make_writable() frees the old skb head, but ipv6_find_hdr()
fails (e.g., because the SRH is only partially linearized), the function
returns -EINVAL without updating srh_state->srh. Could a subsequent helper
then access the dangling srh_state->srh pointer?


[Severity: High]
This is a pre-existing issue, but does bpf_skb_store_bytes() also suffer from
the same use-after-free with the from pointer?

net/core/filter.c:bpf_skb_store_bytes() {
    ...
    if (unlikely(bpf_try_make_writable(skb, offset + len)))
        return -EFAULT;
    ...
    memcpy(ptr, from, len);
}

If from points to packet memory, bpf_try_make_writable() could reallocate
the skb, freeing the data that from points to before the memcpy().

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