* [PATCH bpf] bpf: refresh seg6local SRH pointer after skb pull
@ 2026-09-07 19:21 Weiming Shi
2026-09-07 19:44 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Weiming Shi @ 2026-09-07 19:21 UTC (permalink / raw)
To: Daniel Borkmann, John Fastabend, Stanislav Fomichev,
Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: bpf, netdev, linux-kernel, co+adfca3e91be95776, Xiang Mei,
Weiming Shi, stable
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@bugs.sh
Closes: https://lore.kernel.org/all/GCy0KRM2IcQGoJQTjJEU9D0maBxXzEDHuQpq@bugs.sh/
Cc: stable@vger.kernel.org
Assisted-by: Claude:gpt-5
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
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;
#endif
default:
return lwt_out_func_proto(func_id, prog);
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] bpf: refresh seg6local SRH pointer after skb pull
2026-09-07 19:21 [PATCH bpf] bpf: refresh seg6local SRH pointer after skb pull Weiming Shi
@ 2026-09-07 19:44 ` sashiko-bot
2026-09-07 20:02 ` Alexei Starovoitov
2026-09-09 4:08 ` [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL Weiming Shi
2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-07 19:44 UTC (permalink / raw)
To: Weiming Shi; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The fix for the dangling SRH pointer is conditionally compiled under `CONFIG_IPV6_SEG6_BPF`, leaving the use-after-free vulnerability unpatched when IPv6 is compiled as a module (`CONFIG_IPV6=m`).
- [Critical] Early error returns in `bpf_lwt_seg6_store_bytes`, `bpf_lwt_seg6_adjust_srh`, and `bpf_lwt_seg6_action` leave `srh_state->srh` dangling if the SKB head was reallocated before the failure, causing the exact same use-after-free that the patch attempts to fix for `bpf_skb_pull_data`.
- [High] `bpf_lwt_seg6_store_bytes`, `bpf_lwt_seg6_adjust_srh`, and `bpf_update_srh_state` use `ipv6_find_hdr` without `IP6_FH_F_SKIP_RH`, correctly identifying the wrong routing header if multiple are present.
--
commit 681db76be957fb9fd9b5dfbb7a3c0b06da4f90b7
Author: Weiming Shi <bestswngs@gmail.com>
bpf: refresh seg6local SRH pointer after skb pull
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.
Closes: https://lore.kernel.org/all/GCy0KRM2IcQGoJQTjJEU9D0maBxXzEDHuQpq@bugs.sh/
> 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,
> };
[Severity: High]
This isn't a bug introduced by this patch, but bpf_lwt_seg6_store_bytes(),
bpf_lwt_seg6_adjust_srh(), and bpf_update_srh_state() use ipv6_find_hdr()
without IP6_FH_F_SKIP_RH, which can correctly identify the wrong routing
header if multiple are present.
If a remote packet arrives with an exhausted Routing Header (e.g., Type 0)
followed by an active SRH, seg6_get_srh() initially correctly skips the
exhausted header using IP6_FH_F_SKIP_RH.
However, when the BPF program calls bpf_lwt_seg6_store_bytes() or
bpf_lwt_seg6_adjust_srh(), they recalculate the pointer using
ipv6_find_hdr() without IP6_FH_F_SKIP_RH, which stops at the exhausted
first header:
net/core/filter.c:bpf_lwt_seg6_store_bytes() {
...
if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
return -EINVAL;
...
}
This reassigns srh_state->srh to the wrong header. Later,
seg6_bpf_has_valid_srh() overwrites the hdrlen of the exhausted header and
validates against it, which can cause packet corruption and SRH validation
bypass.
Should we update these functions to use IP6_FH_F_SKIP_RH when finding the
routing header to ensure we match the correct active SRH?
> +
> +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;
> +}
[Severity: Critical]
This is a pre-existing issue, but I noticed early error returns in
bpf_lwt_seg6_store_bytes(), bpf_lwt_seg6_adjust_srh(), and
bpf_lwt_seg6_action() leave srh_state->srh dangling if the SKB head was
reallocated before the failure, causing the exact same use-after-free
regression that this patch attempts to fix for bpf_skb_pull_data().
For example, in bpf_lwt_seg6_store_bytes():
net/core/filter.c:bpf_lwt_seg6_store_bytes() {
...
if (unlikely(bpf_try_make_writable(skb, offset + len)))
return -EFAULT;
...
}
If bpf_try_make_writable() successfully reallocates skb->head and frees the
old head, but then fails in a subsequent step such as an skb_clone()
fragment failure, it returns an error code immediately.
Because the early error paths do not update srh_state->srh, it remains
pointing to the freed old head. When the BPF program finishes,
seg6_bpf_has_valid_srh() accesses this freed memory.
A similar situation occurs in bpf_lwt_seg6_adjust_srh():
net/core/filter.c:bpf_lwt_seg6_adjust_srh() {
...
if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
return -EINVAL;
...
}
If the earlier skb_cow_head() successfully freed the old head, and then this
IPv6 header search fails, it returns early and leaves the SRH pointer dangling.
Also in bpf_lwt_seg6_action():
net/core/filter.c:bpf_lwt_seg6_action() {
...
if (!pskb_pull(skb, hdroff))
return -EBADMSG;
...
}
If pskb_pull() reallocates the head and fails, bpf_update_srh_state(skb) is
never called, leaving the SRH pointer dangling.
Should we also ensure the SRH pointer is updated on early error returns in
these functions, similarly to how it is handled for bpf_skb_pull_data() here?
> +
> +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 */
[ ... ]
> @@ -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;
> #endif
> default:
> return lwt_out_func_proto(func_id, prog);
[Severity: Critical]
This is a pre-existing issue, but I noticed the fix for the dangling SRH
pointer is conditionally compiled under CONFIG_IPV6_SEG6_BPF, leaving the
use-after-free regression unpatched when IPv6 is compiled as a module
(CONFIG_IPV6=m).
When IPv6 is built as a module, CONFIG_IPV6_SEG6_BPF evaluates to 'n'.
BPF programs can still be attached via parse_nla_bpf() which unconditionally
supports BPF_PROG_TYPE_LWT_SEG6LOCAL. The verifier defaults
BPF_FUNC_skb_pull_data to the standard bpf_skb_pull_data_proto wrapper
instead of this newly introduced safe wrapper because of this fallback:
net/core/filter.c:lwt_seg6local_func_proto() {
...
#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
...
#endif
default:
return lwt_out_func_proto(func_id, prog);
}
This standard wrapper can reallocate the SKB head without updating the
per-CPU srh_state->srh pointer. When the BPF program finishes,
input_action_end_bpf() in net/ipv6/seg6_local.c reads the dangling
pointer regardless of CONFIG_IPV6_SEG6_BPF:
net/ipv6/seg6_local.c:input_action_end_bpf() {
...
if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
goto drop;
...
}
Can we make sure the safe wrapper is also used when IPv6 is compiled as a
module to avoid this regression?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260907192129.557377-2-bestswngs@gmail.com?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] bpf: refresh seg6local SRH pointer after skb pull
2026-09-07 19:21 [PATCH bpf] bpf: refresh seg6local SRH pointer after skb pull Weiming Shi
2026-09-07 19:44 ` sashiko-bot
@ 2026-09-07 20:02 ` Alexei Starovoitov
2026-09-08 15:35 ` Weiming Shi
2026-09-09 4:08 ` [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL Weiming Shi
2 siblings, 1 reply; 8+ messages in thread
From: Alexei Starovoitov @ 2026-09-07 20:02 UTC (permalink / raw)
To: Weiming Shi, Daniel Borkmann, John Fastabend, Stanislav Fomichev,
Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: bpf, netdev, linux-kernel, co+adfca3e91be95776, Xiang Mei, stable
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@bugs.sh
> Closes: https://lore.kernel.org/all/GCy0KRM2IcQGoJQTjJEU9D0maBxXzEDHuQpq@bugs.sh/
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:gpt-5
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] bpf: refresh seg6local SRH pointer after skb pull
2026-09-07 20:02 ` Alexei Starovoitov
@ 2026-09-08 15:35 ` Weiming Shi
0 siblings, 0 replies; 8+ messages in thread
From: Weiming Shi @ 2026-09-08 15:35 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, John Fastabend, Stanislav Fomichev,
Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, bpf, netdev, linux-kernel, co+adfca3e91be95776,
Xiang Mei, stable
Alexei Starovoitov <alexei.starovoitov@gmail.com> 于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@bugs.sh
> > Closes: https://lore.kernel.org/all/GCy0KRM2IcQGoJQTjJEU9D0maBxXzEDHuQpq@bugs.sh/
> > Cc: stable@vger.kernel.org
> > Assisted-by: Claude:gpt-5
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> > 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL
2026-09-07 19:21 [PATCH bpf] bpf: refresh seg6local SRH pointer after skb pull Weiming Shi
2026-09-07 19:44 ` sashiko-bot
2026-09-07 20:02 ` Alexei Starovoitov
@ 2026-09-09 4:08 ` Weiming Shi
2026-09-09 4:27 ` sashiko-bot
` (2 more replies)
2 siblings, 3 replies; 8+ messages in thread
From: Weiming Shi @ 2026-09-09 4:08 UTC (permalink / raw)
To: Daniel Borkmann, John Fastabend, Stanislav Fomichev,
Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: bpf, netdev, linux-kernel, David Lebrun, Mathieu Xhonneux,
co+adfca3e91be95776, Xiang Mei, Weiming Shi, Alexei Starovoitov,
stable
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.
Disallow bpf_skb_pull_data() for LWT_SEG6LOCAL programs so the verifier
rejects this unsafe helper combination. Other LWT program types continue
to expose the helper through lwt_out_func_proto().
Fixes: 004d4b274e2a ("ipv6: sr: Add seg6local action End.BPF")
Reported-by: co+adfca3e91be95776@bugs.sh
Closes: https://lore.kernel.org/all/GCy0KRM2IcQGoJQTjJEU9D0maBxXzEDHuQpq@bugs.sh/
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Link: https://lore.kernel.org/bpf/DL9COXZQXX4V.1FN45QO2Q77ZH@gmail.com/
Cc: stable@vger.kernel.org
Assisted-by: Claude:gpt-5
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
Changes in v2:
- Disallow bpf_skb_pull_data() for LWT_SEG6LOCAL instead of adding a
wrapper to refresh the cached SRH pointer, as suggested by Alexei.
net/core/filter.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 8513167a858a8..2a84f9d011314 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -9044,6 +9044,8 @@ static const struct bpf_func_proto *
lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
switch (func_id) {
+ case BPF_FUNC_skb_pull_data:
+ return NULL;
#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
case BPF_FUNC_lwt_seg6_store_bytes:
return &bpf_lwt_seg6_store_bytes_proto;
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL
2026-09-09 4:08 ` [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL Weiming Shi
@ 2026-09-09 4:27 ` sashiko-bot
2026-09-09 18:18 ` Emil Tsalapatis
2026-09-09 18:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-09 4:27 UTC (permalink / raw)
To: Weiming Shi; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] LWT-specific SRv6 helpers (bpf_lwt_seg6_action(), bpf_lwt_seg6_adjust_srh(), and bpf_lwt_seg6_store_bytes()) can leave srh_state->srh dangling if an error occurs after skb->head is reallocated, leading to a Use-After-Free when the BPF program finishes.
- [High] bpf_lwt_seg6_adjust_srh() corrupts cloned SKB data when shrinking the header (len < 0).
--
commit 432b32d6ccc70f500393dbc29ce68e92bc184b1c
Author: Weiming Shi <bestswngs@gmail.com>
bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL
An LWT_SEG6LOCAL program can invalidate its cached SRH with
bpf_lwt_seg6_adjust_srh() and then call bpf_skb_pull_data(), leaving the
per-CPU SRH pointer dangling. This commit disallows bpf_skb_pull_data() for
LWT_SEG6LOCAL programs so the verifier rejects this unsafe combination.
Link: https://lore.kernel.org/bpf/DL9COXZQXX4V.1FN45QO2Q77ZH@gmail.com/
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 8513167a858a8..2a84f9d011314 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -9044,6 +9044,8 @@ static const struct bpf_func_proto *
> lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> {
> switch (func_id) {
> + case BPF_FUNC_skb_pull_data:
> + return NULL;
> #if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
> case BPF_FUNC_lwt_seg6_store_bytes:
> return &bpf_lwt_seg6_store_bytes_proto;
> case BPF_FUNC_lwt_seg6_action:
> return &bpf_lwt_seg6_action_proto;
[Severity: High]
This is a pre-existing issue, but do these LWT-specific SRv6 helpers also
leave srh_state->srh dangling if an error occurs after skb->head is
reallocated?
For instance, in bpf_lwt_seg6_action(), if bpf_push_seg6_encap() fails, the
error path skips bpf_update_srh_state(), leaving the cached SRH pointer
dangling if the head was already reallocated:
net/core/filter.c:bpf_lwt_seg6_action() {
...
err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6,
param, param_len);
if (!err)
bpf_update_srh_state(skb);
return err;
}
A similar pattern appears in bpf_lwt_seg6_adjust_srh() where an early return
on ipv6_find_hdr() failure misses updating srh_state->srh:
net/core/filter.c:bpf_lwt_seg6_adjust_srh() {
...
if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
return -EINVAL;
srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
...
}
If the BPF program ignores the error and exits with BPF_OK, does this allow
input_action_end_bpf() to call seg6_bpf_has_valid_srh() and write to the
dangling pointer causing kernel memory corruption?
> case BPF_FUNC_lwt_seg6_adjust_srh:
> return &bpf_lwt_seg6_adjust_srh_proto;
[Severity: High]
This is another pre-existing issue, but does bpf_lwt_seg6_adjust_srh()
corrupt cloned SKB data when shrinking the header?
When len is less than 0, the code skips skb_cow_head() and directly
modifies the SKB via memmove():
net/core/filter.c:bpf_lwt_seg6_adjust_srh() {
...
} else {
ret = bpf_skb_net_hdr_pop(skb, offset, -1 * len);
}
...
}
If an LWT_SEG6LOCAL BPF program is executed on a cloned SKB,
bpf_skb_net_hdr_pop() verifies the buffer via pskb_may_pull() which
returns 1 without uncloning because the target data is already within the
linear portion.
Will bpf_skb_generic_pop() then use memmove() to modify skb->data,
corrupting the data buffer for other users of the cloned SKB?
> #endif
> default:
> return lwt_out_func_proto(func_id, prog);
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909040807.3885815-2-bestswngs@gmail.com?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL
2026-09-09 4:08 ` [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL Weiming Shi
2026-09-09 4:27 ` sashiko-bot
@ 2026-09-09 18:18 ` Emil Tsalapatis
2026-09-09 18:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 8+ messages in thread
From: Emil Tsalapatis @ 2026-09-09 18:18 UTC (permalink / raw)
To: Weiming Shi
Cc: Daniel Borkmann, John Fastabend, Stanislav Fomichev,
Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, Ihor Solodrai, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, bpf,
netdev, linux-kernel, David Lebrun, Mathieu Xhonneux,
co+adfca3e91be95776, Xiang Mei, Alexei Starovoitov, stable
On Wed, Sep 9, 2026 at 12:08 AM Weiming Shi <bestswngs@gmail.com> 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.
>
> Disallow bpf_skb_pull_data() for LWT_SEG6LOCAL programs so the verifier
> rejects this unsafe helper combination. Other LWT program types continue
> to expose the helper through lwt_out_func_proto().
>
> Fixes: 004d4b274e2a ("ipv6: sr: Add seg6local action End.BPF")
> Reported-by: co+adfca3e91be95776@bugs.sh
> Closes: https://lore.kernel.org/all/GCy0KRM2IcQGoJQTjJEU9D0maBxXzEDHuQpq@bugs.sh/
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Link: https://lore.kernel.org/bpf/DL9COXZQXX4V.1FN45QO2Q77ZH@gmail.com/
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:gpt-5
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
> Changes in v2:
> - Disallow bpf_skb_pull_data() for LWT_SEG6LOCAL instead of adding a
> wrapper to refresh the cached SRH pointer, as suggested by Alexei.
>
> net/core/filter.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 8513167a858a8..2a84f9d011314 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -9044,6 +9044,8 @@ static const struct bpf_func_proto *
> lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> {
> switch (func_id) {
> + case BPF_FUNC_skb_pull_data:
> + return NULL;
> #if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
> case BPF_FUNC_lwt_seg6_store_bytes:
> return &bpf_lwt_seg6_store_bytes_proto;
> --
> 2.55.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL
2026-09-09 4:08 ` [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL Weiming Shi
2026-09-09 4:27 ` sashiko-bot
2026-09-09 18:18 ` Emil Tsalapatis
@ 2026-09-09 18:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-09 18:40 UTC (permalink / raw)
To: Weiming Shi
Cc: daniel, john.fastabend, sdf, martin.lau, ast, andrii, eddyz87,
memxor, song, yonghong.song, jolsa, emil, ihor.solodrai, davem,
edumazet, kuba, pabeni, horms, bpf, netdev, linux-kernel, dlebrun,
m.xhonneux, co+adfca3e91be95776, xmei5, alexei.starovoitov,
stable
Hello:
This patch was applied to bpf/bpf.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Wed, 9 Sep 2026 12:08:08 +0800 you 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.
>
> Disallow bpf_skb_pull_data() for LWT_SEG6LOCAL programs so the verifier
> rejects this unsafe helper combination. Other LWT program types continue
> to expose the helper through lwt_out_func_proto().
>
> [...]
Here is the summary with links:
- [bpf,v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL
https://git.kernel.org/bpf/bpf/c/e4a62833adff
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-09 18:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07 19:21 [PATCH bpf] bpf: refresh seg6local SRH pointer after skb pull Weiming Shi
2026-09-07 19:44 ` sashiko-bot
2026-09-07 20:02 ` Alexei Starovoitov
2026-09-08 15:35 ` Weiming Shi
2026-09-09 4:08 ` [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL Weiming Shi
2026-09-09 4:27 ` sashiko-bot
2026-09-09 18:18 ` Emil Tsalapatis
2026-09-09 18:40 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox