BPF List
 help / color / mirror / Atom feed
* [PATCH] bpf: add diagnostics for rejected memory and map accesses
@ 2026-09-21 15:46 Suchit Karunakaran
  2026-09-21 15:59 ` sashiko-bot
  2026-09-21 17:05 ` Alexei Starovoitov
  0 siblings, 2 replies; 7+ messages in thread
From: Suchit Karunakaran @ 2026-09-21 15:46 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>
---
 kernel/bpf/fixups.c   |  5 +++++
 kernel/bpf/verifier.c | 16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index 2add8001c3ec..b8baf77d933e 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -9,6 +9,7 @@
 #include <linux/sort.h>
 #include <linux/perf_event.h>
 #include <net/xdp.h>
+#include "diagnostics.h"
 #include "disasm.h"
 
 #define verbose(env, fmt, args...) bpf_verifier_log_write(env, fmt, ##args)
@@ -958,6 +959,10 @@ 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",
+						"the current JIT does not support this load instruction for arena memory",
+						"Use a kernel and architecture with JIT support for this arena load.");
 					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 3a676fc70f4b..ebfa53930276 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4365,12 +4365,18 @@ 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, "map value write",
+				"this map does not permit BPF programs to write its values",
+				"Remove the write, or use a map that allows writes from BPF programs.");
 		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, "map value read",
+				 "this map does not permit BPF programs to read its values",
+				 "Remove the read, or use a map that allows reads from BPF programs.");
 		return -EACCES;
 	}
 
@@ -6527,6 +6533,9 @@ 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, "write through a read-only memory pointer",
+					"this pointer permits reads only",
+					"Use a writable destination, or copy the data into a writable buffer before modifying it.");
 			return -EACCES;
 		}
 
@@ -6613,6 +6622,9 @@ 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",
+					"direct packet writes are disabled 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 &&
@@ -11188,6 +11200,10 @@ 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", func_id_name(func_id)),
+				"this helper modifies the map, but BPF_F_RDONLY_PROG forbids modifications from BPF programs",
+				"Remove the modifying helper call, or use a map created without BPF_F_RDONLY_PROG if updates from BPF programs are intended.");
 		return -EACCES;
 	}
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] bpf: add diagnostics for rejected memory and map accesses
  2026-09-21 15:46 [PATCH] bpf: add diagnostics for rejected memory and map accesses Suchit Karunakaran
@ 2026-09-21 15:59 ` sashiko-bot
  2026-09-21 17:05 ` Alexei Starovoitov
  1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-21 15:59 UTC (permalink / raw)
  To: Suchit Karunakaran; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: incomplete diagnostic coverage for read-only and packet writes
--

commit 4bb7159fc40bb3fd0415a49f32660f99d3a92102
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
> index 18aad4886f9cf..a4ca95168a01f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -6529,6 +6535,9 @@ 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, "write through a read-only memory pointer",
> +					"this pointer permits reads only",
> +					"Use a writable destination, or copy the data into a writable buffer before modifying it.");
>  			return -EACCES;
>  		}

[Severity: Low]
This isn't a bug introduced by this patch, but does the check for PTR_TO_BUF
also need this diagnostic when rejecting direct writes in check_mem_access()?

kernel/bpf/verifier.c: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;
			}
	...
}

Should writes to read-only PTR_TO_MEM and PTR_TO_BUF via helper function
arguments also emit the structured diagnostic?

kernel/bpf/verifier.c:check_helper_mem_access() {
	...
	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;
			}
		}
	...
}

[ ... ]
> @@ -6608,6 +6617,9 @@ 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",
> +					"direct packet writes are disabled for this program type",
> +					"Remove the direct write, or perform the modification in a program type and hook that support packet writes.");
>  			return -EACCES;
>  		}

[Severity: Low]
This is a pre-existing issue, but should the same diagnostic be emitted when
packet writes are rejected via helper function arguments?

kernel/bpf/verifier.c:check_helper_mem_access() {
	...
	case PTR_TO_PACKET:
	case PTR_TO_PACKET_META:
		if (!may_access_direct_pkt_data(env, meta, access_type)) {
			verbose(env, "function access to the packet is not allowed\n");
			return -EACCES;
		}
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921154650.130677-1-suchitkarunakaran@gmail.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] bpf: add diagnostics for rejected memory and map accesses
  2026-09-21 15:46 [PATCH] bpf: add diagnostics for rejected memory and map accesses Suchit Karunakaran
  2026-09-21 15:59 ` sashiko-bot
@ 2026-09-21 17:05 ` Alexei Starovoitov
  2026-09-21 17:38   ` Suchit Karunakaran
  1 sibling, 1 reply; 7+ messages in thread
From: Alexei Starovoitov @ 2026-09-21 17:05 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 21, 2026 at 09:16 PM Suchit Karunakaran <suchitkarunakaran@gmail.com> wrote:
>  					verbose(env, "sign extending loads from arena are not supported yet\n");
> +					bpf_diag_policy(
> +						env, i + delta, "sign-extending arena load",
> +						"the current JIT does not support this load instruction for arena memory",
> +						"Use a kernel and architecture with JIT support for this arena load.");

Did you read what AI generated for you?
"Use a kernel and architecture with JIT support for this arena load."
?!?!

Please apply human filter to every single line of "your" patches.

pw-bot: cr

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] bpf: add diagnostics for rejected memory and map accesses
  2026-09-21 17:05 ` Alexei Starovoitov
@ 2026-09-21 17:38   ` Suchit Karunakaran
  2026-09-21 19:00     ` Alexei Starovoitov
  0 siblings, 1 reply; 7+ messages in thread
From: Suchit Karunakaran @ 2026-09-21 17:38 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

On Mon, 21 Sept 2026 at 22:35, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Sep 21, 2026 at 09:16 PM Suchit Karunakaran <suchitkarunakaran@gmail.com> wrote:
> >                                       verbose(env, "sign extending loads from arena are not supported yet\n");
> > +                                     bpf_diag_policy(
> > +                                             env, i + delta, "sign-extending arena load",
> > +                                             "the current JIT does not support this load instruction for arena memory",
> > +                                             "Use a kernel and architecture with JIT support for this arena load.");
>
> Did you read what AI generated for you?
> "Use a kernel and architecture with JIT support for this arena load."
> ?!?!
>
> Please apply human filter to every single line of "your" patches.
>
> pw-bot: cr

Hi Alexei. I'm sorry for the oversight. I somehow misinterpreted it as
"Use a kernel and architecture with JIT that supports this arena
load." Is the following diagnostic message fine? If yes I'll correct
it in v2.
bpf_diag_policy(env, i + delta,
"sign-extending arena load", "the current JIT backend doesn't
implement sign-extending loads from arena memory",
 "Use a kernel and architecture where the BPF JIT supports
sign-extending loads from arena memory.");

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] bpf: add diagnostics for rejected memory and map accesses
  2026-09-21 17:38   ` Suchit Karunakaran
@ 2026-09-21 19:00     ` Alexei Starovoitov
  2026-09-22  5:49       ` Suchit Karunakaran
  0 siblings, 1 reply; 7+ messages in thread
From: Alexei Starovoitov @ 2026-09-21 19:00 UTC (permalink / raw)
  To: Suchit Karunakaran
  Cc: daniel, andrii, eddyz87, memxor, martin.lau, song, yonghong.song,
	jolsa, emil, ihor.solodrai, john.fastabend, bpf, linux-kernel

On Mon Sep 21, 2026 at 5:38 PM UTC, Suchit Karunakaran wrote:
> On Mon, 21 Sept 2026 at 22:35, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Mon, Sep 21, 2026 at 09:16 PM Suchit Karunakaran <suchitkarunakaran@gmail.com> wrote:
> > >                                       verbose(env, "sign extending loads from arena are not supported yet\n");
> > > +                                     bpf_diag_policy(
> > > +                                             env, i + delta, "sign-extending arena load",
> > > +                                             "the current JIT does not support this load instruction for arena memory",
> > > +                                             "Use a kernel and architecture with JIT support for this arena load.");
> >
> > Did you read what AI generated for you?
> > "Use a kernel and architecture with JIT support for this arena load."
> > ?!?!
> >
> > Please apply human filter to every single line of "your" patches.
> >
> > pw-bot: cr
>
> Hi Alexei. I'm sorry for the oversight. I somehow misinterpreted it as
> "Use a kernel and architecture with JIT that supports this arena
> load." Is the following diagnostic message fine? If yes I'll correct
> it in v2.
> bpf_diag_policy(env, i + delta,
> "sign-extending arena load", "the current JIT backend doesn't
> implement sign-extending loads from arena memory",
>  "Use a kernel and architecture where the BPF JIT supports
> sign-extending loads from arena memory.");

No. That is still wrong. Put yourself in user's shoes.
How would you react to this message?


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] bpf: add diagnostics for rejected memory and map accesses
  2026-09-21 19:00     ` Alexei Starovoitov
@ 2026-09-22  5:49       ` Suchit Karunakaran
  2026-09-25  1:42         ` Alexei Starovoitov
  0 siblings, 1 reply; 7+ messages in thread
From: Suchit Karunakaran @ 2026-09-22  5:49 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

On Tue, 22 Sept 2026 at 00:30, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon Sep 21, 2026 at 5:38 PM UTC, Suchit Karunakaran wrote:
> > On Mon, 21 Sept 2026 at 22:35, Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Mon, Sep 21, 2026 at 09:16 PM Suchit Karunakaran <suchitkarunakaran@gmail.com> wrote:
> > > >                                       verbose(env, "sign extending loads from arena are not supported yet\n");
> > > > +                                     bpf_diag_policy(
> > > > +                                             env, i + delta, "sign-extending arena load",
> > > > +                                             "the current JIT does not support this load instruction for arena memory",
> > > > +                                             "Use a kernel and architecture with JIT support for this arena load.");
> > >
> > > Did you read what AI generated for you?
> > > "Use a kernel and architecture with JIT support for this arena load."
> > > ?!?!
> > >
> > > Please apply human filter to every single line of "your" patches.
> > >
> > > pw-bot: cr
> >
> > Hi Alexei. I'm sorry for the oversight. I somehow misinterpreted it as
> > "Use a kernel and architecture with JIT that supports this arena
> > load." Is the following diagnostic message fine? If yes I'll correct
> > it in v2.
> > bpf_diag_policy(env, i + delta,
> > "sign-extending arena load", "the current JIT backend doesn't
> > implement sign-extending loads from arena memory",
> >  "Use a kernel and architecture where the BPF JIT supports
> > sign-extending loads from arena memory.");
>
> No. That is still wrong. Put yourself in user's shoes.
> How would you react to this message?
>

Hi Alexei, thanks for the review. After some research, I found that
the sign extending load instruction was introduced in BPF ISA V4.
Given that, would the following diagnostic message be more helpful for
users?
bpf_diag_policy(env, i + delta,
"sign-extending load from arena memory",
"the current JIT backend doesn't implement sign-extending loads from
arena memory",
"Recompile the BPF program with Clang's -mcpu=v3 to avoid generating
sign-extending load instructions.");

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] bpf: add diagnostics for rejected memory and map accesses
  2026-09-22  5:49       ` Suchit Karunakaran
@ 2026-09-25  1:42         ` Alexei Starovoitov
  0 siblings, 0 replies; 7+ messages in thread
From: Alexei Starovoitov @ 2026-09-25  1:42 UTC (permalink / raw)
  To: Suchit Karunakaran
  Cc: daniel, andrii, eddyz87, memxor, martin.lau, song, yonghong.song,
	jolsa, emil, ihor.solodrai, john.fastabend, bpf, linux-kernel

On Tue, Sep 22, 2026 at 11:19 AM Suchit Karunakaran <suchitkarunakaran@gmail.com> wrote:
> Hi Alexei, thanks for the review. After some research, I found that
> the sign extending load instruction was introduced in BPF ISA V4.
> Given that, would the following diagnostic message be more helpful for
> users?
> bpf_diag_policy(env, i + delta,
> "sign-extending load from arena memory",
> "the current JIT backend doesn't implement sign-extending loads from
> arena memory",
> "Recompile the BPF program with Clang's -mcpu=v3 to avoid generating
> sign-extending load instructions.");

Yes. That's something the user can do.

But look at what bpf_diag_policy() prints with these strings:

  The sign-extending load from arena memory is not allowed: the current
  JIT backend doesn't implement sign-extending loads from arena memory.

The same thing twice.

The other messages in the patch need the same treatment.
Read every report the way the user will see it before sending v2.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-25  1:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-21 15:46 [PATCH] bpf: add diagnostics for rejected memory and map accesses Suchit Karunakaran
2026-09-21 15:59 ` sashiko-bot
2026-09-21 17:05 ` Alexei Starovoitov
2026-09-21 17:38   ` Suchit Karunakaran
2026-09-21 19:00     ` Alexei Starovoitov
2026-09-22  5:49       ` Suchit Karunakaran
2026-09-25  1:42         ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox