Re: [PATCH bpf 1/1] bpf: Fix insn_aux_data leak on verifier err_free_env path

[Anton Protopopov]

On 26/06/24 08:35PM, KaFai Wan wrote:
> When bpf_check() allocates env->insn_aux_data successfully but later
> fails to allocate env->succ, it jumps directly to err_free_env.
> 
> The existing vfree(env->insn_aux_data) sits before the err_free_env
> label, so that direct jump bypasses it and leaks insn_aux_data.
> 
> Move vfree(env->insn_aux_data) into err_free_env so all early and late
> exit paths release it consistently.
> 
> Fixes: 2f69c5685427 ("bpf: make bpf_insn_successors to return a pointer")
> Signed-off-by: KaFai Wan <kafai.wan@xxxxxxxxx>
> ---
>  kernel/bpf/verifier.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 21a365d436a5..3ccea8985946 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -19994,13 +19994,13 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
>  	if (!is_priv)
>  		mutex_unlock(&bpf_verifier_lock);
>  	bpf_clear_insn_aux_data(env, 0, env->prog->len);
> -	vfree(env->insn_aux_data);
>  err_free_env:
>  	bpf_stack_liveness_free(env);
>  	kvfree(env->cfg.insn_postorder);
>  	kvfree(env->scc_info);
>  	kvfree(env->succ);
>  	kvfree(env->gotox_tmp_buf);
> +	vfree(env->insn_aux_data);

Thanks!

Reviewed-by: Anton Protopopov <a.s.protopopov@xxxxxxxxx>

>  	kvfree(env);
>  	return ret;
>  }
> -- 
> 2.43.0
>