On Tue, May 13, 2025 at 12:54 AM Viktor Malik <vmalik@xxxxxxxxxx> wrote: > > On 5/13/25 08:48, Matt Bobrowski wrote: > > On Fri, May 09, 2025 at 09:20:53AM -0700, Alexei Starovoitov wrote: > >> On Thu, May 8, 2025 at 2:09 AM Matt Bobrowski <mattbobrowski@xxxxxxxxxx> wrote: > >>> > >>>> > >>>> static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, > >>>> - u32 regno, u32 mem_size) > >>>> + u32 regno, u32 mem_size, bool read_only) > >>> > >>> Maybe s/read_only/write_mem_access? > >> > >> 'bool' arguments are not readable at the callsite. > >> Let's use enum bpf_access_type BPF_READ|WRITE here > >> or introduce another enum ? > > > > Yes, I agree, and using enum bpf_access_type is also something that > > had crossed my mind. I think that's what should be used here in favour > > of the boolean. > > Reusing bpf_access_type feels like the right thing here, however, it is > missing an option for read/write access. Should we introduce a new > BPF_READ_WRITE enum value? Or assume that BPF_WRITE should also perform > the BPF_READ check? Or make this argument an int and pass > `BPF_READ | BPF_WRITE`? I think most, if not all, other places in the verifier assume that BPF_WRITE also means that read has to work. So I don't see a need for new enum values.
