* [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program @ 2023-01-16 13:29 Jiri Olsa 2023-01-16 13:29 ` [PATCHv3 bpf-next 2/2] bpf/selftests: Add verifier tests for loading sleepable programs Jiri Olsa 2023-01-17 7:17 ` [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program Yonghong Song 0 siblings, 2 replies; 5+ messages in thread From: Jiri Olsa @ 2023-01-16 13:29 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Song Liu, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo Currently we allow to load any tracing program as sleepable, but BPF_TRACE_RAW_TP can't sleep. Making the check explicit for tracing programs attach types, so sleepable BPF_TRACE_RAW_TP will fail to load. Updating the verifier error to mention iter programs as well. Acked-by: Song Liu <song@kernel.org> Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- v3 changes: - use switch in can_be_sleepable [Alexei] - added acks [Song] kernel/bpf/verifier.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index fa4c911603e9..966dbfc14288 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -16743,6 +16743,23 @@ BTF_ID(func, rcu_read_unlock_strict) #endif BTF_SET_END(btf_id_deny) +static bool can_be_sleepable(struct bpf_prog *prog) +{ + if (prog->type == BPF_PROG_TYPE_TRACING) { + switch (prog->expected_attach_type) { + case BPF_TRACE_FENTRY: + case BPF_TRACE_FEXIT: + case BPF_MODIFY_RETURN: + case BPF_TRACE_ITER: + return true; + default: + return false; + } + } + return prog->type == BPF_PROG_TYPE_LSM || + prog->type == BPF_PROG_TYPE_KPROBE; +} + static int check_attach_btf_id(struct bpf_verifier_env *env) { struct bpf_prog *prog = env->prog; @@ -16761,9 +16778,8 @@ 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"); + if (prog->aux->sleepable && !can_be_sleepable(prog)) { + verbose(env, "Only fentry/fexit/fmod_ret, lsm, iter and kprobe/uprobe programs can be sleepable\n"); return -EINVAL; } -- 2.39.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCHv3 bpf-next 2/2] bpf/selftests: Add verifier tests for loading sleepable programs 2023-01-16 13:29 [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program Jiri Olsa @ 2023-01-16 13:29 ` Jiri Olsa 2023-01-17 7:17 ` [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program Yonghong Song 1 sibling, 0 replies; 5+ messages in thread From: Jiri Olsa @ 2023-01-16 13:29 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Song Liu, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo Adding verifier tests for loading all types od allowed sleepable programs plus reject for tp_btf type. Acked-by: Song Liu <song@kernel.org> Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- .../selftests/bpf/verifier/sleepable.c | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 tools/testing/selftests/bpf/verifier/sleepable.c diff --git a/tools/testing/selftests/bpf/verifier/sleepable.c b/tools/testing/selftests/bpf/verifier/sleepable.c new file mode 100644 index 000000000000..4248c3326b89 --- /dev/null +++ b/tools/testing/selftests/bpf/verifier/sleepable.c @@ -0,0 +1,91 @@ +{ + "sleepable fentry accept", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_TRACING, + .expected_attach_type = BPF_TRACE_FENTRY, + .kfunc = "bpf_fentry_test1", + .result = ACCEPT, + .flags = BPF_F_SLEEPABLE, + .runs = -1, +}, +{ + "sleepable fexit accept", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_TRACING, + .expected_attach_type = BPF_TRACE_FENTRY, + .kfunc = "bpf_fentry_test1", + .result = ACCEPT, + .flags = BPF_F_SLEEPABLE, + .runs = -1, +}, +{ + "sleepable fmod_ret accept", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_TRACING, + .expected_attach_type = BPF_MODIFY_RETURN, + .kfunc = "bpf_fentry_test1", + .result = ACCEPT, + .flags = BPF_F_SLEEPABLE, + .runs = -1, +}, +{ + "sleepable iter accept", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_TRACING, + .expected_attach_type = BPF_TRACE_ITER, + .kfunc = "task", + .result = ACCEPT, + .flags = BPF_F_SLEEPABLE, + .runs = -1, +}, +{ + "sleepable lsm accept", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_LSM, + .kfunc = "bpf", + .expected_attach_type = BPF_LSM_MAC, + .result = ACCEPT, + .flags = BPF_F_SLEEPABLE, + .runs = -1, +}, +{ + "sleepable kprobe accept", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_KPROBE, + .kfunc = "bpf_fentry_test1", + .result = ACCEPT, + .flags = BPF_F_SLEEPABLE, + .runs = -1, +}, +{ + "sleepable raw tracepoint reject", + .insns = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_TRACING, + .expected_attach_type = BPF_TRACE_RAW_TP, + .kfunc = "sched_switch", + .result = REJECT, + .errstr = "Only fentry/fexit/fmod_ret, lsm, iter and kprobe/uprobe programs can be sleepable", + .flags = BPF_F_SLEEPABLE, + .runs = -1, +}, -- 2.39.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program 2023-01-16 13:29 [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program Jiri Olsa 2023-01-16 13:29 ` [PATCHv3 bpf-next 2/2] bpf/selftests: Add verifier tests for loading sleepable programs Jiri Olsa @ 2023-01-17 7:17 ` Yonghong Song 2023-01-17 9:20 ` Jiri Olsa 1 sibling, 1 reply; 5+ messages in thread From: Yonghong Song @ 2023-01-17 7:17 UTC (permalink / raw) To: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Song Liu, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo On 1/16/23 5:29 AM, Jiri Olsa wrote: > Currently we allow to load any tracing program as sleepable, > but BPF_TRACE_RAW_TP can't sleep. Making the check explicit > for tracing programs attach types, so sleepable BPF_TRACE_RAW_TP > will fail to load. > > Updating the verifier error to mention iter programs as well. > > Acked-by: Song Liu <song@kernel.org> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> Ack with a minor comment below. Acked-by: Yonghong Song <yhs@fb.com> > --- > v3 changes: > - use switch in can_be_sleepable [Alexei] > - added acks [Song] > > kernel/bpf/verifier.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index fa4c911603e9..966dbfc14288 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -16743,6 +16743,23 @@ BTF_ID(func, rcu_read_unlock_strict) > #endif > BTF_SET_END(btf_id_deny) > > +static bool can_be_sleepable(struct bpf_prog *prog) > +{ > + if (prog->type == BPF_PROG_TYPE_TRACING) { > + switch (prog->expected_attach_type) { > + case BPF_TRACE_FENTRY: > + case BPF_TRACE_FEXIT: > + case BPF_MODIFY_RETURN: > + case BPF_TRACE_ITER: > + return true; > + default: > + return false; > + } > + } > + return prog->type == BPF_PROG_TYPE_LSM || > + prog->type == BPF_PROG_TYPE_KPROBE; > +} > + > static int check_attach_btf_id(struct bpf_verifier_env *env) > { > struct bpf_prog *prog = env->prog; > @@ -16761,9 +16778,8 @@ 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"); > + if (prog->aux->sleepable && !can_be_sleepable(prog)) { > + verbose(env, "Only fentry/fexit/fmod_ret, lsm, iter and kprobe/uprobe programs can be sleepable\n"); actually kprobe programs cannot be sleepable. See kernel/events/core.c. perf_event_set_bpf_prog(...) ... if (prog->type == BPF_PROG_TYPE_KPROBE && prog->aux->sleepable && !is_uprobe) /* only uprobe programs are allowed to be sleepable */ return -EINVAL; So I suggest to add a comment and remove the above 'kprobe' from error message. > return -EINVAL; > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program 2023-01-17 7:17 ` [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program Yonghong Song @ 2023-01-17 9:20 ` Jiri Olsa 2023-01-17 15:43 ` Yonghong Song 0 siblings, 1 reply; 5+ messages in thread From: Jiri Olsa @ 2023-01-17 9:20 UTC (permalink / raw) To: Yonghong Song Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Song Liu, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo On Mon, Jan 16, 2023 at 11:17:56PM -0800, Yonghong Song wrote: > > > On 1/16/23 5:29 AM, Jiri Olsa wrote: > > Currently we allow to load any tracing program as sleepable, > > but BPF_TRACE_RAW_TP can't sleep. Making the check explicit > > for tracing programs attach types, so sleepable BPF_TRACE_RAW_TP > > will fail to load. > > > > Updating the verifier error to mention iter programs as well. > > > > Acked-by: Song Liu <song@kernel.org> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > Ack with a minor comment below. > > Acked-by: Yonghong Song <yhs@fb.com> > > > --- > > v3 changes: > > - use switch in can_be_sleepable [Alexei] > > - added acks [Song] > > > > kernel/bpf/verifier.c | 22 +++++++++++++++++++--- > > 1 file changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > index fa4c911603e9..966dbfc14288 100644 > > --- a/kernel/bpf/verifier.c > > +++ b/kernel/bpf/verifier.c > > @@ -16743,6 +16743,23 @@ BTF_ID(func, rcu_read_unlock_strict) > > #endif > > BTF_SET_END(btf_id_deny) > > +static bool can_be_sleepable(struct bpf_prog *prog) > > +{ > > + if (prog->type == BPF_PROG_TYPE_TRACING) { > > + switch (prog->expected_attach_type) { > > + case BPF_TRACE_FENTRY: > > + case BPF_TRACE_FEXIT: > > + case BPF_MODIFY_RETURN: > > + case BPF_TRACE_ITER: > > + return true; > > + default: > > + return false; > > + } > > + } > > + return prog->type == BPF_PROG_TYPE_LSM || > > + prog->type == BPF_PROG_TYPE_KPROBE; > > +} > > + > > static int check_attach_btf_id(struct bpf_verifier_env *env) > > { > > struct bpf_prog *prog = env->prog; > > @@ -16761,9 +16778,8 @@ 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"); > > + if (prog->aux->sleepable && !can_be_sleepable(prog)) { > > + verbose(env, "Only fentry/fexit/fmod_ret, lsm, iter and kprobe/uprobe programs can be sleepable\n"); > > actually kprobe programs cannot be sleepable. See kernel/events/core.c. > perf_event_set_bpf_prog(...) > ... > > if (prog->type == BPF_PROG_TYPE_KPROBE && prog->aux->sleepable && > !is_uprobe) > /* only uprobe programs are allowed to be sleepable */ > return -EINVAL; > > So I suggest to add a comment and remove the above 'kprobe' from error > message. ok, is comment below ok? jirka --- diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index fa4c911603e9..ca7db2ce70b9 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -16743,6 +16743,23 @@ BTF_ID(func, rcu_read_unlock_strict) #endif BTF_SET_END(btf_id_deny) +static bool can_be_sleepable(struct bpf_prog *prog) +{ + if (prog->type == BPF_PROG_TYPE_TRACING) { + switch (prog->expected_attach_type) { + case BPF_TRACE_FENTRY: + case BPF_TRACE_FEXIT: + case BPF_MODIFY_RETURN: + case BPF_TRACE_ITER: + return true; + default: + return false; + } + } + return prog->type == BPF_PROG_TYPE_LSM || + prog->type == BPF_PROG_TYPE_KPROBE; /* only for uprobes */ +} + static int check_attach_btf_id(struct bpf_verifier_env *env) { struct bpf_prog *prog = env->prog; @@ -16761,9 +16778,8 @@ 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"); + if (prog->aux->sleepable && !can_be_sleepable(prog)) { + verbose(env, "Only fentry/fexit/fmod_ret, lsm, iter and uprobe programs can be sleepable\n"); return -EINVAL; } ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program 2023-01-17 9:20 ` Jiri Olsa @ 2023-01-17 15:43 ` Yonghong Song 0 siblings, 0 replies; 5+ messages in thread From: Yonghong Song @ 2023-01-17 15:43 UTC (permalink / raw) To: Jiri Olsa Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Song Liu, bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo On 1/17/23 1:20 AM, Jiri Olsa wrote: > On Mon, Jan 16, 2023 at 11:17:56PM -0800, Yonghong Song wrote: >> >> >> On 1/16/23 5:29 AM, Jiri Olsa wrote: >>> Currently we allow to load any tracing program as sleepable, >>> but BPF_TRACE_RAW_TP can't sleep. Making the check explicit >>> for tracing programs attach types, so sleepable BPF_TRACE_RAW_TP >>> will fail to load. >>> >>> Updating the verifier error to mention iter programs as well. >>> >>> Acked-by: Song Liu <song@kernel.org> >>> Signed-off-by: Jiri Olsa <jolsa@kernel.org> >> >> Ack with a minor comment below. >> >> Acked-by: Yonghong Song <yhs@fb.com> >> >>> --- >>> v3 changes: >>> - use switch in can_be_sleepable [Alexei] >>> - added acks [Song] >>> >>> kernel/bpf/verifier.c | 22 +++++++++++++++++++--- >>> 1 file changed, 19 insertions(+), 3 deletions(-) >>> >>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >>> index fa4c911603e9..966dbfc14288 100644 >>> --- a/kernel/bpf/verifier.c >>> +++ b/kernel/bpf/verifier.c >>> @@ -16743,6 +16743,23 @@ BTF_ID(func, rcu_read_unlock_strict) >>> #endif >>> BTF_SET_END(btf_id_deny) >>> +static bool can_be_sleepable(struct bpf_prog *prog) >>> +{ >>> + if (prog->type == BPF_PROG_TYPE_TRACING) { >>> + switch (prog->expected_attach_type) { >>> + case BPF_TRACE_FENTRY: >>> + case BPF_TRACE_FEXIT: >>> + case BPF_MODIFY_RETURN: >>> + case BPF_TRACE_ITER: >>> + return true; >>> + default: >>> + return false; >>> + } >>> + } >>> + return prog->type == BPF_PROG_TYPE_LSM || >>> + prog->type == BPF_PROG_TYPE_KPROBE; >>> +} >>> + >>> static int check_attach_btf_id(struct bpf_verifier_env *env) >>> { >>> struct bpf_prog *prog = env->prog; >>> @@ -16761,9 +16778,8 @@ 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"); >>> + if (prog->aux->sleepable && !can_be_sleepable(prog)) { >>> + verbose(env, "Only fentry/fexit/fmod_ret, lsm, iter and kprobe/uprobe programs can be sleepable\n"); >> >> actually kprobe programs cannot be sleepable. See kernel/events/core.c. >> perf_event_set_bpf_prog(...) >> ... >> >> if (prog->type == BPF_PROG_TYPE_KPROBE && prog->aux->sleepable && >> !is_uprobe) >> /* only uprobe programs are allowed to be sleepable */ >> return -EINVAL; >> >> So I suggest to add a comment and remove the above 'kprobe' from error >> message. > > ok, is comment below ok? Thanks! Sounds good to me. > > jirka > > > --- > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index fa4c911603e9..ca7db2ce70b9 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -16743,6 +16743,23 @@ BTF_ID(func, rcu_read_unlock_strict) > #endif > BTF_SET_END(btf_id_deny) > > +static bool can_be_sleepable(struct bpf_prog *prog) > +{ > + if (prog->type == BPF_PROG_TYPE_TRACING) { > + switch (prog->expected_attach_type) { > + case BPF_TRACE_FENTRY: > + case BPF_TRACE_FEXIT: > + case BPF_MODIFY_RETURN: > + case BPF_TRACE_ITER: > + return true; > + default: > + return false; > + } > + } > + return prog->type == BPF_PROG_TYPE_LSM || > + prog->type == BPF_PROG_TYPE_KPROBE; /* only for uprobes */ > +} > + > static int check_attach_btf_id(struct bpf_verifier_env *env) > { > struct bpf_prog *prog = env->prog; > @@ -16761,9 +16778,8 @@ 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"); > + if (prog->aux->sleepable && !can_be_sleepable(prog)) { > + verbose(env, "Only fentry/fexit/fmod_ret, lsm, iter and uprobe programs can be sleepable\n"); > return -EINVAL; > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-17 15:46 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-16 13:29 [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program Jiri Olsa 2023-01-16 13:29 ` [PATCHv3 bpf-next 2/2] bpf/selftests: Add verifier tests for loading sleepable programs Jiri Olsa 2023-01-17 7:17 ` [PATCHv3 bpf-next 1/2] bpf: Do not allow to load sleepable BPF_TRACE_RAW_TP program Yonghong Song 2023-01-17 9:20 ` Jiri Olsa 2023-01-17 15:43 ` Yonghong Song
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.