Re: [PATCH bpf] bpf: refresh seg6local SRH pointer after skb pull

[Weiming Shi]

Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> 于2026年9月8日周二 04:02写道：
>
> On Mon Sep 7, 2026 at 12:21 PM PDT, Weiming Shi wrote:
> > An LWT_SEG6LOCAL program can invalidate its cached SRH with
> > bpf_lwt_seg6_adjust_srh() and then call bpf_skb_pull_data(). The latter
> > may reallocate skb->head, leaving the per-CPU SRH pointer dangling.
> > Post-program SRH validation then writes through that pointer.
> >
> > BUG: KASAN: slab-use-after-free in seg6_bpf_has_valid_srh (net/ipv6/seg6_local.c:1411)
> > Write of size 1
> >  seg6_bpf_has_valid_srh (net/ipv6/seg6_local.c:1411)
> >  input_action_end_bpf (net/ipv6/seg6_local.c:1463)
> >  seg6_local_input_core (net/ipv6/seg6_local.c:1630)
> >  seg6_local_input (net/ipv6/seg6_local.c:1639)
> >  lwtunnel_input (net/core/lwtunnel.c:466)
> >  ipv6_rcv (net/ipv6/ip6_input.c:351)
> >
> > Give LWT_SEG6LOCAL its own bpf_skb_pull_data() implementation. Save
> > the cached SRH offset before the skb operation and rebuild the pointer
> > from the current skb->data afterwards. Since pulling data can replace
> > storage but does not change packet layout, this preserves the identity
> > of the cached SRH even when multiple Routing Headers are present.
> >
> > Refresh the pointer even on error because __pskb_pull_tail() can replace
> > the head before a later step fails. Preserve the pending hdrlen and valid
> > state so SRH validation semantics remain unchanged.
> >
> > Fixes: 004d4b274e2a ("ipv6: sr: Add seg6local action End.BPF")
> > Reported-by: co+adfca3e91be95776@xxxxxxx
> > Closes: https://lore.kernel.org/all/GCy0KRM2IcQGoJQTjJEU9D0maBxXzEDHuQpq@xxxxxxx/
> > Cc: stable@xxxxxxxxxxxxxxx
> > Assisted-by: Claude:gpt-5
> > Signed-off-by: Weiming Shi <bestswngs@xxxxxxxxx>
> > ---
> >  net/core/filter.c | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 61940e7535523..e61f9e9226b10 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -7162,6 +7162,32 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
> >       .arg2_type      = ARG_ANYTHING,
> >       .arg3_type      = ARG_ANYTHING,
> >  };
> > +
> > +BPF_CALL_2(bpf_lwt_seg6_pull_data, struct sk_buff *, skb, u32, len)
> > +{
> > +     struct seg6_bpf_srh_state *srh_state =
> > +             this_cpu_ptr(&seg6_bpf_srh_states);
> > +     unsigned int srhoff;
> > +     int ret;
> > +
> > +     lockdep_assert_held(&srh_state->bh_lock);
> > +     if (!srh_state->srh)
> > +             return ____bpf_skb_pull_data(skb, len);
> > +
> > +     srhoff = (unsigned char *)srh_state->srh - skb->data;
> > +     ret = ____bpf_skb_pull_data(skb, len);
> > +     srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct bpf_func_proto bpf_lwt_seg6_pull_data_proto = {
> > +     .func           = bpf_lwt_seg6_pull_data,
> > +     .gpl_only       = false,
> > +     .ret_type       = RET_INTEGER,
> > +     .arg1_type      = ARG_PTR_TO_CTX,
> > +     .arg2_type      = ARG_ANYTHING,
> > +};
> >  #endif /* CONFIG_IPV6_SEG6_BPF */
> >
> >  #ifdef CONFIG_INET
> > @@ -9052,6 +9078,8 @@ lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >               return &bpf_lwt_seg6_action_proto;
> >       case BPF_FUNC_lwt_seg6_adjust_srh:
> >               return &bpf_lwt_seg6_adjust_srh_proto;
> > +     case BPF_FUNC_skb_pull_data:
> > +             return &bpf_lwt_seg6_pull_data_proto;
>
> Instead of adding new support that no one will use, just disallow this helper from lwt_seg6.
>
> pw-bot: cr

Thanks for the review. I will incorporate your feedback and send v2.