netdev - Re: [RFC bpf-next] bpf: Use prog->active instead of bpf_prog_active for kprobe

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]

Date:   Tue, 31 May 2022 16:24:22 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Jiri Olsa <jolsa@...nel.org>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>
Subject: Re: [RFC bpf-next] bpf: Use prog->active instead of bpf_prog_active
 for kprobe_multi

On Wed, May 25, 2022 at 4:40 AM Jiri Olsa <jolsa@...nel.org> wrote:
>
> hi,
> Alexei suggested to use prog->active instead global bpf_prog_active
> for programs attached with kprobe multi [1].
>
> AFAICS this will bypass bpf_disable_instrumentation, which seems to be
> ok for some places like hash map update, but I'm not sure about other
> places, hence this is RFC post.
>
> I'm not sure how are kprobes different to trampolines in this regard,
> because trampolines use prog->active and it's not a problem.
>
> thoughts?
>

Let's say we have two kernel functions A and B? B can be called from
BPF program though some BPF helper, ok? Now let's say I have two BPF
programs kprobeX and kretprobeX, both are attached to A and B. With
using prog->active instead of per-cpu bpf_prog_active, what would be
the behavior when A is called somewhere in the kernel.

1. A is called
2. kprobeX is activated for A, calls some helper which eventually calls B
  3. kprobeX is attempted to be called for B, but is skipped due to prog->active
  4. B runs
  5. kretprobeX is activated for B, calls some helper which eventually calls B
    6. kprobeX is ignored (prog->active > 0)
    7. B runs
    8. kretprobeX is ignored (prog->active > 0)
9. kretprobeX is activated for A, calls helper which calls B
  10. kprobeX is activated for B
    11. kprobeX is ignored (prog->active > 0)
    12. B runs
    13. kretprobeX is ignored (prog->active > 0)
  14. B runs
  15. kretprobeX is ignored (prog->active > 0)

If that's correct, we get:

1. kprobeX for A
2. kretprobeX for B
3. kretprobeX for A
4. kprobeX for B

It's quite mind-boggling and annoying in practice. I'd very much
prefer just kprobeX for A followed by kretprobeX for A. That's it.

I'm trying to protect against this in retsnoop with custom per-cpu
logic in each program, but I so much more prefer bpf_prog_active,
which basically says "no nested kprobe calls while kprobe program is
running", which makes a lot of sense in practice.

Given kprobe already used global bpf_prog_active I'd say multi-kprobe
should stick to bpf_prog_active as well.

> thanks,
> jirka
>
>
> [1] https://lore.kernel.org/bpf/20220316185333.ytyh5irdftjcklk6@ast-mbp.dhcp.thefacebook.com/
> ---
>  kernel/trace/bpf_trace.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
>

[...]