netdev - Re: [PATCH bpf v2 3/4] bpf: track subprog poke correctly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAADnVQ+PKYytXscJggO5AfWUsnevJTQAzbgXg8WcKpvi8K-EKA@mail.gmail.com>
Date:   Tue, 22 Jun 2021 16:07:42 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     "Fijalkowski, Maciej" <maciej.fijalkowski@...el.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andriin@...com>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf v2 3/4] bpf: track subprog poke correctly

On Tue, Jun 22, 2021 at 4:02 PM John Fastabend <john.fastabend@...il.com> wrote:
>
> Alexei Starovoitov wrote:
> > On Wed, Jun 16, 2021 at 03:55:39PM -0700, John Fastabend wrote:
> > >
> > > -static void bpf_free_used_maps(struct bpf_prog_aux *aux)
> > > +void bpf_free_used_maps(struct bpf_prog_aux *aux)
> > >  {
> > >     __bpf_free_used_maps(aux, aux->used_maps, aux->used_map_cnt);
> > >     kfree(aux->used_maps);
> > > @@ -2211,8 +2211,10 @@ static void bpf_prog_free_deferred(struct work_struct *work)
> > >  #endif
> > >     if (aux->dst_trampoline)
> > >             bpf_trampoline_put(aux->dst_trampoline);
> > > -   for (i = 0; i < aux->func_cnt; i++)
> > > +   for (i = 0; i < aux->func_cnt; i++) {
> > > +           bpf_free_used_maps(aux->func[i]->aux);
> > >             bpf_jit_free(aux->func[i]);
> > > +   }
> >
> > The sub-progs don't have all the properties of the main prog.
> > Only main prog suppose to keep maps incremented.
> > After this patch the prog with 100 subprogs will artificially bump maps
> > refcnt 100 times as a workaround for poke_tab access.
>
> Yep.
>
> > May be we can use single poke_tab in the main prog instead.
> > Looks like jit_subprogs is splitting the poke_tab into individual arrays
> > for each subprog, but maps are tracked by the main prog only.
> > That's the root cause of the issue, right?
>
> Correct.
>
> > I think that split of poke_tab isn't necessary.
> > bpf_int_jit_compile() can look into main prog poke_tab instead.
> > Then the loop:
> > for (j = 0; j < prog->aux->size_poke_tab)
> >     bpf_jit_add_poke_descriptor(func[i], &prog->aux->poke_tab[j]);
> > can be removed (It will address the concern in patch 2 as well, right?)
> > And hopefully will fix UAF too?
>
> Looks like it to me as well. A few details to work out around
> imm value and emit hooks on the jit side, but looks doable to me.
> I'll give it a try tomorrow or tonight.

Perfect. Thank you John.