Re: [PATCH v4 2/5] bpf/verifier: do not limit maximum direct offset into arena map

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2025-12-16 at 10:38 -0800, Andrii Nakryiko wrote:
> On Tue, Dec 16, 2025 at 10:02 AM <bot+bpf-ci@kernel.org> wrote:
> > 
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index a31c032b2dd6..d6b8a77fbe3b 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -21132,11 +21132,6 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
> > >               } else {
> > >                       u32 off = insn[1].imm;
> > > 
> > > -                     if (off >= BPF_MAX_VAR_OFF) {
> > > -                             verbose(env, "direct value offset of %u is not allowed\n", off);
> > > -                             return -EINVAL;
> > > -                     }
> > > -
> > >                       if (!map->ops->map_direct_value_addr) {
> > >                               verbose(env, "no direct value access support for this map type\n");
> > >                               return -EINVAL;
> > 
> > The map_direct_value_addr() implementations for array and arena maps
> > return only the base address, and then resolve_pseudo_ldimm64() adds the
> > offset at line 21148:
> > 
> >     aux->map_off = off;
> >     addr += off;
> > 
> > However, insn_array_map_direct_value_addr() (kernel/bpf/bpf_insn_array.c)
> > has a different implementation pattern:
> > 
> >     static int insn_array_map_direct_value_addr(const struct bpf_map *map,
> >                                                  u64 *imm, u32 off)
> >     {
> >         struct bpf_insn_array *insn_array = cast_insn_array(map);
> > 
> >         if ((off % sizeof(long)) != 0 ||
> >             (off / sizeof(long)) >= map->max_entries)
> >             return -EINVAL;
> > 
> >         /* from BPF's point of view, this map is a jump table */
> >         *imm = (unsigned long)insn_array->ips + off;
> > 
> >         return 0;
> >     }
> > 
> > Notice that insn_array adds the offset to the base address inside the
> > function (line: *imm = (unsigned long)insn_array->ips + off). When
> > resolve_pseudo_ldimm64() then adds the offset again at line 21148, the
> > result is:
> > 
> >     addr = (insn_array->ips + off) + off = insn_array->ips + 2*off
> > 
> > Can this cause insn_array maps to access incorrect array elements? For
> > example, if a program requests offset 32 (which passes the bounds check
> > since 32/8 = 4 < max_entries), the actual offset becomes 64, potentially
> > accessing insn_array->ips[8] when max_entries is only 10.
> 
> That's a question to Anton (cc'ed), not really related to this patch.
> Anton, please check and send a fix, if necessary.

Seem to be a valid concern.
We don't have tests for BPF_PSEUDO_MAP_VALUE used in combination with
instruction maps.

[...]