* [PATCH bpf 00/11] skb/arena bugfixes
@ 2026-09-16 5:08 Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
` (10 more replies)
0 siblings, 11 replies; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, eddyz87, memxor, daniel, netdev, etsal,
Emil Tsalapatis
From: etsal <etsal@alpine05.astextra>
Set of verifier-focused bugfixes related to dynptrs, SKB
management in BPF, and dynptrs.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
Emil Tsalapatis (11):
bpf: Fix bounds check for skb-backed dynptrs
selftests/bpf: Test dynptr slices past end of skb
bpf: Fix bpf_sock context code generation
selftests/bpf: Add selftests for rx_queue_mapping context access
bpf: Reject pkt arguments in mutating subprogs
selftests/bpf: Test rejection of pkt args to mutating subprogs
bpf: Prevent variable arena/non-arena register contents
selftests/bpf: Test for mixed arena/nonarena code paths
bpf: Track whether dynptr type is known
bpf: Track skb memory invalidation by packet-backed dynptrs
selftests/bpf: Test dynptr slice invalidation on skb clobber
include/linux/bpf_verifier.h | 5 +
include/linux/skbuff.h | 3 +-
kernel/bpf/cfg.c | 10 +-
kernel/bpf/verifier.c | 112 +++++++++--
net/core/filter.c | 7 +-
.../testing/selftests/bpf/prog_tests/dynptr.c | 10 +
.../testing/selftests/bpf/progs/dynptr_fail.c | 183 ++++++++++++++++++
.../selftests/bpf/progs/dynptr_success.c | 20 ++
.../selftests/bpf/progs/verifier_arena.c | 49 +++++
.../bpf/progs/verifier_global_ptr_args.c | 53 +++++
.../selftests/bpf/progs/verifier_sock.c | 38 ++++
11 files changed, 472 insertions(+), 18 deletions(-)
--
2.54.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 11:52 ` Jiayuan Chen
2026-09-16 23:08 ` Jakub Kicinski
2026-09-16 5:08 ` [PATCH bpf 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
` (9 subsequent siblings)
10 siblings, 2 replies; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 UTC (permalink / raw)
To: bpf
Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis,
Nicholas Carlini
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>
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
While the code for this is in skbuff.h, the only consumer is
BPF-related, as is the selftest that validates it in the next
patch. So I think it makes sense to route through BPF. If there
are any objections I will split the patch off and resend to net.
include/linux/skbuff.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 421f6fc45..d0c1463db 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4372,7 +4372,8 @@ 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))
+ if (likely((u64)offset <= skb_headlen(skb) &&
+ (u64)len <= skb_headlen(skb) - (u64)offset))
return skb->data + offset;
return NULL;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH bpf 02/11] selftests/bpf: Test dynptr slices past end of skb
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 5:18 ` sashiko-bot
2026-09-16 5:08 ` [PATCH bpf 03/11] bpf: Fix bpf_sock context code generation Emil Tsalapatis
` (8 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 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] 23+ messages in thread
* [PATCH bpf 03/11] bpf: Fix bpf_sock context code generation
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 12:21 ` Jiayuan Chen
2026-09-16 5:08 ` [PATCH bpf 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access Emil Tsalapatis
` (7 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 UTC (permalink / raw)
To: bpf
Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Emil Tsalapatis,
Nicholas Carlini
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>
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 61940e753..5d1705508 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10565,11 +10565,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] 23+ messages in thread
* [PATCH bpf 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
` (2 preceding siblings ...)
2026-09-16 5:08 ` [PATCH bpf 03/11] bpf: Fix bpf_sock context code generation Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs Emil Tsalapatis
` (6 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 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] 23+ messages in thread
* [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
` (3 preceding siblings ...)
2026-09-16 5:08 ` [PATCH bpf 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 5:57 ` Amery Hung
2026-09-16 5:08 ` [PATCH bpf 06/11] selftests/bpf: Test rejection of pkt args to " Emil Tsalapatis
` (5 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 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 6c6b8d852..507bc14b4 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10375,6 +10375,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] 23+ messages in thread
* [PATCH bpf 06/11] selftests/bpf: Test rejection of pkt args to mutating subprogs
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
` (4 preceding siblings ...)
2026-09-16 5:08 ` [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 07/11] bpf: Prevent variable arena/non-arena register contents Emil Tsalapatis
` (4 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 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] 23+ messages in thread
* [PATCH bpf 07/11] bpf: Prevent variable arena/non-arena register contents
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
` (5 preceding siblings ...)
2026-09-16 5:08 ` [PATCH bpf 06/11] selftests/bpf: Test rejection of pkt args to " Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 08/11] selftests/bpf: Test for mixed arena/nonarena code paths Emil Tsalapatis
` (3 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 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 cf85141ea..d5b4ab0ba 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 507bc14b4..2c08ea94d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -16056,6 +16056,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;
@@ -16067,12 +16068,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.
@@ -16085,6 +16097,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] 23+ messages in thread
* [PATCH bpf 08/11] selftests/bpf: Test for mixed arena/nonarena code paths
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
` (6 preceding siblings ...)
2026-09-16 5:08 ` [PATCH bpf 07/11] bpf: Prevent variable arena/non-arena register contents Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 5:17 ` sashiko-bot
2026-09-16 5:08 ` [PATCH bpf 09/11] bpf: Track whether dynptr type is known Emil Tsalapatis
` (2 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 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] 23+ messages in thread
* [PATCH bpf 09/11] bpf: Track whether dynptr type is known
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
` (7 preceding siblings ...)
2026-09-16 5:08 ` [PATCH bpf 08/11] selftests/bpf: Test for mixed arena/nonarena code paths Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 5:22 ` sashiko-bot
2026-09-16 5:08 ` [PATCH bpf 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber Emil Tsalapatis
10 siblings, 1 reply; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 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/verifier.c | 27 ++++++++++++++++++---------
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index d5b4ab0ba..76aa724de 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/verifier.c b/kernel/bpf/verifier.c
index 2c08ea94d..357ed7c30 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -688,24 +688,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,
@@ -717,6 +719,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);
@@ -772,10 +775,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;
}
@@ -1945,7 +1950,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
@@ -1957,6 +1963,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;
}
@@ -7783,6 +7790,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;
}
@@ -19889,8 +19897,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] 23+ messages in thread
* [PATCH bpf 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
` (8 preceding siblings ...)
2026-09-16 5:08 ` [PATCH bpf 09/11] bpf: Track whether dynptr type is known Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber Emil Tsalapatis
10 siblings, 0 replies; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 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 76aa724de..64cfeda5b 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1585,6 +1585,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;
@@ -1639,6 +1640,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 357ed7c30..bcd4bd2dc 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8260,6 +8260,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);
@@ -9294,6 +9295,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:
@@ -11720,7 +11730,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
@@ -12742,9 +12753,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] 23+ messages in thread
* [PATCH bpf 11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
` (9 preceding siblings ...)
2026-09-16 5:08 ` [PATCH bpf 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs Emil Tsalapatis
@ 2026-09-16 5:08 ` Emil Tsalapatis
10 siblings, 0 replies; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 5:08 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] 23+ messages in thread
* Re: [PATCH bpf 08/11] selftests/bpf: Test for mixed arena/nonarena code paths
2026-09-16 5:08 ` [PATCH bpf 08/11] selftests/bpf: Test for mixed arena/nonarena code paths Emil Tsalapatis
@ 2026-09-16 5:17 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-09-16 5:17 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: verifier tests for mixed-type CFG paths are vulnerable to compiler optimizations
--
commit 2865d7b4af7e52310b9ec10118abb32164639af3
Author: Emil Tsalapatis <emil@etsalapatis.com>
selftests/bpf: Test for mixed arena/nonarena code paths
Add a selftest to confirm the verifier rejects ALU operations
that return arena or non-arena results depending on code path.
> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/testing/selftests/bpf/progs/verifier_arena.c
> index 332322c9b54cf..1ce9c4b784816 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;
[Severity: Medium]
Are these tests vulnerable to compiler optimizations?
Since these tests rely on the control flow converging to evaluate a single
instruction with mixed types (SCALAR_VALUE and PTR_TO_ARENA), could compiler
optimizations like Clang's tail duplication duplicate the reg += 1; return 0;
block into both branches?
If the tail block is duplicated, the verifier would evaluate two separate
addition instructions, each with a uniform type, bypassing the mixed-type
check entirely and causing the test to fail to produce the expected
__failure __msg(...).
Should these functions be written using __naked and explicit assembly (like
mixed_arena_stack_atomic) to guarantee the exact instruction sharing
required for this test?
> +}
> +
> +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;
[Severity: Medium]
Similarly, could the tail addition here be duplicated into both execution
paths by the compiler, invalidating the test's mixed-type validation logic?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260916050830.8774-1-emil@etsalapatis.com?part=8
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 02/11] selftests/bpf: Test dynptr slices past end of skb
2026-09-16 5:08 ` [PATCH bpf 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
@ 2026-09-16 5:18 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-09-16 5:18 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: test misses exact boundary condition for non-linear slices
--
commit 1de6d61a90f0dec0227f3a07a5bdda96d9ca4516
Author: Emil Tsalapatis <emil@etsalapatis.com>
selftests/bpf: Test dynptr slices past end of skb
This commit adds a BPF selftest to verify that dynptr slices cannot read
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: Medium]
Does this test miss the exact boundary condition?
TEST_SKB_LINEAR_SIZE defines the exact size of the linear portion of the
SKB. By requesting an offset of TEST_SKB_LINEAR_SIZE + 1, the test skips
the exact boundary offset and leaves a one-byte gap in test coverage.
An off-by-one error in the kernel's bpf_dynptr_slice boundary check might
not be caught by this test.
> + if (data)
> + err = 2;
> +
> + return 1;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260916050830.8774-1-emil@etsalapatis.com?part=2
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 09/11] bpf: Track whether dynptr type is known
2026-09-16 5:08 ` [PATCH bpf 09/11] bpf: Track whether dynptr type is known Emil Tsalapatis
@ 2026-09-16 5:22 ` sashiko-bot
2026-09-16 18:41 ` Emil Tsalapatis
0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-09-16 5:22 UTC (permalink / raw)
To: Emil Tsalapatis; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: missing type_unknown check in stacksafe() allows unsafe pruning
--
commit 8f00e7cb51fcf9e4513f3c3e9be943021ba739cf
Author: Emil Tsalapatis <emil@etsalapatis.com>
bpf: Track whether dynptr type is known
This commit adds a type_unknown field to struct bpf_reg_state's dynptr
state to distinguish locally created dynptrs from global subprog arguments.
It updates the verifier to set this flag, preparing for proper invalidation
of unknown dynptrs in subsequent commits.
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index d5b4ab0bacd35..76aa724de1bcb 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;
[Severity: High]
Does this new field also need to be checked in stacksafe() when comparing
states?
Looking at stacksafe() in kernel/bpf/states.c, the STACK_DYNPTR case compares
type, first_slot, id, and parent_id, but it omits the new type_unknown field:
kernel/bpf/states.c:stacksafe() {
...
case STACK_DYNPTR:
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.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))
return false;
break;
...
}
If a BPF program conditionally spills a locally created dynptr
(type_unknown=false) or an unknown argument dynptr (type_unknown=true) to
the same stack slot, could the verifier incorrectly treat the states as
equivalent and prune the unknown dynptr path? This might allow bypassing
the packet pointer invalidation logic when writing to unknown dynptrs,
potentially leading to a use-after-free of packet pointers.
> /* 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
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260916050830.8774-1-emil@etsalapatis.com?part=9
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs
2026-09-16 5:08 ` [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs Emil Tsalapatis
@ 2026-09-16 5:57 ` Amery Hung
2026-09-16 5:58 ` Amery Hung
2026-09-16 18:41 ` Emil Tsalapatis
0 siblings, 2 replies; 23+ messages in thread
From: Amery Hung @ 2026-09-16 5:57 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev,
Nicholas Carlini, Mahe Tardy
On Tue, Sep 15, 2026 at 10:10 PM 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.
CC Mahe.
Hi Emil,
There is a related but separate issue [1] addressed by 8fe994c80af2
(“bpf: Consolidate function call pkt_access validation”). I think a
long-term solution could be supporting ARG_PTR_TO_PACKET for global
subprograms. For example:
int parse_something(struct __sk_buff *skb, __u16 off,
char *data_start __arg_packet, ...);
The verifier could require a real PTR_TO_PACKET at the call site and
verify the global subprogram with a real PTR_TO_PACKET, rather than
converting it to PTR_TO_MEM. This would preserve packet access rules,
allow reads from cgroup_skb programs while rejecting writes in the
callee, and automatically invalidate the argument after calls such as
bpf_skb_pull_data().
[1] https://lore.kernel.org/bpf/aqkhifLvyAhw66jg@gmail.com/#t
>
> 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 6c6b8d852..507bc14b4 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10375,6 +10375,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 [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs
2026-09-16 5:57 ` Amery Hung
@ 2026-09-16 5:58 ` Amery Hung
2026-09-16 18:41 ` Emil Tsalapatis
1 sibling, 0 replies; 23+ messages in thread
From: Amery Hung @ 2026-09-16 5:58 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev,
Nicholas Carlini, Mahe Tardy
On Tue, Sep 15, 2026 at 10:57 PM Amery Hung <ameryhung@gmail.com> wrote:
>
> On Tue, Sep 15, 2026 at 10:10 PM 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.
>
> CC Mahe.
>
> Hi Emil,
>
> There is a related but separate issue [1] addressed by 8fe994c80af2
> (“bpf: Consolidate function call pkt_access validation”). I think a
> long-term solution could be supporting ARG_PTR_TO_PACKET for global
> subprograms. For example:
>
> int parse_something(struct __sk_buff *skb, __u16 off,
> char *data_start __arg_packet, ...);
>
> The verifier could require a real PTR_TO_PACKET at the call site and
> verify the global subprogram with a real PTR_TO_PACKET, rather than
> converting it to PTR_TO_MEM. This would preserve packet access rules,
> allow reads from cgroup_skb programs while rejecting writes in the
> callee, and automatically invalidate the argument after calls such as
> bpf_skb_pull_data().
>
> [1] https://lore.kernel.org/bpf/aqkhifLvyAhw66jg@gmail.com/#t
Forgot to add that I think we should still have the short term fix.
>
> >
> > 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 6c6b8d852..507bc14b4 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -10375,6 +10375,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 [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs
2026-09-16 5:08 ` [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
@ 2026-09-16 11:52 ` Jiayuan Chen
2026-09-16 23:08 ` Jakub Kicinski
1 sibling, 0 replies; 23+ messages in thread
From: Jiayuan Chen @ 2026-09-16 11:52 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Nicholas Carlini
On 9/16/26 1:08 PM, Emil Tsalapatis 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>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
>
> While the code for this is in skbuff.h, the only consumer is
> BPF-related, as is the selftest that validates it in the next
> patch. So I think it makes sense to route through BPF. If there
> are any objections I will split the patch off and resend to net.
>
> include/linux/skbuff.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 421f6fc45..d0c1463db 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -4372,7 +4372,8 @@ 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))
> + if (likely((u64)offset <= skb_headlen(skb) &&
trailing whitespace after "&&"
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 03/11] bpf: Fix bpf_sock context code generation
2026-09-16 5:08 ` [PATCH bpf 03/11] bpf: Fix bpf_sock context code generation Emil Tsalapatis
@ 2026-09-16 12:21 ` Jiayuan Chen
0 siblings, 0 replies; 23+ messages in thread
From: Jiayuan Chen @ 2026-09-16 12:21 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: ast, andrii, eddyz87, memxor, daniel, netdev, Nicholas Carlini
On 9/16/26 1:08 PM, Emil Tsalapatis wrote:
> 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>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
> 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 61940e753..5d1705508 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -10565,11 +10565,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;
> }
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs
2026-09-16 5:57 ` Amery Hung
2026-09-16 5:58 ` Amery Hung
@ 2026-09-16 18:41 ` Emil Tsalapatis
2026-09-16 19:26 ` Amery Hung
1 sibling, 1 reply; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 18:41 UTC (permalink / raw)
To: Amery Hung, Emil Tsalapatis
Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev,
Nicholas Carlini, Mahe Tardy
On Wed Sep 16, 2026 at 5:57 AM UTC, Amery Hung wrote:
> On Tue, Sep 15, 2026 at 10:10 PM 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.
>
> CC Mahe.
>
> Hi Emil,
>
> There is a related but separate issue [1] addressed by 8fe994c80af2
> (“bpf: Consolidate function call pkt_access validation”). I think a
> long-term solution could be supporting ARG_PTR_TO_PACKET for global
> subprograms. For example:
>
> int parse_something(struct __sk_buff *skb, __u16 off,
> char *data_start __arg_packet, ...);
>
> The verifier could require a real PTR_TO_PACKET at the call site and
> verify the global subprogram with a real PTR_TO_PACKET, rather than
> converting it to PTR_TO_MEM. This would preserve packet access rules,
> allow reads from cgroup_skb programs while rejecting writes in the
> callee, and automatically invalidate the argument after calls such as
> bpf_skb_pull_data().
>
Hi Amery,
that makes sense to me, especially since this is a feature expected
by existing programs. The main issue I see is that since we want to be able
to pass it with subprogs we need to find a way to annotate its current size
(maybe passing it as a __sz that is checked by the verifier to be <= the
range - off of the pointer in the caller?)
> [1] https://lore.kernel.org/bpf/aqkhifLvyAhw66jg@gmail.com/#t
>
>>
>> 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 6c6b8d852..507bc14b4 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -10375,6 +10375,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 [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 09/11] bpf: Track whether dynptr type is known
2026-09-16 5:22 ` sashiko-bot
@ 2026-09-16 18:41 ` Emil Tsalapatis
0 siblings, 0 replies; 23+ messages in thread
From: Emil Tsalapatis @ 2026-09-16 18:41 UTC (permalink / raw)
To: sashiko-reviews, Emil Tsalapatis; +Cc: bpf
On Wed Sep 16, 2026 at 5:22 AM UTC, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] bpf: missing type_unknown check in stacksafe() allows unsafe pruning
> --
>
> commit 8f00e7cb51fcf9e4513f3c3e9be943021ba739cf
> Author: Emil Tsalapatis <emil@etsalapatis.com>
>
> bpf: Track whether dynptr type is known
>
> This commit adds a type_unknown field to struct bpf_reg_state's dynptr
> state to distinguish locally created dynptrs from global subprog arguments.
> It updates the verifier to set this flag, preparing for proper invalidation
> of unknown dynptrs in subsequent commits.
>
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index d5b4ab0bacd35..76aa724de1bcb 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;
>
> [Severity: High]
> Does this new field also need to be checked in stacksafe() when comparing
> states?
>
> Looking at stacksafe() in kernel/bpf/states.c, the STACK_DYNPTR case compares
> type, first_slot, id, and parent_id, but it omits the new type_unknown field:
>
> kernel/bpf/states.c:stacksafe() {
> ...
> case STACK_DYNPTR:
> 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.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))
> return false;
> break;
> ...
> }
>
> If a BPF program conditionally spills a locally created dynptr
> (type_unknown=false) or an unknown argument dynptr (type_unknown=true) to
> the same stack slot, could the verifier incorrectly treat the states as
> equivalent and prune the unknown dynptr path? This might allow bypassing
> the packet pointer invalidation logic when writing to unknown dynptrs,
> potentially leading to a use-after-free of packet pointers.
Bot is right, will update accordingly.
>
>> /* 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs
2026-09-16 18:41 ` Emil Tsalapatis
@ 2026-09-16 19:26 ` Amery Hung
0 siblings, 0 replies; 23+ messages in thread
From: Amery Hung @ 2026-09-16 19:26 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev,
Nicholas Carlini, Mahe Tardy
On Wed, Sep 16, 2026 at 11:41 AM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>
> On Wed Sep 16, 2026 at 5:57 AM UTC, Amery Hung wrote:
> > On Tue, Sep 15, 2026 at 10:10 PM 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.
> >
> > CC Mahe.
> >
> > Hi Emil,
> >
> > There is a related but separate issue [1] addressed by 8fe994c80af2
> > (“bpf: Consolidate function call pkt_access validation”). I think a
> > long-term solution could be supporting ARG_PTR_TO_PACKET for global
> > subprograms. For example:
> >
> > int parse_something(struct __sk_buff *skb, __u16 off,
> > char *data_start __arg_packet, ...);
> >
> > The verifier could require a real PTR_TO_PACKET at the call site and
> > verify the global subprogram with a real PTR_TO_PACKET, rather than
> > converting it to PTR_TO_MEM. This would preserve packet access rules,
> > allow reads from cgroup_skb programs while rejecting writes in the
> > callee, and automatically invalidate the argument after calls such as
> > bpf_skb_pull_data().
> >
>
> Hi Amery,
>
> that makes sense to me, especially since this is a feature expected
> by existing programs. The main issue I see is that since we want to be able
> to pass it with subprogs we need to find a way to annotate its current size
> (maybe passing it as a __sz that is checked by the verifier to be <= the
> range - off of the pointer in the caller?)
Yes. Supporting mem+size for global subprog similar to kfunc is
another direction, but ARG_PTR_TO_PACKET is simipler considering there
are other complication with packet (e.g., pkt ptr invalidation). With
ARG_PTR_TO_PACKET, the global subprog can be verified with a symbolic
packet pointer and establish its range through the usual data_end
checks.
>
> > [1] https://lore.kernel.org/bpf/aqkhifLvyAhw66jg@gmail.com/#t
> >
> >>
> >> 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 6c6b8d852..507bc14b4 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -10375,6 +10375,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 [flat|nested] 23+ messages in thread
* Re: [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs
2026-09-16 5:08 ` [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
2026-09-16 11:52 ` Jiayuan Chen
@ 2026-09-16 23:08 ` Jakub Kicinski
1 sibling, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2026-09-16 23:08 UTC (permalink / raw)
To: Emil Tsalapatis
Cc: bpf, ast, andrii, eddyz87, memxor, daniel, netdev,
Nicholas Carlini
On Wed, 16 Sep 2026 05:08:19 +0000 Emil Tsalapatis wrote:
> - if (likely(skb_headlen(skb) - offset >= len))
> + if (likely((u64)offset <= skb_headlen(skb) &&
> + (u64)len <= skb_headlen(skb) - (u64)offset))
I don't think the 64b arithmetic is needed since the max value
(headlen) itself is 32b. You can cast to unsigned int.
Also the continuation line is mis-aligned
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2026-09-16 23:08 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-16 5:08 [PATCH bpf 00/11] skb/arena bugfixes Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
2026-09-16 11:52 ` Jiayuan Chen
2026-09-16 23:08 ` Jakub Kicinski
2026-09-16 5:08 ` [PATCH bpf 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
2026-09-16 5:18 ` sashiko-bot
2026-09-16 5:08 ` [PATCH bpf 03/11] bpf: Fix bpf_sock context code generation Emil Tsalapatis
2026-09-16 12:21 ` Jiayuan Chen
2026-09-16 5:08 ` [PATCH bpf 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 05/11] bpf: Reject pkt arguments in mutating subprogs Emil Tsalapatis
2026-09-16 5:57 ` Amery Hung
2026-09-16 5:58 ` Amery Hung
2026-09-16 18:41 ` Emil Tsalapatis
2026-09-16 19:26 ` Amery Hung
2026-09-16 5:08 ` [PATCH bpf 06/11] selftests/bpf: Test rejection of pkt args to " Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 07/11] bpf: Prevent variable arena/non-arena register contents Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 08/11] selftests/bpf: Test for mixed arena/nonarena code paths Emil Tsalapatis
2026-09-16 5:17 ` sashiko-bot
2026-09-16 5:08 ` [PATCH bpf 09/11] bpf: Track whether dynptr type is known Emil Tsalapatis
2026-09-16 5:22 ` sashiko-bot
2026-09-16 18:41 ` Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs Emil Tsalapatis
2026-09-16 5:08 ` [PATCH bpf 11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber Emil Tsalapatis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox