Re: [PATCH v11 bpf-next 01/12] bpf, x86: add new map type: instructions array

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 25/11/21 06:40PM, Alexei Starovoitov wrote:
> On Sun, Nov 16, 2025 at 4:51 AM Anton Protopopov
> <a.s.protopopov@gmail.com> wrote:
> >
> > On 25/11/06 09:08AM, Alexei Starovoitov wrote:
> > > On Thu, Nov 6, 2025 at 1:54 AM Anton Protopopov
> > > <a.s.protopopov@gmail.com> wrote:
> > > >
> > > > On 25/11/05 06:03PM, Alexei Starovoitov wrote:
> > > > > On Wed, Nov 5, 2025 at 12:58 AM Anton Protopopov
> > > > > <a.s.protopopov@gmail.com> wrote:
> > > > > > @@ -21695,6 +21736,8 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> > > > > >                 func[i]->aux->jited_linfo = prog->aux->jited_linfo;
> > > > > >                 func[i]->aux->linfo_idx = env->subprog_info[i].linfo_idx;
> > > > > >                 func[i]->aux->arena = prog->aux->arena;
> > > > > > +               func[i]->aux->used_maps = env->used_maps;
> > > > > > +               func[i]->aux->used_map_cnt = env->used_map_cnt;
> > > > >
> > > > > ...
> > > > >
> > > > > > It might be called before the used_maps are copied into aux...
> > > > >
> > > > > wat?
> > > >
> > > > It is called from fixup_call_arg() which happens before
> > > > the env->prog->aux->used_maps is populated as a copy of
> > > > env->used_maps.
> > > >
> > > > In any case, I will take a closer look and follow up on
> > > > this after Kubecon (which is the next week).
> > >
> > > Pls look at the diff
> > > and also
> > > line 22074:
> > > func[i]->aux->main_prog_aux = prog->aux;
> > > line 22099:
> > > func[i]->aux->used_maps = env->used_maps;
> >
> > [Sorry for the delay, I was travelling and didn't have access to my lab.]
> >
> > I've seen this diff and tested it before sending the previous reply.
> > It didn't work, and it doesn't work now on bpf-next/master: the
> > "./test_progs -a bpf_insn_array/deletions-with-functions" test
> > still breaks.
> >
> > The reason is as follows. There are two cases for which JIT is called
> > differently.
> >
> > 1) For a program without sub-functions the JIT is called from the
> > bpf_prog_select_runtime() function. By this time
> > aux->main_prog_aux->used_maps are populated and thus
> > aux->main_prog_aux could be used; or just aux, as there is only one
> > prog.
> >
> > 2) When program has sub-functions, say one, the jit is called from
> > jit_subprogs() and later the call in bpf_prog_select_runtime() is
> > skipped. The jit_subprogs() is called before the bpf_check()
> > epilogue, and thus not func[i]->aux nor aux->main_prog_aux
> > contain a copy of used_maps, it is only copied later.
> >
> > To make two cases look the same ("aux->used_maps is correct"), I've
> > added
> >
> >     func[i]->aux->used_maps = env->used_maps;
> >     func[i]->aux->used_map_cnt = env->used_map_cnt;
> >
> > Note, again, that in case 2, without this copy, no functions will
> > have used_maps set, even main_prog.
> 
> I see. Thanks for explaining. That's one more reason to split prog and subprog.
> Could you please follow up with the patch to clear them back
> after JITing:
> func[i]->aux->used_maps = NULL;

Thanks, makes sense, sent. (Will send a few more follow ups later this week.)

> it's not great to have pointers to freed memory sitting there
> for the lifetime of the program. I suspect it might confuse
> tools like kmemleak that simply scan 8 byte values.