bpf: refresh seg6local SRH pointer after skb pull

PATCH In review Weiming Shi 5 participants 8 messages Sep 7 – Sep 9

Subsystems (5)

Networking Network drivers Stable branch BPF +1 more subsystems

lore

Share & tools

lore

Weiming Shi

author

2026-09-07 19:22 UTC #0 lore raw

PATCH 1/1 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.

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 [email protected]

Closes https://lore.kernel.org/all/[email protected]/ open here

Cc [email protected]

Assisted-by Claude:gpt-5

Signed-off-by Weiming Shi <[email protected]>

---
 net/core/filter.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

net/core/filter.c +28 -0

index 61940e7535523..e61f9e9226b10 100644
@@ -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);

Signature · 1 line

2.55.0

[email protected]

AI review

2026-09-07 19:44 UTC #1 lore raw

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 <[email protected]>

bpf: refresh seg6local SRH pointer after skb pull

Closes https://lore.kernel.org/all/[email protected]/ open here

net/core/filter.c static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = { · line 7164

Quoted 8 lines

net/core/filter.c +0 -0

index 61940e7535523..e61f9e9226b10 100644
@@ -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?

Quoted 8 lines

net/core/filter.c +8 -0

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

net/core/filter.c static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = { · line 7182

Quoted 10 lines

net/core/filter.c +10 -0

+	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?

Quoted 9 lines

net/core/filter.c +8 -0

+
+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 */

[ ... ]

net/core/filter.c lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) · line 9085

Quoted 9 lines

net/core/filter.c +2 -0

@@ -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?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Alexei Starovoitov

2026-09-07 20:02 UTC #2 lore raw

In reply to Weiming Shi

Quoted 72 lines · Weiming Shi

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 [email protected]
Closes https://lore.kernel.org/all/[email protected]/ open here
Cc [email protected]
Assisted-by Claude:gpt-5
Signed-off-by Weiming Shi <[email protected]>
---
 net/core/filter.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
net/core/filter.c +26 -0
index 61940e7535523..e61f9e9226b10 100644
@@ -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

net/core/filter.c lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) · line 9082

Quoted 5 lines

net/core/filter.c +2 -0

@@ -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

Weiming Shi

author

2026-09-08 15:36 UTC #3 lore raw

Quoted 82 lines · Alexei Starovoitov <[email protected]> 于2026年9月8日周二 04:02写道：

Quoted 72 lines · Weiming Shi
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 [email protected]
Closes https://lore.kernel.org/all/[email protected]/ open here
Cc [email protected]
Assisted-by Claude:gpt-5
Signed-off-by Weiming Shi <[email protected]>
---
 net/core/filter.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
net/core/filter.c +26 -0
index 61940e7535523..e61f9e9226b10 100644
@@ -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
net/core/filter.c lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) · line 9082
Quoted 5 lines
net/core/filter.c +2 -0
@@ -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.

Quoted 1 line

pw-bot: cr

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

Weiming Shi

author

2026-09-09 04:08 UTC #4 lore raw

PATCH v2 1/1 bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL

In reply to Weiming Shi

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 [email protected]

Closes https://lore.kernel.org/all/[email protected]/ open here

Link https://lore.kernel.org/bpf/[email protected]/ open here

Cc [email protected]

Assisted-by Claude:gpt-5

Signed-off-by Weiming Shi <[email protected]>

---
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(+)

net/core/filter.c +2 -0

index 8513167a858a8..2a84f9d011314 100644
@@ -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;

Signature · 1 line

2.55.0

[email protected]

AI review

2026-09-09 04:27 UTC #5 lore raw

Re: [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL

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 <[email protected]>

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/[email protected]/ open here

Quoted 13 lines

net/core/filter.c +2 -0

index 8513167a858a8..2a84f9d011314 100644
@@ -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;

net/core/filter.c

Quoted 2 lines

net/core/filter.c +0 -0

 	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?

net/core/filter.c

Quoted 2 lines

net/core/filter.c +0 -0

 	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?

Quoted 5 lines

#endif

net/core/filter.c +0 -0

 	default:
 		return lwt_out_func_proto(func_id, prog);
 	}
 }

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Emil Tsalapatis

reviewer Reviewed-by on v2 1/1

2026-09-09 18:18 UTC #6 lore raw

Re: [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL

In reply to Weiming Shi

Quoted 18 lines · Weiming Shi

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 [email protected]
Closes https://lore.kernel.org/all/[email protected]/ open here
Suggested-by Alexei Starovoitov <[email protected]>
Link https://lore.kernel.org/bpf/[email protected]/ open here
Cc [email protected]
Assisted-by Claude:gpt-5
Signed-off-by Weiming Shi <[email protected]>

Quoted 1 line

---

Reviewed-by Emil Tsalapatis <[email protected]>

Quoted 22 lines

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(+)

net/core/filter.c +2 -1

index 8513167a858a8..2a84f9d011314 100644
@@ -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

[email protected]

CI Applied

2026-09-09 18:41 UTC #7 lore raw

Re: [PATCH bpf v2] bpf: disallow bpf_skb_pull_data() for LWT_SEG6LOCAL

In reply to Weiming Shi

This patch was applied to bpf/bpf.git (master) by Daniel Borkmann <[email protected]>:

Hello:

This patch was applied to bpf/bpf.git (master)
by Daniel Borkmann <[email protected]>:

Quoted 10 lines · 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!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html