Re: [PATCH v2] bpf: add diagnostics for rejected memory and map accesses

[Suchit Karunakaran]

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@xxxxxxxxx> wrote:
>
> On Mon, Sep 28, 2026 at 01:12 AM Suchit Karunakaran <suchitkarunakaran@xxxxxxxxx> 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.