On Tue, Jul 5, 2022 at 5:26 PM Song Liu <song@xxxxxxxxxx> wrote:
>
> syzbot reported a few issues with bpf_prog_pack [1], [2]. These are
> triggered when the program passed initial JIT in jit_subprogs(), but
> failed final pass of JIT. At this point, bpf_jit_binary_pack_free() is
> called before bpf_jit_binary_pack_finalize(), and the whole 2MB page is
> freed.
>
> Fix this with a custom bpf_jit_free() for x86_64, which calls
> bpf_jit_binary_pack_finalize() if necessary. Also, with custom
> bpf_jit_free(), bpf_prog_aux->use_bpf_prog_pack is not needed any more,
> remove it.
>
> Fixes: 1022a5498f6f ("bpf, x86_64: Use bpf_jit_binary_pack_alloc")
> [1] https://syzkaller.appspot.com/bug?extid=2f649ec6d2eea1495a8f
> [2] https://syzkaller.appspot.com/bug?extid=87f65c75f4a72db05445
> Reported-by: syzbot+2f649ec6d2eea1495a8f@xxxxxxxxxxxxxxxxxxxxxxxxx
> Reported-by: syzbot+87f65c75f4a72db05445@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Song Liu <song@xxxxxxxxxx>
> ---
> arch/x86/net/bpf_jit_comp.c | 25 +++++++++++++++++++++++++
> include/linux/bpf.h | 1 -
> include/linux/filter.h | 8 ++++++++
> kernel/bpf/core.c | 29 ++++++++++++-----------------
> 4 files changed, 45 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index c98b8c0ed3b8..c3dca4c97e48 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2492,3 +2492,28 @@ void *bpf_arch_text_copy(void *dst, void *src, size_t len)
> return ERR_PTR(-EINVAL);
> return dst;
> }
> +
> +void bpf_jit_free(struct bpf_prog *prog)
> +{
> + if (prog->jited) {
> + struct x64_jit_data *jit_data = prog->aux->jit_data;
> + struct bpf_binary_header *hdr;
> +
> + /*
> + * If we fail the final pass of JIT (from jit_subprogs),
> + * the program may not be finalized yet. Call finalize here
> + * before freeing it.
> + */
> + if (jit_data) {
> + bpf_jit_binary_pack_finalize(prog, jit_data->header,
> + jit_data->rw_header);
> + kvfree(jit_data->addrs);
> + kfree(jit_data);
> + }
It looks like a workaround for missed cleanup on the JIT side.
When bpf_int_jit_compile() fails it is supposed to free jit_data
immediately.
> passed initial JIT in jit_subprogs(), but
> failed final pass of JIT. At this point, bpf_jit_binary_pack_free() is
> called before bpf_jit_binary_pack_finalize()
It feels that bpf_int_jit_compile() should call
bpf_jit_binary_pack_finalize() instead in the path where
it's failing.
I could be missing details on what exactly
"failed final pass of JIT" means.
