* [PATCH v2] bpf: add diagnostics for rejected memory and map accesses
@ 2026-09-27 19:42 Suchit Karunakaran
2026-09-27 19:50 ` sashiko-bot
2026-09-28 7:42 ` Alexei Starovoitov
0 siblings, 2 replies; 4+ messages in thread
From: Suchit Karunakaran @ 2026-09-27 19:42 UTC (permalink / raw)
To: ast, daniel, andrii, eddyz87, memxor, martin.lau
Cc: song, yonghong.song, jolsa, emil, ihor.solodrai, john.fastabend,
bpf, linux-kernel, Suchit Karunakaran
Emit structured verifier diagnostics when BPF programs are rejected for
unsupported or prohibited memory operations. Cover sign-extending arena
loads without JIT support, read/write access restrictions on maps, writes
through read-only pointers, direct packet writes, and modifying helpers
used with BPF_F_RDONLY_PROG.
Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
---
Changes since v1:
- Modified the diagnostics messages to better explain the issues to the
user
---
kernel/bpf/fixups.c | 6 ++++++
kernel/bpf/verifier.c | 24 ++++++++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index e9c2d6c06218..4216b359bf28 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -10,6 +10,7 @@
#include <linux/perf_event.h>
#include <linux/sched/signal.h>
#include <net/xdp.h>
+#include "diagnostics.h"
#include "disasm.h"
#define verbose(env, fmt, args...) bpf_verifier_log_write(env, fmt, ##args)
@@ -1055,6 +1056,11 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
if (BPF_MODE(insn->code) == BPF_MEMSX) {
if (!bpf_jit_supports_insn(insn, true)) {
verbose(env, "sign extending loads from arena are not supported yet\n");
+ bpf_diag_policy(
+ env, i + delta,
+ "sign-extending arena load",
+ "this JIT backend doesn't support it yet",
+ "Recompile the BPF program with Clang's -mcpu=v3 to avoid generating sign-extending load instructions.");
return -EOPNOTSUPP;
}
insn->code = BPF_CLASS(insn->code) | BPF_PROBE_MEM32SX | BPF_SIZE(insn->code);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5c8626215ee6..bbe88c259fd0 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4472,12 +4472,22 @@ static int check_map_access_type(struct bpf_verifier_env *env, struct bpf_reg_st
if (type == BPF_WRITE && !(cap & BPF_MAP_CAN_WRITE)) {
verbose(env, "write into map forbidden, value_size=%d off=%lld size=%d\n",
map->value_size, reg_smin(reg) + off, size);
+ bpf_diag_policy(env, env->insn_idx,
+ bpf_diag_fmt(env, "write to map '%s'",
+ map->name[0] ? map->name : "unnamed"),
+ "this map was created with BPF_F_RDONLY_PROG, which allows BPF programs to only read it",
+ "Remove the write, or create the map without BPF_F_RDONLY_PROG if BPF programs need to write to it.");
return -EACCES;
}
if (type == BPF_READ && !(cap & BPF_MAP_CAN_READ)) {
verbose(env, "read from map forbidden, value_size=%d off=%lld size=%d\n",
map->value_size, reg_smin(reg) + off, size);
+ bpf_diag_policy(env, env->insn_idx,
+ bpf_diag_fmt(env, "read from map '%s'",
+ map->name[0] ? map->name : "unnamed"),
+ "this map was created with BPF_F_WRONLY_PROG, which allows BPF programs to only write it",
+ "Remove the read, or create the map without BPF_F_WRONLY_PROG if BPF programs need to read from it.");
return -EACCES;
}
@@ -6944,6 +6954,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
if (t == BPF_WRITE && rdonly_mem) {
verbose(env, "%s cannot write into %s\n",
reg_arg_name(env, argno), reg_type_str(env, reg->type));
+ bpf_diag_policy(env, insn_idx, "memory write",
+ bpf_diag_fmt(env, "%s points to read-only memory",
+ reg_arg_name(env, argno)),
+ "Use a writable destination, or copy the data into a writable buffer before modifying it.");
return -EACCES;
}
@@ -7032,6 +7046,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
} else if (reg_is_pkt_pointer(reg)) {
if (t == BPF_WRITE && !may_access_direct_pkt_data(env, NULL, t)) {
verbose(env, "cannot write into packet\n");
+ bpf_diag_policy(env, insn_idx,
+ "direct packet write",
+ "packet data is read-only through direct access for this program type",
+ "Remove the direct write, or perform the modification in a program type and hook that support packet writes.");
return -EACCES;
}
if (t == BPF_WRITE && value_regno >= 0 &&
@@ -11805,6 +11823,12 @@ record_func_map(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
func_id == BPF_FUNC_map_push_elem ||
func_id == BPF_FUNC_map_pop_elem)) {
verbose(env, "write into map forbidden\n");
+ bpf_diag_policy(env, insn_idx,
+ bpf_diag_fmt(env, "call to %s on map '%s'",
+ func_id_name(func_id),
+ map->name[0] ? map->name : "unnamed"),
+ "the map was created with BPF_F_RDONLY_PROG, which blocks modifications from BPF programs",
+ "Remove the modifying helper call, or use a map created without BPF_F_RDONLY_PROG if BPF programs need to modify it.");
return -EACCES;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] bpf: add diagnostics for rejected memory and map accesses
2026-09-27 19:42 [PATCH v2] bpf: add diagnostics for rejected memory and map accesses Suchit Karunakaran
@ 2026-09-27 19:50 ` sashiko-bot
2026-09-28 7:42 ` Alexei Starovoitov
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-09-27 19:50 UTC (permalink / raw)
To: Suchit Karunakaran; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: missing diagnostics for some read-only memory writes
--
commit 184c7fe51457fa3d0ba8959d5aae6f21fcf5effc
Author: Suchit Karunakaran <suchitkarunakaran@gmail.com>
bpf: add diagnostics for rejected memory and map accesses
Emit structured verifier diagnostics when BPF programs are rejected for
unsupported or prohibited memory operations. Cover sign-extending arena
loads without JIT support, read/write access restrictions on maps, writes
through read-only pointers, direct packet writes, and modifying helpers
used with BPF_F_RDONLY_PROG.
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6449,6 +6459,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
> if (t == BPF_WRITE && rdonly_mem) {
> verbose(env, "%s cannot write into %s\n",
> reg_arg_name(env, argno), reg_type_str(env, reg->type));
> + bpf_diag_policy(env, insn_idx, "memory write",
> + bpf_diag_fmt(env, "%s points to read-only memory",
> + reg_arg_name(env, argno)),
> + "Use a writable destination, or copy the data into a writable buffer before modifying it.");
> return -EACCES;
> }
[ ... ]
> } else if (type_is_sk_pointer(reg->type)) {
> if (t == BPF_WRITE) {
> verbose(env, "%s cannot write into %s\n",
> reg_arg_name(env, argno), reg_type_str(env, reg->type));
> return -EACCES;
> }
[Severity: Medium]
Since the commit message mentions adding diagnostics for writes through
read-only pointers, should bpf_diag_policy() also be called here for
direct writes to read-only socket pointers in check_mem_access()?
[ ... ]
> } else if (base_type(reg->type) == PTR_TO_BUF &&
> !type_may_be_null(reg->type)) {
> bool rdonly_mem = type_is_rdonly_mem(reg->type);
> u32 *max_access;
>
> if (rdonly_mem) {
> if (t == BPF_WRITE) {
> verbose(env, "%s cannot write into %s\n",
> reg_arg_name(env, argno), reg_type_str(env, reg->type));
> return -EACCES;
> }
[Severity: Medium]
Does this direct write to a read-only PTR_TO_BUF also need the new
bpf_diag_policy() diagnostic, similar to how it was added for
PTR_TO_MEM earlier in check_mem_access()?
[ ... ]
> @@ -7061,6 +7061,15 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
[ ... ]
> case PTR_TO_MEM:
> if (type_is_rdonly_mem(reg->type)) {
> if (access_type == BPF_WRITE) {
> verbose(env, "%s cannot write into %s\n",
> reg_arg_name(env, argno), reg_type_str(env, reg->type));
> return -EACCES;
> }
> }
[Severity: Medium]
If a BPF program passes a read-only PTR_TO_MEM to a helper function that
expects a writable buffer, the access is rejected here in
check_helper_mem_access(). Should this path also emit the structured
diagnostic?
> return check_mem_region_access(env, reg, argno, 0,
> access_size, reg->mem_size,
> zero_size_allowed);
> case PTR_TO_BUF:
> if (type_is_rdonly_mem(reg->type)) {
> if (access_type == BPF_WRITE) {
> verbose(env, "%s cannot write into %s\n",
> reg_arg_name(env, argno), reg_type_str(env, reg->type));
> return -EACCES;
> }
[Severity: Medium]
Similarly, does the rejection of helper function writes to read-only
PTR_TO_BUF pointers in check_helper_mem_access() need a
bpf_diag_policy() call?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260927194205.125086-1-suchitkarunakaran@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] bpf: add diagnostics for rejected memory and map accesses
2026-09-27 19:42 [PATCH v2] bpf: add diagnostics for rejected memory and map accesses Suchit Karunakaran
2026-09-27 19:50 ` sashiko-bot
@ 2026-09-28 7:42 ` Alexei Starovoitov
2026-09-28 10:04 ` Suchit Karunakaran
1 sibling, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2026-09-28 7:42 UTC (permalink / raw)
To: Suchit Karunakaran, daniel, andrii, eddyz87, memxor, martin.lau
Cc: song, yonghong.song, jolsa, emil, ihor.solodrai, john.fastabend,
bpf, linux-kernel
On Mon, Sep 28, 2026 at 01:12 AM Suchit Karunakaran <suchitkarunakaran@gmail.com> wrote:
> @@ -4472,12 +4472,22 @@ static int check_map_access_type(struct bpf_verifier_env *env, struct bpf_reg_st
> if (type == BPF_WRITE && !(cap & BPF_MAP_CAN_WRITE)) {
> verbose(env, "write into map forbidden, value_size=%d off=%lld size=%d\n",
> map->value_size, reg_smin(reg) + off, size);
> + bpf_diag_policy(env, env->insn_idx,
> + bpf_diag_fmt(env, "write to map '%s'",
> + map->name[0] ? map->name : "unnamed"),
> + "this map was created with BPF_F_RDONLY_PROG, which allows BPF programs to only read it",
> + "Remove the write, or create the map without BPF_F_RDONLY_PROG if BPF programs need to write to it.");
That's not true.
dev_map_init_map() and insn_array_alloc() set BPF_F_RDONLY_PROG
in the kernel for every devmap and insn_array.
libbpf sets it for .rodata when the prog has a const global.
The user didn't create the map with that flag and
cannot create it without.
Same in record_func_map().
[...]
> @@ -6944,6 +6954,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
> + "Use a writable destination, or copy the data into a writable buffer before modifying it.");
[...]
> @@ -7032,6 +7046,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
> + "Remove the direct write, or perform the modification in a program type and hook that support packet writes.");
These two are the same as in v1 and don't tell the user anything.
Pls focus your tokens elsewhere. I don't feel we will converge here.
pw-bot: cr
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] bpf: add diagnostics for rejected memory and map accesses
2026-09-28 7:42 ` Alexei Starovoitov
@ 2026-09-28 10:04 ` Suchit Karunakaran
0 siblings, 0 replies; 4+ messages in thread
From: Suchit Karunakaran @ 2026-09-28 10:04 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: daniel, andrii, eddyz87, memxor, martin.lau, song, yonghong.song,
jolsa, emil, ihor.solodrai, john.fastabend, bpf, linux-kernel
Hi Alexei. Sorry for the inconvenience, I didn't mean to ignore your
feedback. Please let me know if the following suggestions align with
your expectations. Of course, these are loose ideas, just to help me
understand how to frame the suggestions as I'm a bit clueless.
On Mon, 28 Sept 2026 at 13:12, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Sep 28, 2026 at 01:12 AM Suchit Karunakaran <suchitkarunakaran@gmail.com> wrote:
> > @@ -4472,12 +4472,22 @@ static int check_map_access_type(struct bpf_verifier_env *env, struct bpf_reg_st
> > if (type == BPF_WRITE && !(cap & BPF_MAP_CAN_WRITE)) {
> > verbose(env, "write into map forbidden, value_size=%d off=%lld size=%d\n",
> > map->value_size, reg_smin(reg) + off, size);
> > + bpf_diag_policy(env, env->insn_idx,
> > + bpf_diag_fmt(env, "write to map '%s'",
> > + map->name[0] ? map->name : "unnamed"),
> > + "this map was created with BPF_F_RDONLY_PROG, which allows BPF programs to only read it",
> > + "Remove the write, or create the map without BPF_F_RDONLY_PROG if BPF programs need to write to it.");
>
> That's not true.
> dev_map_init_map() and insn_array_alloc() set BPF_F_RDONLY_PROG
> in the kernel for every devmap and insn_array.
> libbpf sets it for .rodata when the prog has a const global.
> The user didn't create the map with that flag and
> cannot create it without.
> Same in record_func_map().
>
I'm sorry I wasn't aware of this. I wrote the suggestion based on the
fact that BPF_F_RDONLY_PROG and BPF_F_WRONLY_PROG cannot coexist.
How about something like below?
"Move the map updates and deletions to userspace using
bpf_map_update_elem() and bpf_map_delete_elem() if the map and program
types support it". Or maybe I'll try to write suggestions based on the
map type.
> [...]
>
> > @@ -6944,6 +6954,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
> > + "Use a writable destination, or copy the data into a writable buffer before modifying it.");
>
> [...]
>
> > @@ -7032,6 +7046,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
> > + "Remove the direct write, or perform the modification in a program type and hook that support packet writes.");
>
> These two are the same as in v1 and don't tell the user anything.
>
> Pls focus your tokens elsewhere. I don't feel we will converge here.
>
For direct packet writes: "Move packet modification to a TC classifier
(BPF_PROG_TYPE_SCHED_CLS) attached at ingress or egress or an XDP
program attached at ingress. For lightweight tunnels, use the transmit
hook '(BPF_PROG_TYPE_LWT_XMIT)'"
For read-only memory access: "Copy the data into a local stack
variable and perform modifications on the new variable."
Does this direction feel right, or would you like the suggestions to
be even more specific?
I’ll think harder before sending the next revision. I’ll also split
the patch so that each commit adds diagnostics for related issues and
I missed some places, as the AI suggested.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-28 10:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-27 19:42 [PATCH v2] bpf: add diagnostics for rejected memory and map accesses Suchit Karunakaran
2026-09-27 19:50 ` sashiko-bot
2026-09-28 7:42 ` Alexei Starovoitov
2026-09-28 10:04 ` Suchit Karunakaran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox