mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: Fix non-linear SRH access in bpf_update_srh_state()
@ 2026-09-01 18:31 Cen Zhang (Microsoft Security FORGE Labs)
  2026-09-08 23:19 ` Emil Tsalapatis
  0 siblings, 1 reply; 3+ messages in thread
From: Cen Zhang (Microsoft Security FORGE Labs) @ 2026-09-01 18:31 UTC (permalink / raw)
  To: bpf
  Cc: daniel, john.fastabend, sdf, martin.lau, ast, andrii, eddyz87,
	memxor, song, yonghong.song, jolsa, emil, ihor.solodrai, davem,
	edumazet, kuba, pabeni, horms, m.xhonneux, dlebrun, netdev,
	linux-kernel, AutonomousCodeSecurity, xmei5, tgopinath, kys,
	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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf] bpf: Fix non-linear SRH access in bpf_update_srh_state()
  2026-09-01 18:31 [PATCH bpf] bpf: Fix non-linear SRH access in bpf_update_srh_state() Cen Zhang (Microsoft Security FORGE Labs)
@ 2026-09-08 23:19 ` Emil Tsalapatis
  2026-09-09 13:18   ` Cen Zhang (Microsoft Security FORGE Labs)
  0 siblings, 1 reply; 3+ messages in thread
From: Emil Tsalapatis @ 2026-09-08 23:19 UTC (permalink / raw)
  To: Cen Zhang (Microsoft Security FORGE Labs)
  Cc: bpf, daniel, john.fastabend, sdf, martin.lau, ast, andrii,
	eddyz87, memxor, song, yonghong.song, jolsa, ihor.solodrai,
	davem, edumazet, kuba, pabeni, horms, m.xhonneux, dlebrun,
	netdev, linux-kernel, AutonomousCodeSecurity, xmei5, tgopinath,
	kys

On Tue, Sep 1, 2026 at 2:32 PM Cen Zhang (Microsoft Security FORGE
Labs) <cenzhang@linux.microsoft.com> wrote:
>
> 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

This fix has an issue afaict, commenting inline:


> ---
> 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);
>

Here and for the case below, making the SRH state recomputation
unconditional  can
cause logic errors. It's only safe to recompute the state if
bpf_push_seg6_encap()
actually adds an SRH up front, then fails, because then the header we
want is the
very first one.

If we don't add an SRH before calling seg6_get_srh(), it calls
ipv6_find_hdr() w/o IP6_FH_F_SKIP_RH and
end up pointing to the first header it finds even if it is a spent
SRH. That normally doesn't
matter because the caller should not continue processing after an
error, but it is still
inconsistent.

The easy fix on top of this patch is afaict:

1) Track the old length of the skb before the bpf_push_seg6_encap();
2) in bpf_lwt_seg6_action:

if (new_len != old_len)
    bpf_update_srh_state(skb);

instead of unconditional.

wdyt?

pw-bot: cr

>                 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf] bpf: Fix non-linear SRH access in bpf_update_srh_state()
  2026-09-08 23:19 ` Emil Tsalapatis
@ 2026-09-09 13:18   ` Cen Zhang (Microsoft Security FORGE Labs)
  0 siblings, 0 replies; 3+ messages in thread
From: Cen Zhang (Microsoft Security FORGE Labs) @ 2026-09-09 13:18 UTC (permalink / raw)
  To: emil
  Cc: AutonomousCodeSecurity, andrii, ast, bpf, cenzhang, daniel,
	davem, dlebrun, eddyz87, edumazet, horms, ihor.solodrai,
	john.fastabend, jolsa, kuba, kys, linux-kernel, m.xhonneux,
	martin.lau, memxor, netdev, pabeni, sdf, song, tgopinath, xmei5,
	yonghong.song

Thanks, I'll make this change in v2 shortly.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-09 13:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-01 18:31 [PATCH bpf] bpf: Fix non-linear SRH access in bpf_update_srh_state() Cen Zhang (Microsoft Security FORGE Labs)
2026-09-08 23:19 ` Emil Tsalapatis
2026-09-09 13:18   ` Cen Zhang (Microsoft Security FORGE Labs)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®