| Message ID | 9bae67335d76cfffadf9777be79c32c0f1deb897.1651103126.git.delyank@fb.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | sleepable uprobe support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for bpf-next, async |
| netdev/apply | fail | Patch does not apply to bpf-next |
| bpf/vmtest-bpf-next-PR | fail | merge-conflict |
On Thu, Apr 28, 2022 at 9:54 AM Delyan Kratunov <delyank@fb.com> wrote: > > uprobe and kprobe programs have the same program type, KPROBE, which is > currently not allowed to load sleepable programs. > > To avoid adding a new UPROBE type, we instead allow sleepable KPROBE > programs to load and defer the is-it-actually-a-uprobe-program check > to attachment time, where we're already validating the corresponding > perf_event. > > A corollary of this patch is that you can now load a sleepable kprobe > program but cannot attach it. > > Signed-off-by: Delyan Kratunov <delyank@fb.com> > --- > kernel/bpf/syscall.c | 8 ++++++++ > kernel/bpf/verifier.c | 4 ++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index e9e3e49c0eb7..3ce923f489d7 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3009,6 +3009,14 @@ static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *pro > } > > event = perf_file->private_data; > + if (prog->aux->sleepable) { > + if (!event->tp_event || (event->tp_event->flags & TRACE_EVENT_FL_UPROBE) == 0) { so far TRACE_EVENT_FL_UPROBE was contained to within kernel/trace so far, maybe it's better to instead expose a helper function to check if perf_event represents uprobe? > + err = -EINVAL; > + bpf_link_cleanup(&link_primer); > + goto out_put_file; > + } > + } > + > err = perf_event_set_bpf_prog(event, prog, attr->link_create.perf_event.bpf_cookie); > if (err) { > bpf_link_cleanup(&link_primer); > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 71827d14724a..c6258118dd75 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -14313,8 +14313,8 @@ static int check_attach_btf_id(struct bpf_verifier_env *env) > } > > if (prog->aux->sleepable && prog->type != BPF_PROG_TYPE_TRACING && > - prog->type != BPF_PROG_TYPE_LSM) { > - verbose(env, "Only fentry/fexit/fmod_ret and lsm programs can be sleepable\n"); > + 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; > } > > -- > 2.35.1
On Thu, Apr 28, 2022 at 11:22 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Apr 28, 2022 at 9:54 AM Delyan Kratunov <delyank@fb.com> wrote: > > > > uprobe and kprobe programs have the same program type, KPROBE, which is > > currently not allowed to load sleepable programs. > > > > To avoid adding a new UPROBE type, we instead allow sleepable KPROBE > > programs to load and defer the is-it-actually-a-uprobe-program check > > to attachment time, where we're already validating the corresponding > > perf_event. > > > > A corollary of this patch is that you can now load a sleepable kprobe > > program but cannot attach it. > > > > Signed-off-by: Delyan Kratunov <delyank@fb.com> > > --- > > kernel/bpf/syscall.c | 8 ++++++++ > > kernel/bpf/verifier.c | 4 ++-- > > 2 files changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index e9e3e49c0eb7..3ce923f489d7 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -3009,6 +3009,14 @@ static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *pro > > } > > > > event = perf_file->private_data; > > + if (prog->aux->sleepable) { > > + if (!event->tp_event || (event->tp_event->flags & TRACE_EVENT_FL_UPROBE) == 0) { > > so far TRACE_EVENT_FL_UPROBE was contained to within kernel/trace so > far, maybe it's better to instead expose a helper function to check if > perf_event represents uprobe? or we can move prog->aux->sleepable check down into perf_event_set_bpf_prog(). Which is probably cleaner. We check other prog flags there.
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index e9e3e49c0eb7..3ce923f489d7 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3009,6 +3009,14 @@ static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *pro } event = perf_file->private_data; + if (prog->aux->sleepable) { + if (!event->tp_event || (event->tp_event->flags & TRACE_EVENT_FL_UPROBE) == 0) { + err = -EINVAL; + bpf_link_cleanup(&link_primer); + goto out_put_file; + } + } + err = perf_event_set_bpf_prog(event, prog, attr->link_create.perf_event.bpf_cookie); if (err) { bpf_link_cleanup(&link_primer); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 71827d14724a..c6258118dd75 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -14313,8 +14313,8 @@ static int check_attach_btf_id(struct bpf_verifier_env *env) } if (prog->aux->sleepable && prog->type != BPF_PROG_TYPE_TRACING && - prog->type != BPF_PROG_TYPE_LSM) { - verbose(env, "Only fentry/fexit/fmod_ret and lsm programs can be sleepable\n"); + 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; }
uprobe and kprobe programs have the same program type, KPROBE, which is currently not allowed to load sleepable programs. To avoid adding a new UPROBE type, we instead allow sleepable KPROBE programs to load and defer the is-it-actually-a-uprobe-program check to attachment time, where we're already validating the corresponding perf_event. A corollary of this patch is that you can now load a sleepable kprobe program but cannot attach it. Signed-off-by: Delyan Kratunov <delyank@fb.com> --- kernel/bpf/syscall.c | 8 ++++++++ kernel/bpf/verifier.c | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) -- 2.35.1