On Fri, Dec 22, 2023 at 7:12 AM Dmitrii Dolgov <9erthalion6@xxxxxxxxx> wrote:
>
> From: Jiri Olsa <olsajiri@xxxxxxxxx>
>
> The following case can cause a crash due to missing attach_btf:
>
> 1) load rawtp program
> 2) load fentry program with rawtp as target_fd
> 3) create tracing link for fentry program with target_fd = 0
> 4) repeat 3
>
> In the end we have:
>
> - prog->aux->dst_trampoline == NULL
> - tgt_prog == NULL (because we did not provide target_fd to link_create)
> - prog->aux->attach_btf == NULL (the program was loaded with attach_prog_fd=X)
> - the program was loaded for tgt_prog but we have no way to find out which one
>
> BUG: kernel NULL pointer dereference, address: 0000000000000058
> Call Trace:
> <TASK>
> ? __die+0x20/0x70
> ? page_fault_oops+0x15b/0x430
> ? fixup_exception+0x22/0x330
> ? exc_page_fault+0x6f/0x170
> ? asm_exc_page_fault+0x22/0x30
> ? bpf_tracing_prog_attach+0x279/0x560
> ? btf_obj_id+0x5/0x10
> bpf_tracing_prog_attach+0x439/0x560
> __sys_bpf+0x1cf4/0x2de0
> __x64_sys_bpf+0x1c/0x30
> do_syscall_64+0x41/0xf0
> entry_SYSCALL_64_after_hwframe+0x6e/0x76
>
> Return -EINVAL in this situation.
>
> Signed-off-by: Jiri Olsa <olsajiri@xxxxxxxxx>
> Acked-by: Jiri Olsa <olsajiri@xxxxxxxxx>
> Signed-off-by: Dmitrii Dolgov <9erthalion6@xxxxxxxxx>
Acked-by: Song Liu <song@xxxxxxxxxx>
I guess we also need:
Fixes: f3a95075549e0 ("bpf: Allow trampoline re-attach for tracing and
lsm programs")
Cc: stable@xxxxxxxxxxxxxxx
> ---
> kernel/bpf/syscall.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index c40cad8886e9..5096ddfbe426 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3201,6 +3201,10 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> *
> * - if prog->aux->dst_trampoline and tgt_prog is NULL, the program
> * was detached and is going for re-attachment.
> + *
> + * - if prog->aux->dst_trampoline is NULL and tgt_prog and prog->aux->attach_btf
> + * are NULL, then program was already attached and user did not provide
> + * tgt_prog_fd so we have no way to find out or create trampoline
> */
> if (!prog->aux->dst_trampoline && !tgt_prog) {
> /*
> @@ -3214,6 +3218,11 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> err = -EINVAL;
> goto out_unlock;
> }
> + /* We can allow re-attach only if we have valid attach_btf. */
> + if (!prog->aux->attach_btf) {
> + err = -EINVAL;
> + goto out_unlock;
> + }
> btf_id = prog->aux->attach_btf_id;
> key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf, btf_id);
> }
> --
> 2.41.0
>
