On Thu, Jan 5, 2023 at 1:47 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Wed, Jan 04, 2023 at 11:08:23PM +0100, Jiri Olsa wrote:
> > On Wed, Jan 04, 2023 at 10:42:02PM +0100, Jiri Olsa wrote:
> > > On Wed, Jan 04, 2023 at 04:24:27PM +0100, Victor Laforet wrote:
> > > > Ok thanks. As I understand, tp_btf/+ probes (specifically tp_btf/sched_switch that I need) cannot be sleepable? It is then not possible to read user space memory from the bpf code?
> > >
> > > yes, only fentry/fexit/fmod_ret, lsm, and kprobe/uprobe programs can be sleepable
>
> we actually allow to create tp_btf program with BPF_F_SLEEPABLE flag,
> because it's TRACING prog type, but still bpf program can't sleep when
> executed in tracepoint context.. so I wonder we should not allow to
> load it, Alexei?
>
> jirka
>
>
> ---
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 98a8051ce316..390621d79fbb 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -16755,10 +16755,14 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
> return -EINVAL;
> }
>
> - if (prog->aux->sleepable && prog->type != BPF_PROG_TYPE_TRACING &&
> - prog->type != BPF_PROG_TYPE_LSM && prog->type != BPF_PROG_TYPE_KPROBE) {
> - verbose(env, "Only fentry/fexit/fmod_ret, lsm, and kprobe/uprobe programs can be sleepable\n");
> - return -EINVAL;
> + if (prog->aux->sleepable) {
> + if ((prog->type == BPF_PROG_TYPE_TRACING &&
> + prog->expected_attach_type == BPF_TRACE_RAW_TP) ||
> + (prog->type != BPF_PROG_TYPE_TRACING &&
> + prog->type != BPF_PROG_TYPE_LSM && prog->type != BPF_PROG_TYPE_KPROBE)) {
> + verbose(env, "Only fentry/fexit/fmod_ret, lsm, and kprobe/uprobe programs can be sleepable\n");
> + return -EINVAL;
Ahh. good catch. The sleepable flag makes no difference for tp_btf.
Indeed, let's disable this combo.
I was thinking whether explicit list of
fentry/fexit/fmod_ret || lsm || kprobe would be cleaner?
