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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210622220037.6uwrba6tl7vofcu5@ast-mbp.dhcp.thefacebook.com>
Date:   Tue, 22 Jun 2021 15:00:37 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     maciej.fijalkowski@...el.com, ast@...nel.org, daniel@...earbox.net,
        andriin@...com, netdev@...r.kernel.org
Subject: Re: [PATCH bpf v2 3/4] bpf: track subprog poke correctly

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.
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?
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?