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

[Emil Tsalapatis]

On Wed, Sep 9, 2026 at 9:53 AM Cen Zhang (Microsoft Security FORGE
Labs) <cenzhang@xxxxxxxxxxxxxxxxxxx> 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 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@xxxxxxx>
> Link: https://lore.kernel.org/bpf/20260901183151.16648-1-cenzhang@xxxxxxxxxxxxxxxxxxx/
> Suggested-by: Emil Tsalapatis <emil@xxxxxxxxxxxxxxx>
> Link: https://lore.kernel.org/bpf/CABFh=a5iLOEJdPhoaWUhLc0eEqAuhnd83_jJr9MVZZG6gSJAEw@xxxxxxxxxxxxxx/
> Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@xxxxxxxxxxxxxxxxxxx>
> Assisted-by: Copilot:gpt-5.6-sol
> ---
> 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:
> - 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 | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 8513167a858a..b037d70e5fe6 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;
>
> @@ -7058,32 +7060,37 @@ 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);

Sorry but sth we missed on v1:, We need to do tbl_id = *(int *)param;
before we start
pulling. The verifier allows the *param pointer to point to packet
memory, and we may
be freeing the packet in pskb_may_pull(). There is no good reason for
*param to be
pointing to packet memory, but it's still possible.

pw-bot: cr

>         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: 15e2565f1c43771af0bc5324971cabaad79ac286
> --
> 2.55.0