* [PATCH bpf v2 01/11] bpf: Fix bounds check for skb-backed dynptrs
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 17:27 ` sashiko-bot
2026-09-22 20:15 ` Amery Hung
2026-09-22 17:20 ` [PATCH bpf v2 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
` (10 subsequent siblings)
11 siblings, 2 replies; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf
Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis,
Nicholas Carlini, Jiayuan Chen
The skb_pointer_if_linear() function checks whether a
memory region of length len starting at offset off into
the skb is in the linear area, and returns a pointer to
the region if so. The check currently subtracts between
skb_headlen and offset of the check, and since skb_headlen
is unsigned the subtraction can underflow. This causes the
bounds check to spuriously pass and generate an arbitrary
pointer of the form *(skb->data + off).
The only user of this helper is currently skb-backed BPF
dynptr code. Returning the wrong pointer leads to the
dynptr erroneously being backed with invalid memory.
Ensure the subtraction cannot underflow, and fail the check if
it would. Use u64 arithmetic to also prevent overflow when
calculating (skb_headlen(skb) - off) since off is unsigned.
Fixes: 6f5a630d7c57 ("bpf, net: Introduce skb_pointer_if_linear().")
Reported-by: Nicholas Carlini <nicholas@carlini.com>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
include/linux/skbuff.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 421f6fc45..c8e219030 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4372,7 +4372,10 @@ skb_header_pointer_careful(const struct sk_buff *skb, int offset,
static inline void * __must_check
skb_pointer_if_linear(const struct sk_buff *skb, int offset, int len)
{
- if (likely(skb_headlen(skb) - offset >= len))
+ unsigned int uoffset = (unsigned int)offset;
+
+ if (likely(uoffset <= skb_headlen(skb) &&
+ (unsigned int)len <= skb_headlen(skb) - uoffset))
return skb->data + offset;
return NULL;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 01/11] bpf: Fix bounds check for skb-backed dynptrs
2026-09-22 17:20 ` [PATCH bpf v2 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
@ 2026-09-22 17:27 ` sashiko-bot
2026-09-22 20:15 ` Amery Hung
1 sibling, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-22 17:27 UTC (permalink / raw)
To: Emil Tsalapatis; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: commit message claims u64 arithmetic but uses 32-bit unsigned
--
commit 5861693b35f3cce5042bdc052c43183ecc29425f
Author: Emil Tsalapatis <emil@etsalapatis.com>
bpf: Fix bounds check for skb-backed dynptrs
This commit fixes an underflow vulnerability in the bounds check for
skb-backed dynptrs within skb_pointer_if_linear(). It prevents spurious
passes by converting offsets and lengths to unsigned integers before
performing bounds comparisons.
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 421f6fc454511..c8e21903074c3 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -4372,7 +4372,10 @@ skb_header_pointer_careful(const struct sk_buff *skb, int offset,
> static inline void * __must_check
> skb_pointer_if_linear(const struct sk_buff *skb, int offset, int len)
> {
> - if (likely(skb_headlen(skb) - offset >= len))
> + unsigned int uoffset = (unsigned int)offset;
> +
> + if (likely(uoffset <= skb_headlen(skb) &&
> + (unsigned int)len <= skb_headlen(skb) - uoffset))
[Severity: Medium]
The commit message explicitly states "Use u64 arithmetic to also prevent
overflow when calculating (skb_headlen(skb) - off)", but this implementation
strictly uses 32-bit unsigned arithmetic.
Since the 32-bit unsigned math appears to correctly prevent the underflow and
overflow in this context, is the commit message's reference to u64 arithmetic
inaccurate? Does the commit message need to be updated to match the
unsigned int implementation here?
> return skb->data + offset;
> return NULL;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260922172028.6269-1-emil@etsalapatis.com?part=1
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 01/11] bpf: Fix bounds check for skb-backed dynptrs
2026-09-22 17:20 ` [PATCH bpf v2 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
2026-09-22 17:27 ` sashiko-bot
@ 2026-09-22 20:15 ` Amery Hung
1 sibling, 0 replies; 24+ messages in thread
From: Amery Hung @ 2026-09-22 20:15 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev,
Nicholas Carlini, Jiayuan Chen
On Tue, Sep 22, 2026 at 10:21 AM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>
> The skb_pointer_if_linear() function checks whether a
> memory region of length len starting at offset off into
> the skb is in the linear area, and returns a pointer to
> the region if so. The check currently subtracts between
> skb_headlen and offset of the check, and since skb_headlen
> is unsigned the subtraction can underflow. This causes the
> bounds check to spuriously pass and generate an arbitrary
> pointer of the form *(skb->data + off).
>
> The only user of this helper is currently skb-backed BPF
> dynptr code. Returning the wrong pointer leads to the
> dynptr erroneously being backed with invalid memory.
>
> Ensure the subtraction cannot underflow, and fail the check if
> it would. Use u64 arithmetic to also prevent overflow when
> calculating (skb_headlen(skb) - off) since off is unsigned.
>
> Fixes: 6f5a630d7c57 ("bpf, net: Introduce skb_pointer_if_linear().")
> Reported-by: Nicholas Carlini <nicholas@carlini.com>
> Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
Reviewed-by: Amery Hung <ameryhung@gmail.com>
> ---
> include/linux/skbuff.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 421f6fc45..c8e219030 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -4372,7 +4372,10 @@ skb_header_pointer_careful(const struct sk_buff *skb, int offset,
> static inline void * __must_check
> skb_pointer_if_linear(const struct sk_buff *skb, int offset, int len)
> {
> - if (likely(skb_headlen(skb) - offset >= len))
> + unsigned int uoffset = (unsigned int)offset;
> +
> + if (likely(uoffset <= skb_headlen(skb) &&
> + (unsigned int)len <= skb_headlen(skb) - uoffset))
> return skb->data + offset;
> return NULL;
> }
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf v2 02/11] selftests/bpf: Test dynptr slices past end of skb
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 17:28 ` sashiko-bot
2026-09-22 20:16 ` Amery Hung
2026-09-22 17:20 ` [PATCH bpf v2 03/11] bpf: Fix bpf_sock context code generation Emil Tsalapatis
` (9 subsequent siblings)
11 siblings, 2 replies; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis
Add a selftest to ensure dynptr slices cannot include
past the end of the linear area of an skb.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
.../testing/selftests/bpf/prog_tests/dynptr.c | 10 ++++++++++
.../selftests/bpf/progs/dynptr_success.c | 20 +++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c
index 5fda11590..439656036 100644
--- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
+++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
@@ -9,6 +9,7 @@
enum test_setup_type {
SETUP_SYSCALL_SLEEP,
SETUP_SKB_PROG,
+ SETUP_SKB_PROG_NONLINEAR,
SETUP_SKB_PROG_TP,
SETUP_XDP_PROG,
};
@@ -32,6 +33,7 @@ static struct {
{"test_ringbuf", SETUP_SYSCALL_SLEEP},
{"test_skb_readonly", SETUP_SKB_PROG},
{"test_dynptr_skb_data", SETUP_SKB_PROG},
+ {"test_dynptr_skb_slice_non_linear", SETUP_SKB_PROG_NONLINEAR},
{"test_dynptr_skb_meta_data", SETUP_SKB_PROG},
{"test_dynptr_skb_meta_flags", SETUP_SKB_PROG},
{"test_adjust", SETUP_SYSCALL_SLEEP},
@@ -94,7 +96,9 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ
bpf_link__destroy(link);
break;
case SETUP_SKB_PROG:
+ case SETUP_SKB_PROG_NONLINEAR:
{
+ struct __sk_buff ctx = {};
int prog_fd;
char buf[64];
@@ -106,6 +110,12 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ
.repeat = 1,
);
+ if (setup_type == SETUP_SKB_PROG_NONLINEAR) {
+ ctx.data_end = ETH_HLEN + sizeof(struct iphdr);
+ topts.ctx_in = &ctx;
+ topts.ctx_size_in = sizeof(ctx);
+ }
+
prog_fd = bpf_program__fd(prog);
if (!ASSERT_GE(prog_fd, 0, "prog_fd"))
goto cleanup;
diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c
index e0745b6e4..b668ebd61 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_success.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
@@ -10,6 +10,7 @@
#include "errno.h"
#define PAGE_SIZE_64K 65536
+#define TEST_SKB_LINEAR_SIZE (sizeof(struct ethhdr) + sizeof(struct iphdr))
char _license[] SEC("license") = "GPL";
@@ -211,6 +212,25 @@ int test_dynptr_skb_data(struct __sk_buff *skb)
return 1;
}
+SEC("?tc")
+int test_dynptr_skb_slice_non_linear(struct __sk_buff *skb)
+{
+ struct bpf_dynptr ptr;
+ void *data;
+
+ if (bpf_dynptr_from_skb(skb, 0, &ptr)) {
+ err = 1;
+ return 1;
+ }
+
+ /* Ensure we cannot read past the end of the buffer. */
+ data = bpf_dynptr_slice(&ptr, TEST_SKB_LINEAR_SIZE + 1, NULL, 1);
+ if (data)
+ err = 2;
+
+ return 1;
+}
+
SEC("?tc")
int test_dynptr_skb_meta_data(struct __sk_buff *skb)
{
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 02/11] selftests/bpf: Test dynptr slices past end of skb
2026-09-22 17:20 ` [PATCH bpf v2 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
@ 2026-09-22 17:28 ` sashiko-bot
2026-09-22 20:16 ` Amery Hung
1 sibling, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-22 17:28 UTC (permalink / raw)
To: Emil Tsalapatis; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] test: off-by-one boundary skip in bpf_dynptr_slice test
--
commit b231913ac8efedbed65769b0e27691d0098590c1
Author: Emil Tsalapatis <emil@etsalapatis.com>
selftests/bpf: Test dynptr slices past end of skb
Add a selftest to ensure dynptr slices cannot include
past the end of the linear area of an skb.
> diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c
> index e0745b6e467ea..b668ebd61fc79 100644
> --- a/tools/testing/selftests/bpf/progs/dynptr_success.c
> +++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
[ ... ]
> @@ -211,6 +212,25 @@ int test_dynptr_skb_data(struct __sk_buff *skb)
> return 1;
> }
>
> +SEC("?tc")
> +int test_dynptr_skb_slice_non_linear(struct __sk_buff *skb)
> +{
> + struct bpf_dynptr ptr;
> + void *data;
> +
> + if (bpf_dynptr_from_skb(skb, 0, &ptr)) {
> + err = 1;
> + return 1;
> + }
> +
> + /* Ensure we cannot read past the end of the buffer. */
> + data = bpf_dynptr_slice(&ptr, TEST_SKB_LINEAR_SIZE + 1, NULL, 1);
[Severity: Low]
Does this skip the exact linear/non-linear boundary byte by adding 1 here?
Since the linear portion's size is exactly TEST_SKB_LINEAR_SIZE (34 bytes), the
first byte of the non-linear area is at index 34. By using
TEST_SKB_LINEAR_SIZE + 1 (index 35), this avoids testing the boundary directly.
If an off-by-one error existed where index 34 was incorrectly treated as linear,
this test might fail to catch it.
> + if (data)
> + err = 2;
> +
> + return 1;
> +}
> +
> SEC("?tc")
> int test_dynptr_skb_meta_data(struct __sk_buff *skb)
> {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260922172028.6269-1-emil@etsalapatis.com?part=2
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 02/11] selftests/bpf: Test dynptr slices past end of skb
2026-09-22 17:20 ` [PATCH bpf v2 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
2026-09-22 17:28 ` sashiko-bot
@ 2026-09-22 20:16 ` Amery Hung
1 sibling, 0 replies; 24+ messages in thread
From: Amery Hung @ 2026-09-22 20:16 UTC (permalink / raw)
To: Emil Tsalapatis; +Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev
On Tue, Sep 22, 2026 at 11:37 AM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>
> Add a selftest to ensure dynptr slices cannot include
> past the end of the linear area of an skb.
>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
> .../testing/selftests/bpf/prog_tests/dynptr.c | 10 ++++++++++
> .../selftests/bpf/progs/dynptr_success.c | 20 +++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c
> index 5fda11590..439656036 100644
> --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
> +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
> @@ -9,6 +9,7 @@
> enum test_setup_type {
> SETUP_SYSCALL_SLEEP,
> SETUP_SKB_PROG,
> + SETUP_SKB_PROG_NONLINEAR,
> SETUP_SKB_PROG_TP,
> SETUP_XDP_PROG,
> };
> @@ -32,6 +33,7 @@ static struct {
> {"test_ringbuf", SETUP_SYSCALL_SLEEP},
> {"test_skb_readonly", SETUP_SKB_PROG},
> {"test_dynptr_skb_data", SETUP_SKB_PROG},
> + {"test_dynptr_skb_slice_non_linear", SETUP_SKB_PROG_NONLINEAR},
> {"test_dynptr_skb_meta_data", SETUP_SKB_PROG},
> {"test_dynptr_skb_meta_flags", SETUP_SKB_PROG},
> {"test_adjust", SETUP_SYSCALL_SLEEP},
> @@ -94,7 +96,9 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ
> bpf_link__destroy(link);
> break;
> case SETUP_SKB_PROG:
> + case SETUP_SKB_PROG_NONLINEAR:
> {
> + struct __sk_buff ctx = {};
> int prog_fd;
> char buf[64];
>
> @@ -106,6 +110,12 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ
> .repeat = 1,
> );
>
> + if (setup_type == SETUP_SKB_PROG_NONLINEAR) {
> + ctx.data_end = ETH_HLEN + sizeof(struct iphdr);
> + topts.ctx_in = &ctx;
> + topts.ctx_size_in = sizeof(ctx);
> + }
> +
> prog_fd = bpf_program__fd(prog);
> if (!ASSERT_GE(prog_fd, 0, "prog_fd"))
> goto cleanup;
> diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c
> index e0745b6e4..b668ebd61 100644
> --- a/tools/testing/selftests/bpf/progs/dynptr_success.c
> +++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
> @@ -10,6 +10,7 @@
> #include "errno.h"
>
> #define PAGE_SIZE_64K 65536
> +#define TEST_SKB_LINEAR_SIZE (sizeof(struct ethhdr) + sizeof(struct iphdr))
>
> char _license[] SEC("license") = "GPL";
>
> @@ -211,6 +212,25 @@ int test_dynptr_skb_data(struct __sk_buff *skb)
> return 1;
> }
>
> +SEC("?tc")
> +int test_dynptr_skb_slice_non_linear(struct __sk_buff *skb)
> +{
> + struct bpf_dynptr ptr;
> + void *data;
> +
> + if (bpf_dynptr_from_skb(skb, 0, &ptr)) {
> + err = 1;
> + return 1;
> + }
> +
> + /* Ensure we cannot read past the end of the buffer. */
nit: maybe "end of the linear area of an skb" to be clear?
Reviewed-by: Amery Hung <ameryhung@gmail.com>
> + data = bpf_dynptr_slice(&ptr, TEST_SKB_LINEAR_SIZE + 1, NULL, 1);
> + if (data)
> + err = 2;
> +
> + return 1;
> +}
> +
> SEC("?tc")
> int test_dynptr_skb_meta_data(struct __sk_buff *skb)
> {
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf v2 03/11] bpf: Fix bpf_sock context code generation
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access Emil Tsalapatis
` (8 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf
Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis,
Nicholas Carlini, Jiayuan Chen
Currently, the ctx access code reads the rx_queue_mapping
field with either a 4-byte or 2-byte load. The rest of the bits
in the register are marked known zero by the verifier. However,
the emitted ctx access code places in the register on certain
the special value (-1) using BPF_MOV_IMM64, which gets sign-extended
to turn on all the bits in the register. By shifting this value right,
the program ends up with a value at runtime above what the verifier
assumes is possible.
Fix this by ensuring the read value is as wide as the assumed size.
Use MOV32 instructions instead of MOV64 instructions to keep
the upper bits zero as assumed by the verifier. Also properly report
the size of the destination variable (the bpf_sock field, 4 bytes) instead
of the source (the socket field, 2 bytes).
Fixes: c3c16f2ea6d2 ("bpf: Add rx_queue_mapping to bpf_sock")
Reported-by: Nicholas Carlini <nicholas@carlini.com>
Suggested-by: Nicholas Carlini <nicholas@carlini.com>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
net/core/filter.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 5feb99884..eb0d33fb4 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10577,11 +10577,12 @@ u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
target_size));
*insn++ = BPF_JMP_IMM(BPF_JNE, si->dst_reg, NO_QUEUE_MAPPING,
1);
- *insn++ = BPF_MOV64_IMM(si->dst_reg, -1);
+ *insn++ = BPF_MOV32_IMM(si->dst_reg, -1);
#else
- *insn++ = BPF_MOV64_IMM(si->dst_reg, -1);
- *target_size = 2;
+ *insn++ = BPF_MOV32_IMM(si->dst_reg, -1);
#endif
+ *target_size = sizeof_field(struct bpf_sock, rx_queue_mapping);
+
break;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf v2 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
` (2 preceding siblings ...)
2026-09-22 17:20 ` [PATCH bpf v2 03/11] bpf: Fix bpf_sock context code generation Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 17:36 ` sashiko-bot
2026-09-22 17:20 ` [PATCH bpf v2 05/11] bpf: Reject pkt arguments in mutating subprogs Emil Tsalapatis
` (7 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis
Add tests to ensure the verifier properly tracks the 0 bit state
and width of the rx_queue_mapping field read from struct sock.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
.../selftests/bpf/progs/verifier_sock.c | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c
index 2a136c917..565c4d8bc 100644
--- a/tools/testing/selftests/bpf/progs/verifier_sock.c
+++ b/tools/testing/selftests/bpf/progs/verifier_sock.c
@@ -88,6 +88,44 @@ l0_%=: r0 = *(u32*)(r1 + %[bpf_sock_family]); \
: __clobber_all);
}
+SEC("socket")
+__description("skb->sk: sk->rx_queue_mapping [no sign extension]")
+__success __success_unpriv __retval(0)
+__naked void sk_rx_queue_mapping_no_sign_ext(void)
+{
+ asm volatile (" \
+ r1 = *(u64*)(r1 + %[__sk_buff_sk]); \
+ if r1 != 0 goto l0_%=; \
+ r0 = 0xdead; \
+ exit; \
+l0_%=: r0 = *(u32*)(r1 + %[bpf_sock_rx_queue_mapping]); \
+ r0 >>= 32; \
+ exit; \
+" :
+ : __imm_const(__sk_buff_sk, offsetof(struct __sk_buff, sk)),
+ __imm_const(bpf_sock_rx_queue_mapping, offsetof(struct bpf_sock, rx_queue_mapping))
+ : __clobber_all);
+}
+
+SEC("socket")
+__description("skb->sk: sk->rx_queue_mapping [narrow load mask]")
+__success __success_unpriv __retval(0)
+__naked void sk_rx_queue_mapping_narrow_load_mask(void)
+{
+ asm volatile (" \
+ r1 = *(u64*)(r1 + %[__sk_buff_sk]); \
+ if r1 != 0 goto l0_%=; \
+ r0 = 0xdead; \
+ exit; \
+l0_%=: r0 = *(u16*)(r1 + %[bpf_sock_rx_queue_mapping]); \
+ r0 >>= 16; \
+ exit; \
+" :
+ : __imm_const(__sk_buff_sk, offsetof(struct __sk_buff, sk)),
+ __imm_const(bpf_sock_rx_queue_mapping, offsetof(struct bpf_sock, rx_queue_mapping))
+ : __clobber_all);
+}
+
SEC("cgroup/skb")
__description("skb->sk: sk->type [fullsock field]")
__failure __msg("invalid sock_common access")
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access
2026-09-22 17:20 ` [PATCH bpf v2 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access Emil Tsalapatis
@ 2026-09-22 17:36 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-22 17:36 UTC (permalink / raw)
To: Emil Tsalapatis; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: selftests fail to verify bounds tracking of rx_queue_mapping
--
commit 90d4583595dcbfcce26a9758d799968a757439e5
Author: Emil Tsalapatis <emil@etsalapatis.com>
selftests/bpf: Add selftests for rx_queue_mapping context access
This commit adds selftests for the rx_queue_mapping field read from
struct sock to ensure the verifier properly tracks its 0 bit state and
width.
> diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c
> index 2a136c917680f..565c4d8bc3fa3 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_sock.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_sock.c
[ ... ]
> +SEC("socket")
> +__description("skb->sk: sk->rx_queue_mapping [no sign extension]")
> +__success __success_unpriv __retval(0)
> +__naked void sk_rx_queue_mapping_no_sign_ext(void)
> +{
> + asm volatile (" \
> + r1 = *(u64*)(r1 + %[__sk_buff_sk]); \
> + if r1 != 0 goto l0_%=; \
> + r0 = 0xdead; \
> + exit; \
> +l0_%=: r0 = *(u32*)(r1 + %[bpf_sock_rx_queue_mapping]); \
> + r0 >>= 32; \
> + exit; \
[Severity: Medium]
Does this sequence actually verify the verifier's 0-bit state tracking
of rx_queue_mapping?
The program shifts the loaded value right by 32 and exits. The verifier
always accepts returning an unknown initialized scalar in this context. At
runtime, the CPU zero-extends the value, so it always evaluates to 0 and
passes the __retval(0) check, regardless of what the verifier's static
analysis concluded about the bounds.
> +" :
> + : __imm_const(__sk_buff_sk, offsetof(struct __sk_buff, sk)),
> + __imm_const(bpf_sock_rx_queue_mapping, offsetof(struct bpf_sock, rx_queue_mapping))
> + : __clobber_all);
> +}
> +
> +SEC("socket")
> +__description("skb->sk: sk->rx_queue_mapping [narrow load mask]")
> +__success __success_unpriv __retval(0)
> +__naked void sk_rx_queue_mapping_narrow_load_mask(void)
> +{
> + asm volatile (" \
> + r1 = *(u64*)(r1 + %[__sk_buff_sk]); \
> + if r1 != 0 goto l0_%=; \
> + r0 = 0xdead; \
> + exit; \
> +l0_%=: r0 = *(u16*)(r1 + %[bpf_sock_rx_queue_mapping]); \
> + r0 >>= 16; \
> + exit; \
[Severity: Medium]
Similarly, does this narrow load test pass at runtime without strictly
asserting the verifier's internal bounds tracking?
Like the previous function, this shifts the 16-bit load right by 16 and
exits. It seems this will also implicitly pass at runtime rather than
causing a verification failure if bounds tracking regressions occur.
> +" :
> + : __imm_const(__sk_buff_sk, offsetof(struct __sk_buff, sk)),
> + __imm_const(bpf_sock_rx_queue_mapping, offsetof(struct bpf_sock, rx_queue_mapping))
> + : __clobber_all);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260922172028.6269-1-emil@etsalapatis.com?part=4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf v2 05/11] bpf: Reject pkt arguments in mutating subprogs
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
` (3 preceding siblings ...)
2026-09-22 17:20 ` [PATCH bpf v2 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 20:32 ` Amery Hung
2026-09-22 17:20 ` [PATCH bpf v2 06/11] selftests/bpf: Test rejection of pkt args to " Emil Tsalapatis
` (6 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf
Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis,
Nicholas Carlini
The verifier tracks changes in how PTR_TO_PACKET registers'
bounds are modified across subprog boundaries. PTR_TO_PACKET
registers are actually passed as PTR_TO_MEM, which is assumed
valid for the entire call. This is not the case with packet memory,
where a pskb_* call may invalidate its memory region.
Reject BPF code that passes PTR_TO_PACKET pointers to subprogs that
may mutate a packet. We cannot pass the pointer as a true PTR_TO_PACKET
because we would also need to somehow pass the PTR_TO_PACKET_META
or PTR_TO_PACKET_END to the subprog. Since we cannot avoid representing
the pointer in the subprog as PTR_TO_MEM, only permit it if the
subprog is guaranteed not to mutate the packet.
Fixes: 80f281664f5a ("bpf: Support pointers in global func args")
Reported-by: Nicholas Carlini <nicholas@carlini.com>
Suggested-by: Nicholas Carlini <nicholas@carlini.com>
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
kernel/bpf/verifier.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d62c0f74c..71ad07a9d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10421,6 +10421,16 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
if (check_mem_reg(env, reg, argno, arg->mem_size, BPF_READ | BPF_WRITE, NULL,
NULL))
return -EINVAL;
+ /*
+ * PTR_TO_PACKET get passed as PTR_TO_MEM, preventing
+ * us from adjusting bounds tracking info.
+ */
+ if ((reg_is_pkt_pointer_any(reg) || reg_is_dynptr_slice_pkt(reg)) &&
+ sub->changes_pkt_data) {
+ bpf_log(log, "%s is a packet pointer, but func#%d may change packet data\n",
+ reg_arg_name(env, argno), subprog);
+ return -EINVAL;
+ }
if (!(arg->arg_type & PTR_MAYBE_NULL) &&
(type_may_be_null(reg->type) || bpf_register_is_null(reg))) {
bpf_log(log, "%s is expected to be non-NULL\n",
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 05/11] bpf: Reject pkt arguments in mutating subprogs
2026-09-22 17:20 ` [PATCH bpf v2 05/11] bpf: Reject pkt arguments in mutating subprogs Emil Tsalapatis
@ 2026-09-22 20:32 ` Amery Hung
0 siblings, 0 replies; 24+ messages in thread
From: Amery Hung @ 2026-09-22 20:32 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev,
Nicholas Carlini
On Tue, Sep 22, 2026 at 10:24 AM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>
> The verifier tracks changes in how PTR_TO_PACKET registers'
> bounds are modified across subprog boundaries. PTR_TO_PACKET
> registers are actually passed as PTR_TO_MEM, which is assumed
> valid for the entire call. This is not the case with packet memory,
> where a pskb_* call may invalidate its memory region.
>
> Reject BPF code that passes PTR_TO_PACKET pointers to subprogs that
> may mutate a packet. We cannot pass the pointer as a true PTR_TO_PACKET
> because we would also need to somehow pass the PTR_TO_PACKET_META
> or PTR_TO_PACKET_END to the subprog. Since we cannot avoid representing
> the pointer in the subprog as PTR_TO_MEM, only permit it if the
> subprog is guaranteed not to mutate the packet.
>
> Fixes: 80f281664f5a ("bpf: Support pointers in global func args")
> Reported-by: Nicholas Carlini <nicholas@carlini.com>
> Suggested-by: Nicholas Carlini <nicholas@carlini.com>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
> kernel/bpf/verifier.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d62c0f74c..71ad07a9d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10421,6 +10421,16 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
> if (check_mem_reg(env, reg, argno, arg->mem_size, BPF_READ | BPF_WRITE, NULL,
> NULL))
> return -EINVAL;
> + /*
> + * PTR_TO_PACKET get passed as PTR_TO_MEM, preventing
> + * us from adjusting bounds tracking info.
> + */
> + if ((reg_is_pkt_pointer_any(reg) || reg_is_dynptr_slice_pkt(reg)) &&
> + sub->changes_pkt_data) {
> + bpf_log(log, "%s is a packet pointer, but func#%d may change packet data\n",
This might seem vague to users. How about something like:
bpf_log(log,
"cannot pass packet pointer %s to %s(). The function may
invalidate packet memory\n",
reg_arg_name(env, argno), bpf_subprog_name(env, subprog));
Reviewed-by: Amery Hung <ameryhung@gmail.com>
> + reg_arg_name(env, argno), subprog);
> + return -EINVAL;
> + }
> if (!(arg->arg_type & PTR_MAYBE_NULL) &&
> (type_may_be_null(reg->type) || bpf_register_is_null(reg))) {
> bpf_log(log, "%s is expected to be non-NULL\n",
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf v2 06/11] selftests/bpf: Test rejection of pkt args to mutating subprogs
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
` (4 preceding siblings ...)
2026-09-22 17:20 ` [PATCH bpf v2 05/11] bpf: Reject pkt arguments in mutating subprogs Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 07/11] bpf: Prevent variable arena/non-arena register contents Emil Tsalapatis
` (5 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis
Add a selftests that ensures that PTR_TO_PACKET arguments can
only be passed to subprogs that will never adjust the underlying
packet memory, and are rejected otherwise.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
.../bpf/progs/verifier_global_ptr_args.c | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
index b277b1efb..10019af5e 100644
--- a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
+++ b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
@@ -350,4 +350,57 @@ int anything_to_untrusted_mem(void *ctx)
return 0;
}
+struct pkt_arg {
+ __u64 x;
+ __u8 pad[56];
+};
+
+__weak int subprog_pkt_ptr_no_change(struct pkt_arg *p)
+{
+ if (!p)
+ return 0;
+
+ return p->x;
+}
+
+SEC("?tc")
+__success
+int pkt_ptr_to_global_mem_arg_no_change(struct __sk_buff *skb)
+{
+ void *data = (void *)(long)skb->data;
+ void *data_end = (void *)(long)skb->data_end;
+ struct pkt_arg *p = data;
+
+ if ((void *)(p + 1) > data_end)
+ return 0;
+
+ return subprog_pkt_ptr_no_change(p);
+}
+
+__weak int subprog_pkt_ptr_changes_data(struct __sk_buff *skb __arg_ctx,
+ struct pkt_arg *p)
+{
+ if (!p)
+ return 0;
+
+ bpf_skb_pull_data(skb, 0);
+ return p->x;
+}
+
+SEC("?tc")
+__failure __log_level(2)
+__msg("R2 is a packet pointer, but func#{{[0-9]+}} may change packet data")
+__msg("Caller passes invalid args into func#{{[0-9]+}} ('subprog_pkt_ptr_changes_data')")
+int pkt_ptr_to_global_mem_arg_changes_data(struct __sk_buff *skb)
+{
+ void *data = (void *)(long)skb->data;
+ void *data_end = (void *)(long)skb->data_end;
+ struct pkt_arg *p = data;
+
+ if ((void *)(p + 1) > data_end)
+ return 0;
+
+ return subprog_pkt_ptr_changes_data(skb, p);
+}
+
char _license[] SEC("license") = "GPL";
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf v2 07/11] bpf: Prevent variable arena/non-arena register contents
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
` (5 preceding siblings ...)
2026-09-22 17:20 ` [PATCH bpf v2 06/11] selftests/bpf: Test rejection of pkt args to " Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 08/11] selftests/bpf: Test for mixed arena/nonarena code paths Emil Tsalapatis
` (4 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf
Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis,
Nicholas Carlini
The verifier marks ALU instructions that include at least
one arena operand with needs_zext: These instructions are
fixed up after verification to be ALU32 instructions to
ensure that the result is a valid offset into an arena.
However, different code paths may provide two non-arena
64-bit arguments to the same instruction. The result of
the operation in that code path is wrong, since it is
now unexpectedly truncated to 32 bits and zero-extended.
Add logic to the verifier to ensure every instruction either
always has at least one PTR_TO_ARENA argument, or never does.
Since needs_zext already tracks the first scenario, add a
prevent_zext field in bpf_insn_aux to track the latter.
Reject instructions that use arena arguments and have prevent_zext
set, or do not have arena arguments and have needs_zext set.
Fixes: 6082b6c328b5 ("bpf: Recognize addr_space_cast instruction in the verifier.")
Reported-by: Nicholas Carlini <nicholas@carlini.com>
Suggested-by: Nicholas Carlini <nicholas@carlini.com>
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
include/linux/bpf_verifier.h | 1 +
kernel/bpf/verifier.c | 24 +++++++++++++++++++++++-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 92f528c45..be0ccad15 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -679,6 +679,7 @@ struct bpf_insn_aux_data {
bool nospec_result; /* result is unsafe under speculation, nospec must follow */
bool zext_dst; /* this insn zero extends dst reg */
bool needs_zext; /* alu op needs to clear upper bits */
+ bool prevent_zext; /* alu op cannot be zext (already used with 64-bit scalars) */
bool non_sleepable; /* helper/kfunc may be called from non-sleepable context */
bool is_iter_next; /* bpf_iter_<type>_next() kfunc call */
bool call_with_percpu_alloc_ptr; /* {this,per}_cpu_ptr() with prog percpu alloc */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 71ad07a9d..cebed6c9d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -16130,6 +16130,7 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
struct bpf_reg_state *regs = state->regs, *dst_reg, *src_reg;
struct bpf_reg_state *ptr_reg = NULL, off_reg = {0};
bool alu32 = (BPF_CLASS(insn->code) != BPF_ALU64);
+ struct bpf_insn_aux_data *aux = cur_aux(env);
u8 opcode = BPF_OP(insn->code);
int err;
@@ -16141,12 +16142,23 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
/* Case where at least one operand is an arena. */
if (dst_reg->type == PTR_TO_ARENA || (src_reg && src_reg->type == PTR_TO_ARENA)) {
- struct bpf_insn_aux_data *aux = cur_aux(env);
if (dst_reg->type != PTR_TO_ARENA)
*dst_reg = *src_reg;
if (BPF_CLASS(insn->code) == BPF_ALU64) {
+ /*
+ * Only arena pointers set needs_zext, but doing so
+ * modifies the instruction at fixup time to an ALU32
+ * and makes it unsuitable for 64-bit scalar args. We
+ * prevent zext from being set if the instruction has
+ * been previously called with non-arena registers.
+ */
+ if (aux->prevent_zext) {
+ verbose(env, "same insn cannot be used with and without arena pointer\n");
+ return -EINVAL;
+ }
+
/*
* 32-bit operations zero upper bits automatically.
* 64-bit operations need to be converted to 32.
@@ -16159,6 +16171,16 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
return 0;
}
+ /* Prevent the instruction from being used with arena pointers (see above). */
+ if (env->prog->aux->arena && BPF_CLASS(insn->code) == BPF_ALU64) {
+ if (aux->needs_zext) {
+ verbose(env, "same insn cannot be used with and without arena pointer\n");
+ return -EINVAL;
+ }
+
+ aux->prevent_zext = true;
+ }
+
if (dst_reg->type != SCALAR_VALUE)
ptr_reg = dst_reg;
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf v2 08/11] selftests/bpf: Test for mixed arena/nonarena code paths
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
` (6 preceding siblings ...)
2026-09-22 17:20 ` [PATCH bpf v2 07/11] bpf: Prevent variable arena/non-arena register contents Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 09/11] bpf: Track whether dynptr type is known Emil Tsalapatis
` (3 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis
Add a selftest to confirm the verifier rejects ALU operations
that return arena or non-arena results depending on code path.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
.../selftests/bpf/progs/verifier_arena.c | 49 +++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/testing/selftests/bpf/progs/verifier_arena.c
index 332322c9b..1ce9c4b78 100644
--- a/tools/testing/selftests/bpf/progs/verifier_arena.c
+++ b/tools/testing/selftests/bpf/progs/verifier_arena.c
@@ -561,6 +561,55 @@ int arena_ptr_add_arena_ptr(void *ctx)
return 0;
}
+SEC("syscall")
+__failure __msg("same insn cannot be used with and without arena pointer")
+int mixed_arena_scalar_alu64_scalar_first(void *ctx)
+{
+ volatile register __u64 reg asm("r3");
+ __u32 pick_arena = bpf_get_prandom_u32();
+
+ reg = 1ULL << 32;
+
+ if (pick_arena) {
+ asm volatile (
+ "r9 = %[arena] ll;"
+ "%[reg] = 0;"
+ "%[reg] = addr_space_cast(%[reg], 0x0, 0x1);"
+ : [reg] "=r"(reg)
+ : __imm_addr(arena)
+ : "r9"
+ );
+ }
+
+ reg += 1;
+
+ return 0;
+}
+
+SEC("syscall")
+__failure __msg("same insn cannot be used with and without arena pointer")
+int mixed_arena_scalar_alu64_arena_first(void *ctx)
+{
+ volatile register __u64 reg asm("r3");
+ __u32 pick_scalar = bpf_get_prandom_u32();
+
+ asm volatile (
+ "r9 = %[arena] ll;"
+ "%[reg] = 0;"
+ "%[reg] = addr_space_cast(%[reg], 0x0, 0x1);"
+ : [reg] "=r"(reg)
+ : __imm_addr(arena)
+ : "r9"
+ );
+
+ if (pick_scalar)
+ reg = 1ULL << 32;
+
+ reg += 1;
+
+ return 0;
+}
+
SEC("syscall")
__success __retval(0)
int scalar_xor_arena_ptr(void *ctx)
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf v2 09/11] bpf: Track whether dynptr type is known
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
` (7 preceding siblings ...)
2026-09-22 17:20 ` [PATCH bpf v2 08/11] selftests/bpf: Test for mixed arena/nonarena code paths Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 18:46 ` Alexei Starovoitov
2026-09-22 17:20 ` [PATCH bpf v2 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs Emil Tsalapatis
` (2 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis
The TYPE_LOCAL dynptr type is used for two different
kinds of dynptrs in the codebase: Those that are created
locally and backed with a memory region, and those that
are passed as arguments to a global subprog, whose type
is not known at verification time. The two kinds require
different handling in certain scenarios, e.g., packet
pointer invalidation. However, there is no current way
to distinguish them.
Add a new field, type_unknown, to bpf_reg_state's dynptr-
specific state. The field designates whether the dynptr
type reported is accurate, or a placeholder for "type
unknown". Adding an extra field to bpf_reg_state avoids
unnecessarily splitting TYPE_LOCAL into two types, since
they would behave identically in most cases.
The change is currently non-functional. The new field is
first used in the next commit.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
include/linux/bpf_verifier.h | 2 ++
kernel/bpf/states.c | 1 +
kernel/bpf/verifier.c | 27 ++++++++++++++++++---------
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index be0ccad15..f57730d1d 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -71,6 +71,7 @@ struct bpf_reg_state {
/* For dynptr stack slots */
struct {
enum bpf_dynptr_type type;
+ bool type_unknown;
/* A dynptr is 16 bytes so it takes up 2 stack slots.
* We need to track which slot is the first slot
* to protect against cases where the user may try to
@@ -1531,6 +1532,7 @@ struct bpf_map_desc {
/* The last initialized dynptr; Populated by process_dynptr_func() */
struct bpf_dynptr_desc {
enum bpf_dynptr_type type;
+ bool type_unknown;
u32 id;
u32 parent_id;
};
diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 66fb11b6c..360aeb5da 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -795,6 +795,7 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
old_reg = &old->stack[spi].spilled_ptr;
cur_reg = &cur->stack[spi].spilled_ptr;
if (old_reg->dynptr.type != cur_reg->dynptr.type ||
+ old_reg->dynptr.type_unknown != cur_reg->dynptr.type_unknown ||
old_reg->dynptr.first_slot != cur_reg->dynptr.first_slot ||
!check_ids(old_reg->id, cur_reg->id, idmap) ||
!check_ids(old_reg->parent_id, cur_reg->parent_id, idmap))
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index cebed6c9d..da110cdbc 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -694,24 +694,26 @@ static bool dynptr_type_referenced(enum bpf_dynptr_type type)
static void __mark_dynptr_reg(struct bpf_reg_state *reg,
enum bpf_dynptr_type type,
- bool first_slot, int id, int parent_id);
+ bool first_slot, bool type_unknown,
+ int id, int parent_id);
static void mark_dynptr_stack_regs(struct bpf_verifier_env *env,
struct bpf_reg_state *sreg1,
struct bpf_reg_state *sreg2,
- enum bpf_dynptr_type type, int parent_id)
+ enum bpf_dynptr_type type,
+ bool type_unknown, int parent_id)
{
int id = ++env->id_gen;
- __mark_dynptr_reg(sreg1, type, true, id, parent_id);
- __mark_dynptr_reg(sreg2, type, false, id, parent_id);
+ __mark_dynptr_reg(sreg1, type, true, type_unknown, id, parent_id);
+ __mark_dynptr_reg(sreg2, type, false, type_unknown, id, parent_id);
}
static void mark_dynptr_cb_reg(struct bpf_verifier_env *env,
struct bpf_reg_state *reg,
enum bpf_dynptr_type type)
{
- __mark_dynptr_reg(reg, type, true, ++env->id_gen, 0);
+ __mark_dynptr_reg(reg, type, true, false, ++env->id_gen, 0);
}
static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env,
@@ -723,6 +725,7 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
{
struct bpf_func_state *state = bpf_func(env, reg);
int spi, i, err, parent_id = 0;
+ bool type_unknown = false;
enum bpf_dynptr_type type;
spi = dynptr_get_spi(env, reg);
@@ -778,10 +781,12 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
}
} else { /* bpf_dynptr_clone() */
parent_id = dynptr->parent_id;
+ type_unknown = dynptr->type_unknown;
}
mark_dynptr_stack_regs(env, &state->stack[spi].spilled_ptr,
- &state->stack[spi - 1].spilled_ptr, type, parent_id);
+ &state->stack[spi - 1].spilled_ptr, type,
+ type_unknown, parent_id);
return 0;
}
@@ -1951,7 +1956,8 @@ static void mark_reg_known_zero(struct bpf_verifier_env *env,
}
static void __mark_dynptr_reg(struct bpf_reg_state *reg, enum bpf_dynptr_type type,
- bool first_slot, int id, int parent_id)
+ bool first_slot, bool type_unknown,
+ int id, int parent_id)
{
/* reg->type has no meaning for STACK_DYNPTR, but when we set reg for
* callback arguments, it does need to be CONST_PTR_TO_DYNPTR, so simply
@@ -1963,6 +1969,7 @@ static void __mark_dynptr_reg(struct bpf_reg_state *reg, enum bpf_dynptr_type ty
reg->id = id;
reg->parent_id = parent_id;
reg->dynptr.type = type;
+ reg->dynptr.type_unknown = type_unknown;
reg->dynptr.first_slot = first_slot;
}
@@ -7801,6 +7808,7 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat
}
meta->dynptr.type = reg->dynptr.type;
+ meta->dynptr.type_unknown = reg->dynptr.type_unknown;
meta->dynptr.id = reg->id;
meta->dynptr.parent_id = reg->parent_id;
}
@@ -19963,8 +19971,9 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
reg->type = SCALAR_VALUE;
mark_reg_unknown(env, regs, i);
} else if (arg->arg_type == ARG_PTR_TO_DYNPTR) {
- /* assume unspecial LOCAL dynptr type */
- __mark_dynptr_reg(reg, BPF_DYNPTR_TYPE_LOCAL, true, ++env->id_gen, 0);
+ /* Global subprog args may be backed by any dynptr type. */
+ __mark_dynptr_reg(reg, BPF_DYNPTR_TYPE_LOCAL, true, true,
+ ++env->id_gen, 0);
} else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) {
reg->type = PTR_TO_MEM;
reg->type |= arg->arg_type &
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 09/11] bpf: Track whether dynptr type is known
2026-09-22 17:20 ` [PATCH bpf v2 09/11] bpf: Track whether dynptr type is known Emil Tsalapatis
@ 2026-09-22 18:46 ` Alexei Starovoitov
2026-09-22 20:23 ` Amery Hung
0 siblings, 1 reply; 24+ messages in thread
From: Alexei Starovoitov @ 2026-09-22 18:46 UTC (permalink / raw)
To: Emil Tsalapatis, bpf; +Cc: ast, andrii, eddyz87, memxor, daniel, netdev
On Tue Sep 22, 2026 at 5:20 PM UTC, Emil Tsalapatis wrote:
> The TYPE_LOCAL dynptr type is used for two different
> kinds of dynptrs in the codebase: Those that are created
> locally and backed with a memory region, and those that
> are passed as arguments to a global subprog, whose type
> is not known at verification time. The two kinds require
> different handling in certain scenarios, e.g., packet
> pointer invalidation. However, there is no current way
> to distinguish them.
>
> Add a new field, type_unknown, to bpf_reg_state's dynptr-
> specific state. The field designates whether the dynptr
> type reported is accurate, or a placeholder for "type
> unknown". Adding an extra field to bpf_reg_state avoids
> unnecessarily splitting TYPE_LOCAL into two types, since
> they would behave identically in most cases.
>
> The change is currently non-functional. The new field is
> first used in the next commit.
>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
> include/linux/bpf_verifier.h | 2 ++
> kernel/bpf/states.c | 1 +
> kernel/bpf/verifier.c | 27 ++++++++++++++++++---------
> 3 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index be0ccad15..f57730d1d 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -71,6 +71,7 @@ struct bpf_reg_state {
> /* For dynptr stack slots */
> struct {
> enum bpf_dynptr_type type;
> + bool type_unknown;
why extra bool? Can it be another value in enum?
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 09/11] bpf: Track whether dynptr type is known
2026-09-22 18:46 ` Alexei Starovoitov
@ 2026-09-22 20:23 ` Amery Hung
2026-09-22 20:28 ` Emil Tsalapatis
0 siblings, 1 reply; 24+ messages in thread
From: Amery Hung @ 2026-09-22 20:23 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Emil Tsalapatis, bpf, ast, andrii, eddyz87, memxor, daniel,
netdev
On Tue, Sep 22, 2026 at 11:49 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue Sep 22, 2026 at 5:20 PM UTC, Emil Tsalapatis wrote:
> > The TYPE_LOCAL dynptr type is used for two different
> > kinds of dynptrs in the codebase: Those that are created
> > locally and backed with a memory region, and those that
> > are passed as arguments to a global subprog, whose type
> > is not known at verification time. The two kinds require
> > different handling in certain scenarios, e.g., packet
> > pointer invalidation. However, there is no current way
> > to distinguish them.
> >
> > Add a new field, type_unknown, to bpf_reg_state's dynptr-
> > specific state. The field designates whether the dynptr
> > type reported is accurate, or a placeholder for "type
> > unknown". Adding an extra field to bpf_reg_state avoids
> > unnecessarily splitting TYPE_LOCAL into two types, since
> > they would behave identically in most cases.
> >
> > The change is currently non-functional. The new field is
> > first used in the next commit.
> >
> > Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> > ---
> > include/linux/bpf_verifier.h | 2 ++
> > kernel/bpf/states.c | 1 +
> > kernel/bpf/verifier.c | 27 ++++++++++++++++++---------
> > 3 files changed, 21 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> > index be0ccad15..f57730d1d 100644
> > --- a/include/linux/bpf_verifier.h
> > +++ b/include/linux/bpf_verifier.h
> > @@ -71,6 +71,7 @@ struct bpf_reg_state {
> > /* For dynptr stack slots */
> > struct {
> > enum bpf_dynptr_type type;
> > + bool type_unknown;
>
> why extra bool? Can it be another value in enum?
Second this. Could be a BPF_DYNPTR_TYPE_UNKNOWN or BPF_DYNPTR_TYPE_ANY.
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 09/11] bpf: Track whether dynptr type is known
2026-09-22 20:23 ` Amery Hung
@ 2026-09-22 20:28 ` Emil Tsalapatis
0 siblings, 0 replies; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 20:28 UTC (permalink / raw)
To: Amery Hung, Alexei Starovoitov
Cc: Emil Tsalapatis, bpf, ast, andrii, eddyz87, memxor, daniel,
netdev
On Tue Sep 22, 2026 at 8:23 PM UTC, Amery Hung wrote:
> On Tue, Sep 22, 2026 at 11:49 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Tue Sep 22, 2026 at 5:20 PM UTC, Emil Tsalapatis wrote:
>> > The TYPE_LOCAL dynptr type is used for two different
>> > kinds of dynptrs in the codebase: Those that are created
>> > locally and backed with a memory region, and those that
>> > are passed as arguments to a global subprog, whose type
>> > is not known at verification time. The two kinds require
>> > different handling in certain scenarios, e.g., packet
>> > pointer invalidation. However, there is no current way
>> > to distinguish them.
>> >
>> > Add a new field, type_unknown, to bpf_reg_state's dynptr-
>> > specific state. The field designates whether the dynptr
>> > type reported is accurate, or a placeholder for "type
>> > unknown". Adding an extra field to bpf_reg_state avoids
>> > unnecessarily splitting TYPE_LOCAL into two types, since
>> > they would behave identically in most cases.
>> >
>> > The change is currently non-functional. The new field is
>> > first used in the next commit.
>> >
>> > Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> > ---
>> > include/linux/bpf_verifier.h | 2 ++
>> > kernel/bpf/states.c | 1 +
>> > kernel/bpf/verifier.c | 27 ++++++++++++++++++---------
>> > 3 files changed, 21 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> > index be0ccad15..f57730d1d 100644
>> > --- a/include/linux/bpf_verifier.h
>> > +++ b/include/linux/bpf_verifier.h
>> > @@ -71,6 +71,7 @@ struct bpf_reg_state {
>> > /* For dynptr stack slots */
>> > struct {
>> > enum bpf_dynptr_type type;
>> > + bool type_unknown;
>>
>> why extra bool? Can it be another value in enum?
>
> Second this. Could be a BPF_DYNPTR_TYPE_UNKNOWN or BPF_DYNPTR_TYPE_ANY.
Sounds good, let's go with that. The reasoning behind the extra bool was to
avoid having to add TYPE_UNKNOWN everywhere as an extra case statement, because
the two enums would have identical behavior except for the check added in the
next patch.
>
>>
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf v2 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
` (8 preceding siblings ...)
2026-09-22 17:20 ` [PATCH bpf v2 09/11] bpf: Track whether dynptr type is known Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 20:06 ` Amery Hung
2026-09-22 17:20 ` [PATCH bpf v2 11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber Emil Tsalapatis
2026-09-22 19:40 ` [PATCH bpf v2 00/11] skb/arena bugfixes patchwork-bot+netdevbpf
11 siblings, 1 reply; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf
Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis,
Nicholas Carlini
A dynptr can be backed by skb memory, and kfuncs that
write but also read the underlying area may reallocate
the backing memory in the process of pulling the skb.
However, the verifier does not track these calls as
possibly invalidating packet pointers, and does not
do so after their call site.
Expand the verifier to track dynptr kfuncs for packet
invalidation.
Fixes: 5fc5d8fded57 ("bpf: Add bpf_dynptr_memset() kfunc")
Fixes: a498ee7576de ("bpf: Implement dynptr copy kfuncs")
Fixes: daec295a7094 ("bpf/helpers: Introduce bpf_dynptr_copy kfunc")
Reported-by: Nicholas Carlini <nicholas@carlini.com>
Suggested-by: Nicholas Carlini <nicholas@carlini.com>
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
include/linux/bpf_verifier.h | 2 ++
kernel/bpf/cfg.c | 10 +++++--
kernel/bpf/verifier.c | 51 ++++++++++++++++++++++++++++++++++--
3 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index f57730d1d..8a9b7a2f2 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1589,6 +1589,7 @@ struct bpf_call_arg_meta {
/* Only set by kfunc */
bool r0_rdonly;
+ bool dynptr_may_clobber_pkt_ptr;
u32 kfunc_flags;
const struct btf_type *func_proto;
const char *func_name;
@@ -1642,6 +1643,7 @@ static inline bool bpf_is_kfunc_sleepable(struct bpf_call_arg_meta *meta)
return meta->kfunc_flags & KF_SLEEPABLE;
}
bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta);
+bool bpf_is_kfunc_maybe_pkt_changing(struct bpf_call_arg_meta *meta);
struct bpf_iarray *bpf_iarray_realloc(struct bpf_iarray *old, size_t n_elem);
int bpf_copy_insn_array_uniq(struct bpf_map *map, u32 start, u32 end, u32 *off);
bool bpf_insn_is_cond_jump(u8 code);
diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
index 842c7d1ea..cb499d19d 100644
--- a/kernel/bpf/cfg.c
+++ b/kernel/bpf/cfg.c
@@ -73,6 +73,12 @@ static void mark_subprog_might_throw(struct bpf_verifier_env *env, int off)
subprog->might_throw = true;
}
+static bool bpf_helper_maybe_changes_pkt_data(enum bpf_func_id func_id)
+{
+ return bpf_helper_changes_pkt_data(func_id) ||
+ func_id == BPF_FUNC_dynptr_write;
+}
+
/* 't' is an index of a call-site.
* 'w' is a callee entry point.
* Eventually this function would be called when env->cfg.insn_state[w] == EXPLORED.
@@ -510,7 +516,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
*/
if (ret == 0 && fp->might_sleep)
mark_subprog_might_sleep(env, t);
- if (bpf_helper_changes_pkt_data(insn->imm))
+ if (bpf_helper_maybe_changes_pkt_data(insn->imm))
mark_subprog_changes_pkt_data(env, t);
if (insn->imm == BPF_FUNC_tail_call) {
ret = visit_abnormal_return_insn(env, t);
@@ -543,7 +549,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
*/
if (ret == 0 && bpf_is_kfunc_sleepable(&meta))
mark_subprog_might_sleep(env, t);
- if (ret == 0 && bpf_is_kfunc_pkt_changing(&meta))
+ if (ret == 0 && bpf_is_kfunc_maybe_pkt_changing(&meta))
mark_subprog_changes_pkt_data(env, t);
if (ret == 0 && bpf_is_throw_kfunc(insn))
mark_subprog_might_throw(env, t);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index da110cdbc..e916ce89c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8278,6 +8278,7 @@ static bool is_kfunc_arg_scalar_with_name(const struct btf *btf,
const char *name);
static bool is_bpf_cast_to_kern_ctx_kfunc(const struct bpf_call_arg_meta *meta);
static bool is_bpf_dynptr_clone_kfunc(const struct bpf_call_arg_meta *meta);
+static bool is_kfunc_dynptr_may_clobber_pkt_ptr(struct bpf_call_arg_meta *meta);
static bool is_bpf_iter_css_task_new_kfunc(const struct bpf_call_arg_meta *meta);
static bool is_bpf_obj_drop_kfunc(u32 func_id);
static bool is_bpf_percpu_obj_drop_kfunc(u32 func_id);
@@ -9320,6 +9321,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, u32 slot, u32 p
err = process_dynptr_func(env, reg, argno, insn_idx, arg_type, meta);
if (err)
return err;
+ /*
+ * These kfuncs only clobber packet pointers when their
+ * destination dynptr, argument 0, is backed by skb packet data.
+ */
+ if (arg == 0 && is_kfunc_dynptr_may_clobber_pkt_ptr(meta) &&
+ (meta->dynptr.type_unknown ||
+ meta->dynptr.type == BPF_DYNPTR_TYPE_SKB ||
+ meta->dynptr.type == BPF_DYNPTR_TYPE_SKB_META))
+ meta->dynptr_may_clobber_pkt_ptr = true;
break;
}
case ARG_PTR_TO_ITER:
@@ -11759,7 +11769,8 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
if (dynptr_type == BPF_DYNPTR_TYPE_INVALID)
return -EFAULT;
- if (dynptr_type == BPF_DYNPTR_TYPE_SKB ||
+ if (meta.dynptr.type_unknown ||
+ dynptr_type == BPF_DYNPTR_TYPE_SKB ||
dynptr_type == BPF_DYNPTR_TYPE_SKB_META)
/* this will trigger clear_all_pkt_pointers(), which will
* invalidate all dynptr slices associated with the skb
@@ -12787,9 +12798,45 @@ static bool is_kfunc_bpf_preempt_enable(struct bpf_call_arg_meta *meta)
return is_kfunc_call(meta, special_kfunc_list[KF_bpf_preempt_enable]);
}
+/*
+ * Dynptr kfuncs that may clobber packet pointers when called with an skb or
+ * skb_meta backed destination dynptr by pulling the packet.
+ */
+BTF_SET_START(dynptr_may_clobber_pkt_ptr_kfuncs)
+BTF_ID(func, bpf_dynptr_memset)
+BTF_ID(func, bpf_dynptr_copy)
+#ifdef CONFIG_BPF_EVENTS
+BTF_ID(func, bpf_probe_read_user_dynptr)
+BTF_ID(func, bpf_probe_read_kernel_dynptr)
+BTF_ID(func, bpf_probe_read_user_str_dynptr)
+BTF_ID(func, bpf_probe_read_kernel_str_dynptr)
+BTF_ID(func, bpf_copy_from_user_dynptr)
+BTF_ID(func, bpf_copy_from_user_str_dynptr)
+BTF_ID(func, bpf_copy_from_user_task_dynptr)
+BTF_ID(func, bpf_copy_from_user_task_str_dynptr)
+#endif
+BTF_SET_END(dynptr_may_clobber_pkt_ptr_kfuncs)
+
+static bool is_kfunc_dynptr_may_clobber_pkt_ptr(struct bpf_call_arg_meta *meta)
+{
+ return meta->btf && btf_id_set_contains(&dynptr_may_clobber_pkt_ptr_kfuncs,
+ meta->func_id);
+}
+
bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta)
{
- return is_kfunc_call(meta, special_kfunc_list[KF_bpf_xdp_pull_data]);
+ return is_kfunc_call(meta, special_kfunc_list[KF_bpf_xdp_pull_data]) ||
+ meta->dynptr_may_clobber_pkt_ptr;
+}
+
+/*
+ * More conservative version of the above used in check_cfg(),
+ * where no register state exists and the dynptr type is unknown.
+ */
+bool bpf_is_kfunc_maybe_pkt_changing(struct bpf_call_arg_meta *meta)
+{
+ return bpf_is_kfunc_pkt_changing(meta) ||
+ is_kfunc_dynptr_may_clobber_pkt_ptr(meta);
}
static u32 kfunc_abi_slots(const struct btf_func_model *fm)
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs
2026-09-22 17:20 ` [PATCH bpf v2 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs Emil Tsalapatis
@ 2026-09-22 20:06 ` Amery Hung
2026-09-22 20:29 ` Emil Tsalapatis
0 siblings, 1 reply; 24+ messages in thread
From: Amery Hung @ 2026-09-22 20:06 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev,
Nicholas Carlini
On Tue, Sep 22, 2026 at 10:24 AM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>
> A dynptr can be backed by skb memory, and kfuncs that
> write but also read the underlying area may reallocate
> the backing memory in the process of pulling the skb.
> However, the verifier does not track these calls as
> possibly invalidating packet pointers, and does not
> do so after their call site.
>
> Expand the verifier to track dynptr kfuncs for packet
> invalidation.
>
> Fixes: 5fc5d8fded57 ("bpf: Add bpf_dynptr_memset() kfunc")
> Fixes: a498ee7576de ("bpf: Implement dynptr copy kfuncs")
> Fixes: daec295a7094 ("bpf/helpers: Introduce bpf_dynptr_copy kfunc")
> Reported-by: Nicholas Carlini <nicholas@carlini.com>
> Suggested-by: Nicholas Carlini <nicholas@carlini.com>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
> include/linux/bpf_verifier.h | 2 ++
> kernel/bpf/cfg.c | 10 +++++--
> kernel/bpf/verifier.c | 51 ++++++++++++++++++++++++++++++++++--
> 3 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index f57730d1d..8a9b7a2f2 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -1589,6 +1589,7 @@ struct bpf_call_arg_meta {
>
> /* Only set by kfunc */
> bool r0_rdonly;
> + bool dynptr_may_clobber_pkt_ptr;
I would suggest making it common to helper and kfunc and not specific to dynptr:
bool pkt_changed;
Then, helper and kfunc and all share this following block:
if (meta->pkt_changed)
clear_all_pkt_pointers(env);
This also matches the existing changes_pkt_data terminology.
> u32 kfunc_flags;
> const struct btf_type *func_proto;
> const char *func_name;
> @@ -1642,6 +1643,7 @@ static inline bool bpf_is_kfunc_sleepable(struct bpf_call_arg_meta *meta)
> return meta->kfunc_flags & KF_SLEEPABLE;
> }
> bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta);
> +bool bpf_is_kfunc_maybe_pkt_changing(struct bpf_call_arg_meta *meta);
> struct bpf_iarray *bpf_iarray_realloc(struct bpf_iarray *old, size_t n_elem);
> int bpf_copy_insn_array_uniq(struct bpf_map *map, u32 start, u32 end, u32 *off);
> bool bpf_insn_is_cond_jump(u8 code);
> diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
> index 842c7d1ea..cb499d19d 100644
> --- a/kernel/bpf/cfg.c
> +++ b/kernel/bpf/cfg.c
> @@ -73,6 +73,12 @@ static void mark_subprog_might_throw(struct bpf_verifier_env *env, int off)
> subprog->might_throw = true;
> }
>
> +static bool bpf_helper_maybe_changes_pkt_data(enum bpf_func_id func_id)
> +{
> + return bpf_helper_changes_pkt_data(func_id) ||
> + func_id == BPF_FUNC_dynptr_write;
> +}
> +
> /* 't' is an index of a call-site.
> * 'w' is a callee entry point.
> * Eventually this function would be called when env->cfg.insn_state[w] == EXPLORED.
> @@ -510,7 +516,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
> */
> if (ret == 0 && fp->might_sleep)
> mark_subprog_might_sleep(env, t);
> - if (bpf_helper_changes_pkt_data(insn->imm))
> + if (bpf_helper_maybe_changes_pkt_data(insn->imm))
> mark_subprog_changes_pkt_data(env, t);
> if (insn->imm == BPF_FUNC_tail_call) {
> ret = visit_abnormal_return_insn(env, t);
> @@ -543,7 +549,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
> */
> if (ret == 0 && bpf_is_kfunc_sleepable(&meta))
> mark_subprog_might_sleep(env, t);
> - if (ret == 0 && bpf_is_kfunc_pkt_changing(&meta))
> + if (ret == 0 && bpf_is_kfunc_maybe_pkt_changing(&meta))
> mark_subprog_changes_pkt_data(env, t);
> if (ret == 0 && bpf_is_throw_kfunc(insn))
> mark_subprog_might_throw(env, t);
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index da110cdbc..e916ce89c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -8278,6 +8278,7 @@ static bool is_kfunc_arg_scalar_with_name(const struct btf *btf,
> const char *name);
> static bool is_bpf_cast_to_kern_ctx_kfunc(const struct bpf_call_arg_meta *meta);
> static bool is_bpf_dynptr_clone_kfunc(const struct bpf_call_arg_meta *meta);
> +static bool is_kfunc_dynptr_may_clobber_pkt_ptr(struct bpf_call_arg_meta *meta);
> static bool is_bpf_iter_css_task_new_kfunc(const struct bpf_call_arg_meta *meta);
> static bool is_bpf_obj_drop_kfunc(u32 func_id);
> static bool is_bpf_percpu_obj_drop_kfunc(u32 func_id);
> @@ -9320,6 +9321,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, u32 slot, u32 p
> err = process_dynptr_func(env, reg, argno, insn_idx, arg_type, meta);
> if (err)
> return err;
> + /*
> + * These kfuncs only clobber packet pointers when their
> + * destination dynptr, argument 0, is backed by skb packet data.
> + */
> + if (arg == 0 && is_kfunc_dynptr_may_clobber_pkt_ptr(meta) &&
> + (meta->dynptr.type_unknown ||
> + meta->dynptr.type == BPF_DYNPTR_TYPE_SKB ||
> + meta->dynptr.type == BPF_DYNPTR_TYPE_SKB_META))
> + meta->dynptr_may_clobber_pkt_ptr = true;
> break;
> }
> case ARG_PTR_TO_ITER:
> @@ -11759,7 +11769,8 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> if (dynptr_type == BPF_DYNPTR_TYPE_INVALID)
> return -EFAULT;
>
> - if (dynptr_type == BPF_DYNPTR_TYPE_SKB ||
> + if (meta.dynptr.type_unknown ||
> + dynptr_type == BPF_DYNPTR_TYPE_SKB ||
> dynptr_type == BPF_DYNPTR_TYPE_SKB_META)
> /* this will trigger clear_all_pkt_pointers(), which will
> * invalidate all dynptr slices associated with the skb
> @@ -12787,9 +12798,45 @@ static bool is_kfunc_bpf_preempt_enable(struct bpf_call_arg_meta *meta)
> return is_kfunc_call(meta, special_kfunc_list[KF_bpf_preempt_enable]);
> }
>
> +/*
> + * Dynptr kfuncs that may clobber packet pointers when called with an skb or
> + * skb_meta backed destination dynptr by pulling the packet.
> + */
> +BTF_SET_START(dynptr_may_clobber_pkt_ptr_kfuncs)
This set identifies kfuncs that write to dynptr-backed memory. Whether
such a write can invalidate packet pointers is determined separately
from the destination dynptr type. How about:
BTF_SET_START(dynptr_memory_write_kfuncs)
> +BTF_ID(func, bpf_dynptr_memset)
> +BTF_ID(func, bpf_dynptr_copy)
> +#ifdef CONFIG_BPF_EVENTS
> +BTF_ID(func, bpf_probe_read_user_dynptr)
> +BTF_ID(func, bpf_probe_read_kernel_dynptr)
> +BTF_ID(func, bpf_probe_read_user_str_dynptr)
> +BTF_ID(func, bpf_probe_read_kernel_str_dynptr)
> +BTF_ID(func, bpf_copy_from_user_dynptr)
> +BTF_ID(func, bpf_copy_from_user_str_dynptr)
> +BTF_ID(func, bpf_copy_from_user_task_dynptr)
> +BTF_ID(func, bpf_copy_from_user_task_str_dynptr)
> +#endif
> +BTF_SET_END(dynptr_may_clobber_pkt_ptr_kfuncs)
> +
> +static bool is_kfunc_dynptr_may_clobber_pkt_ptr(struct bpf_call_arg_meta *meta)
Likewise, perhaps:
static bool is_kfunc_dynptr_memory_write(...)
This keeps the two concepts separate: the set classifies the
operation, while meta.changes_pkt_data records the effect for this
particular call.
> +{
> + return meta->btf && btf_id_set_contains(&dynptr_may_clobber_pkt_ptr_kfuncs,
> + meta->func_id);
> +}
> +
> bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta)
> {
> - return is_kfunc_call(meta, special_kfunc_list[KF_bpf_xdp_pull_data]);
> + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_xdp_pull_data]) ||
> + meta->dynptr_may_clobber_pkt_ptr;
> +}
> +
> +/*
> + * More conservative version of the above used in check_cfg(),
> + * where no register state exists and the dynptr type is unknown.
> + */
> +bool bpf_is_kfunc_maybe_pkt_changing(struct bpf_call_arg_meta *meta)
> +{
> + return bpf_is_kfunc_pkt_changing(meta) ||
> + is_kfunc_dynptr_may_clobber_pkt_ptr(meta);
> }
>
> static u32 kfunc_abi_slots(const struct btf_func_model *fm)
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs
2026-09-22 20:06 ` Amery Hung
@ 2026-09-22 20:29 ` Emil Tsalapatis
0 siblings, 0 replies; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 20:29 UTC (permalink / raw)
To: Amery Hung, Emil Tsalapatis
Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev,
Nicholas Carlini
On Tue Sep 22, 2026 at 8:06 PM UTC, Amery Hung wrote:
> On Tue, Sep 22, 2026 at 10:24 AM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>>
>> A dynptr can be backed by skb memory, and kfuncs that
>> write but also read the underlying area may reallocate
>> the backing memory in the process of pulling the skb.
>> However, the verifier does not track these calls as
>> possibly invalidating packet pointers, and does not
>> do so after their call site.
>>
>> Expand the verifier to track dynptr kfuncs for packet
>> invalidation.
>>
>> Fixes: 5fc5d8fded57 ("bpf: Add bpf_dynptr_memset() kfunc")
>> Fixes: a498ee7576de ("bpf: Implement dynptr copy kfuncs")
>> Fixes: daec295a7094 ("bpf/helpers: Introduce bpf_dynptr_copy kfunc")
>> Reported-by: Nicholas Carlini <nicholas@carlini.com>
>> Suggested-by: Nicholas Carlini <nicholas@carlini.com>
>> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> ---
>> include/linux/bpf_verifier.h | 2 ++
>> kernel/bpf/cfg.c | 10 +++++--
>> kernel/bpf/verifier.c | 51 ++++++++++++++++++++++++++++++++++--
>> 3 files changed, 59 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index f57730d1d..8a9b7a2f2 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -1589,6 +1589,7 @@ struct bpf_call_arg_meta {
>>
>> /* Only set by kfunc */
>> bool r0_rdonly;
>> + bool dynptr_may_clobber_pkt_ptr;
>
> I would suggest making it common to helper and kfunc and not specific to dynptr:
>
> bool pkt_changed;
>
> Then, helper and kfunc and all share this following block:
>
> if (meta->pkt_changed)
> clear_all_pkt_pointers(env);
>
> This also matches the existing changes_pkt_data terminology.
This and the point below both make sense to me, thank you. I will adjust accordingly.
>
>> u32 kfunc_flags;
>> const struct btf_type *func_proto;
>> const char *func_name;
>> @@ -1642,6 +1643,7 @@ static inline bool bpf_is_kfunc_sleepable(struct bpf_call_arg_meta *meta)
>> return meta->kfunc_flags & KF_SLEEPABLE;
>> }
>> bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta);
>> +bool bpf_is_kfunc_maybe_pkt_changing(struct bpf_call_arg_meta *meta);
>> struct bpf_iarray *bpf_iarray_realloc(struct bpf_iarray *old, size_t n_elem);
>> int bpf_copy_insn_array_uniq(struct bpf_map *map, u32 start, u32 end, u32 *off);
>> bool bpf_insn_is_cond_jump(u8 code);
>> diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
>> index 842c7d1ea..cb499d19d 100644
>> --- a/kernel/bpf/cfg.c
>> +++ b/kernel/bpf/cfg.c
>> @@ -73,6 +73,12 @@ static void mark_subprog_might_throw(struct bpf_verifier_env *env, int off)
>> subprog->might_throw = true;
>> }
>>
>> +static bool bpf_helper_maybe_changes_pkt_data(enum bpf_func_id func_id)
>> +{
>> + return bpf_helper_changes_pkt_data(func_id) ||
>> + func_id == BPF_FUNC_dynptr_write;
>> +}
>> +
>> /* 't' is an index of a call-site.
>> * 'w' is a callee entry point.
>> * Eventually this function would be called when env->cfg.insn_state[w] == EXPLORED.
>> @@ -510,7 +516,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
>> */
>> if (ret == 0 && fp->might_sleep)
>> mark_subprog_might_sleep(env, t);
>> - if (bpf_helper_changes_pkt_data(insn->imm))
>> + if (bpf_helper_maybe_changes_pkt_data(insn->imm))
>> mark_subprog_changes_pkt_data(env, t);
>> if (insn->imm == BPF_FUNC_tail_call) {
>> ret = visit_abnormal_return_insn(env, t);
>> @@ -543,7 +549,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
>> */
>> if (ret == 0 && bpf_is_kfunc_sleepable(&meta))
>> mark_subprog_might_sleep(env, t);
>> - if (ret == 0 && bpf_is_kfunc_pkt_changing(&meta))
>> + if (ret == 0 && bpf_is_kfunc_maybe_pkt_changing(&meta))
>> mark_subprog_changes_pkt_data(env, t);
>> if (ret == 0 && bpf_is_throw_kfunc(insn))
>> mark_subprog_might_throw(env, t);
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index da110cdbc..e916ce89c 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -8278,6 +8278,7 @@ static bool is_kfunc_arg_scalar_with_name(const struct btf *btf,
>> const char *name);
>> static bool is_bpf_cast_to_kern_ctx_kfunc(const struct bpf_call_arg_meta *meta);
>> static bool is_bpf_dynptr_clone_kfunc(const struct bpf_call_arg_meta *meta);
>> +static bool is_kfunc_dynptr_may_clobber_pkt_ptr(struct bpf_call_arg_meta *meta);
>> static bool is_bpf_iter_css_task_new_kfunc(const struct bpf_call_arg_meta *meta);
>> static bool is_bpf_obj_drop_kfunc(u32 func_id);
>> static bool is_bpf_percpu_obj_drop_kfunc(u32 func_id);
>> @@ -9320,6 +9321,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, u32 slot, u32 p
>> err = process_dynptr_func(env, reg, argno, insn_idx, arg_type, meta);
>> if (err)
>> return err;
>> + /*
>> + * These kfuncs only clobber packet pointers when their
>> + * destination dynptr, argument 0, is backed by skb packet data.
>> + */
>> + if (arg == 0 && is_kfunc_dynptr_may_clobber_pkt_ptr(meta) &&
>> + (meta->dynptr.type_unknown ||
>> + meta->dynptr.type == BPF_DYNPTR_TYPE_SKB ||
>> + meta->dynptr.type == BPF_DYNPTR_TYPE_SKB_META))
>> + meta->dynptr_may_clobber_pkt_ptr = true;
>> break;
>> }
>> case ARG_PTR_TO_ITER:
>> @@ -11759,7 +11769,8 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>> if (dynptr_type == BPF_DYNPTR_TYPE_INVALID)
>> return -EFAULT;
>>
>> - if (dynptr_type == BPF_DYNPTR_TYPE_SKB ||
>> + if (meta.dynptr.type_unknown ||
>> + dynptr_type == BPF_DYNPTR_TYPE_SKB ||
>> dynptr_type == BPF_DYNPTR_TYPE_SKB_META)
>> /* this will trigger clear_all_pkt_pointers(), which will
>> * invalidate all dynptr slices associated with the skb
>> @@ -12787,9 +12798,45 @@ static bool is_kfunc_bpf_preempt_enable(struct bpf_call_arg_meta *meta)
>> return is_kfunc_call(meta, special_kfunc_list[KF_bpf_preempt_enable]);
>> }
>>
>> +/*
>> + * Dynptr kfuncs that may clobber packet pointers when called with an skb or
>> + * skb_meta backed destination dynptr by pulling the packet.
>> + */
>> +BTF_SET_START(dynptr_may_clobber_pkt_ptr_kfuncs)
>
> This set identifies kfuncs that write to dynptr-backed memory. Whether
> such a write can invalidate packet pointers is determined separately
> from the destination dynptr type. How about:
>
> BTF_SET_START(dynptr_memory_write_kfuncs)
>
>> +BTF_ID(func, bpf_dynptr_memset)
>> +BTF_ID(func, bpf_dynptr_copy)
>> +#ifdef CONFIG_BPF_EVENTS
>> +BTF_ID(func, bpf_probe_read_user_dynptr)
>> +BTF_ID(func, bpf_probe_read_kernel_dynptr)
>> +BTF_ID(func, bpf_probe_read_user_str_dynptr)
>> +BTF_ID(func, bpf_probe_read_kernel_str_dynptr)
>> +BTF_ID(func, bpf_copy_from_user_dynptr)
>> +BTF_ID(func, bpf_copy_from_user_str_dynptr)
>> +BTF_ID(func, bpf_copy_from_user_task_dynptr)
>> +BTF_ID(func, bpf_copy_from_user_task_str_dynptr)
>> +#endif
>> +BTF_SET_END(dynptr_may_clobber_pkt_ptr_kfuncs)
>> +
>> +static bool is_kfunc_dynptr_may_clobber_pkt_ptr(struct bpf_call_arg_meta *meta)
>
> Likewise, perhaps:
>
> static bool is_kfunc_dynptr_memory_write(...)
>
> This keeps the two concepts separate: the set classifies the
> operation, while meta.changes_pkt_data records the effect for this
> particular call.
>
>> +{
>> + return meta->btf && btf_id_set_contains(&dynptr_may_clobber_pkt_ptr_kfuncs,
>> + meta->func_id);
>> +}
>> +
>> bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta)
>> {
>> - return is_kfunc_call(meta, special_kfunc_list[KF_bpf_xdp_pull_data]);
>> + return is_kfunc_call(meta, special_kfunc_list[KF_bpf_xdp_pull_data]) ||
>> + meta->dynptr_may_clobber_pkt_ptr;
>> +}
>> +
>> +/*
>> + * More conservative version of the above used in check_cfg(),
>> + * where no register state exists and the dynptr type is unknown.
>> + */
>> +bool bpf_is_kfunc_maybe_pkt_changing(struct bpf_call_arg_meta *meta)
>> +{
>> + return bpf_is_kfunc_pkt_changing(meta) ||
>> + is_kfunc_dynptr_may_clobber_pkt_ptr(meta);
>> }
>>
>> static u32 kfunc_abi_slots(const struct btf_func_model *fm)
>> --
>> 2.54.0
>>
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf v2 11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
` (9 preceding siblings ...)
2026-09-22 17:20 ` [PATCH bpf v2 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs Emil Tsalapatis
@ 2026-09-22 17:20 ` Emil Tsalapatis
2026-09-22 19:40 ` [PATCH bpf v2 00/11] skb/arena bugfixes patchwork-bot+netdevbpf
11 siblings, 0 replies; 24+ messages in thread
From: Emil Tsalapatis @ 2026-09-22 17:20 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis
Add tests that ensure that dynptr slices backed by skb memory
are invalidated on any operation that potentially moves/frees
the backing skb memory. Also test that only skb-backed dynptrs
may invalidate pointers into skbs.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
.../testing/selftests/bpf/progs/dynptr_fail.c | 183 ++++++++++++++++++
1 file changed, 183 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
index 1cd61d72c..0f030f397 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
@@ -13,6 +13,9 @@
char _license[] SEC("license") = "GPL";
+extern int bpf_dynptr_memset(const struct bpf_dynptr *p, __u64 offset, __u64 size,
+ __u8 val) __ksym __weak;
+
struct test_info {
int x;
struct bpf_dynptr ptr;
@@ -1274,6 +1277,186 @@ int skb_invalid_data_slice4(struct __sk_buff *skb)
return SK_PASS;
}
+/*
+ * A kfunc that may clobber packet pointers must invalidate skb data slices.
+ */
+SEC("?tc")
+__failure __msg("invalid mem access")
+int skb_invalid_data_slice_after_dynptr_memset(struct __sk_buff *skb)
+{
+ struct bpf_dynptr ptr;
+ struct ethhdr *hdr;
+ char buffer[sizeof(*hdr)] = {};
+
+ bpf_dynptr_from_skb(skb, 0, &ptr);
+
+ hdr = bpf_dynptr_slice_rdwr(&ptr, 0, buffer, sizeof(buffer));
+ if (!hdr)
+ return SK_DROP;
+
+ bpf_dynptr_memset(&ptr, 0, 0, 0);
+
+ /* this should fail */
+ val = hdr->h_proto;
+
+ return SK_PASS;
+}
+
+/*
+ * A dynptr kfunc that may clobber packet pointers must not invalidate packet
+ * pointers when called with a non-skb dynptr.
+ */
+SEC("?tc")
+__success
+int pkt_ptr_valid_after_dynptr_memset_on_mem(struct __sk_buff *skb)
+{
+ struct bpf_dynptr ptr;
+ int *p = (void *)(long)skb->data;
+
+ if ((void *)(p + 1) > (void *)(long)skb->data_end)
+ return SK_DROP;
+
+ if (bpf_dynptr_from_mem(&val, sizeof(val), 0, &ptr))
+ return SK_DROP;
+
+ bpf_dynptr_memset(&ptr, 0, 0, 0);
+
+ return *p ? SK_PASS : SK_DROP;
+}
+
+__noinline
+int skb_dynptr_memset(struct __sk_buff *skb)
+{
+ struct bpf_dynptr ptr;
+
+ if (bpf_dynptr_from_skb(skb, 0, &ptr))
+ return 0;
+
+ return bpf_dynptr_memset(&ptr, 0, 0, 0);
+}
+
+/*
+ * A global subprog that may clobber packet pointers must invalidate packet
+ * pointers held by its caller.
+ */
+SEC("?tc")
+__failure __msg("invalid mem access")
+int skb_invalid_pkt_ptr_after_dynptr_memset_global(struct __sk_buff *skb)
+{
+ int *p = (void *)(long)skb->data;
+
+ if ((void *)(p + 1) > (void *)(long)skb->data_end)
+ return SK_DROP;
+
+ skb_dynptr_memset(skb);
+
+ /* this should fail */
+ val = *p;
+
+ return SK_PASS;
+}
+
+__noinline
+int skb_dynptr_memset_global_arg(struct __sk_buff *skb,
+ struct bpf_dynptr *ptr)
+{
+ int *p = (void *)(long)skb->data;
+
+ if ((void *)(p + 1) > (void *)(long)skb->data_end)
+ return SK_DROP;
+
+ bpf_dynptr_memset(ptr, 0, sizeof(*p), 0);
+
+ /* this should fail */
+ val = *p;
+
+ return SK_PASS;
+}
+
+/*
+ * A global subprog must conservatively treat a dynptr argument as possibly
+ * skb-backed when checking whether a kfunc invalidates packet pointers.
+ */
+SEC("?tc")
+__failure __msg("invalid mem access")
+int skb_invalid_pkt_ptr_after_dynptr_memset_global_arg(struct __sk_buff *skb)
+{
+ struct bpf_dynptr ptr;
+
+ if (bpf_dynptr_from_skb(skb, 0, &ptr))
+ return SK_DROP;
+
+ return skb_dynptr_memset_global_arg(skb, &ptr);
+}
+
+__noinline
+int skb_dynptr_write_global_arg(struct bpf_dynptr *ptr)
+{
+ int write_data = 0;
+
+ return bpf_dynptr_write(ptr, 0, &write_data, sizeof(write_data), 0);
+}
+
+/*
+ * A global subprog that calls bpf_dynptr_write() on a dynptr argument must
+ * invalidate packet pointers held by its caller.
+ */
+SEC("?tc")
+__failure __msg("invalid mem access")
+int skb_invalid_pkt_ptr_after_dynptr_write_global_arg(struct __sk_buff *skb)
+{
+ struct bpf_dynptr ptr;
+ int *p = (void *)(long)skb->data;
+
+ if ((void *)(p + 1) > (void *)(long)skb->data_end)
+ return SK_DROP;
+
+ if (bpf_dynptr_from_skb(skb, 0, &ptr))
+ return SK_DROP;
+
+ skb_dynptr_write_global_arg(&ptr);
+
+ /* this should fail */
+ val = *p;
+
+ return SK_PASS;
+}
+
+__noinline
+int skb_dynptr_write_global_arg_with_pkt_ptr(struct __sk_buff *skb,
+ struct bpf_dynptr *ptr)
+{
+ int write_data = 0;
+ int *p = (void *)(long)skb->data;
+
+ if ((void *)(p + 1) > (void *)(long)skb->data_end)
+ return SK_DROP;
+
+ bpf_dynptr_write(ptr, 0, &write_data, sizeof(write_data), 0);
+
+ /* this should fail */
+ val = *p;
+
+ return SK_PASS;
+}
+
+/*
+ * A global subprog must conservatively treat a dynptr argument as possibly
+ * skb-backed when checking whether bpf_dynptr_write() invalidates packet
+ * pointers in the callee.
+ */
+SEC("?tc")
+__failure __msg("invalid mem access")
+int skb_invalid_pkt_ptr_after_dynptr_write_global_arg_in_subprog(struct __sk_buff *skb)
+{
+ struct bpf_dynptr ptr;
+
+ if (bpf_dynptr_from_skb(skb, 0, &ptr))
+ return SK_DROP;
+
+ return skb_dynptr_write_global_arg_with_pkt_ptr(skb, &ptr);
+}
+
/* Read-only skb data slice is invalidated on write to skb metadata */
SEC("?tc")
__failure __msg("invalid mem access 'scalar'")
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf v2 00/11] skb/arena bugfixes
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
` (10 preceding siblings ...)
2026-09-22 17:20 ` [PATCH bpf v2 11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber Emil Tsalapatis
@ 2026-09-22 19:40 ` patchwork-bot+netdevbpf
11 siblings, 0 replies; 24+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-22 19:40 UTC (permalink / raw)
To: Emil Tsalapatis; +Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev
Hello:
This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Tue, 22 Sep 2026 17:20:17 +0000 you wrote:
> Set of verifier-focused bugfixes related to dynptrs, SKB
> management in BPF, and dynptrs.
>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>
> v1 -> v2 (https://lore.kernel.org/bpf/20260916050830.8774-1-emil@etsalapatis.com/)
>
> [...]
Here is the summary with links:
- [bpf,v2,01/11] bpf: Fix bounds check for skb-backed dynptrs
https://git.kernel.org/bpf/bpf/c/ed6eec97b534
- [bpf,v2,02/11] selftests/bpf: Test dynptr slices past end of skb
https://git.kernel.org/bpf/bpf/c/4fd72eb9f1c9
- [bpf,v2,03/11] bpf: Fix bpf_sock context code generation
https://git.kernel.org/bpf/bpf/c/4a4852376e3a
- [bpf,v2,04/11] selftests/bpf: Add selftests for rx_queue_mapping context access
https://git.kernel.org/bpf/bpf/c/dec0c209a680
- [bpf,v2,05/11] bpf: Reject pkt arguments in mutating subprogs
https://git.kernel.org/bpf/bpf/c/a6c1edfbe240
- [bpf,v2,06/11] selftests/bpf: Test rejection of pkt args to mutating subprogs
https://git.kernel.org/bpf/bpf/c/1ed69a54d318
- [bpf,v2,07/11] bpf: Prevent variable arena/non-arena register contents
https://git.kernel.org/bpf/bpf/c/f85f5917aa2f
- [bpf,v2,08/11] selftests/bpf: Test for mixed arena/nonarena code paths
https://git.kernel.org/bpf/bpf/c/a9e86dd9de4f
- [bpf,v2,09/11] bpf: Track whether dynptr type is known
(no matching commit)
- [bpf,v2,10/11] bpf: Track skb memory invalidation by packet-backed dynptrs
(no matching commit)
- [bpf,v2,11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber
(no matching commit)
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] 24+ messages in thread