| Message ID | 20210203070636.70926-1-alexei.starovoitov@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 548f1191d86ccb9bde2a5305988877b7584c01eb |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf] bpf: Unbreak BPF_PROG_TYPE_KPROBE when kprobe is called via do_int3 | expand |
On 3.02.21 г. 9:06 ч., Alexei Starovoitov wrote: > From: Alexei Starovoitov <ast@kernel.org> > > The commit 0d00449c7a28 ("x86: Replace ist_enter() with nmi_enter()") > converted do_int3 handler to be "NMI-like". > That made old if (in_nmi()) check abort execution of bpf programs > attached to kprobe when kprobe is firing via int3 > (For example when kprobe is placed in the middle of the function). > Remove the check to restore user visible behavior. > > Fixes: 0d00449c7a28 ("x86: Replace ist_enter() with nmi_enter()") > Reported-by: Nikolay Borisov <nborisov@suse.com> > Signed-off-by: Alexei Starovoitov <ast@kernel.org> Tested-by: Nikolay Borisov <nborisov@suse.com> So I take it you have verified the callpaths and deemed that it's safe to remove this check? > --- > kernel/trace/bpf_trace.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 6c0018abe68a..764400260eb6 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -96,9 +96,6 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) > { > unsigned int ret; > > - if (in_nmi()) /* not supported yet */ > - return 1; > - > cant_sleep(); > > if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) { >
On Tue, Feb 2, 2021 at 11:09 PM Nikolay Borisov <nborisov@suse.com> wrote: > > > > On 3.02.21 г. 9:06 ч., Alexei Starovoitov wrote: > > From: Alexei Starovoitov <ast@kernel.org> > > > > The commit 0d00449c7a28 ("x86: Replace ist_enter() with nmi_enter()") > > converted do_int3 handler to be "NMI-like". > > That made old if (in_nmi()) check abort execution of bpf programs > > attached to kprobe when kprobe is firing via int3 > > (For example when kprobe is placed in the middle of the function). > > Remove the check to restore user visible behavior. > > > > Fixes: 0d00449c7a28 ("x86: Replace ist_enter() with nmi_enter()") > > Reported-by: Nikolay Borisov <nborisov@suse.com> > > Signed-off-by: Alexei Starovoitov <ast@kernel.org> > > Tested-by: Nikolay Borisov <nborisov@suse.com> > > > So I take it you have verified the callpaths and deemed that it's safe > to remove this check? I stared a lot into different places. It's not pretty. I will follow up with tightening patches for bpf-next, but I couldn't come up with anything better for bpf tree.
On Tue, 2 Feb 2021 23:06:36 -0800 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > From: Alexei Starovoitov <ast@kernel.org> > > The commit 0d00449c7a28 ("x86: Replace ist_enter() with nmi_enter()") > converted do_int3 handler to be "NMI-like". > That made old if (in_nmi()) check abort execution of bpf programs > attached to kprobe when kprobe is firing via int3 > (For example when kprobe is placed in the middle of the function). > Remove the check to restore user visible behavior. > > Fixes: 0d00449c7a28 ("x86: Replace ist_enter() with nmi_enter()") > Reported-by: Nikolay Borisov <nborisov@suse.com> > Signed-off-by: Alexei Starovoitov <ast@kernel.org> Looks good to me :) Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org> Thanks! > --- > kernel/trace/bpf_trace.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 6c0018abe68a..764400260eb6 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -96,9 +96,6 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) > { > unsigned int ret; > > - if (in_nmi()) /* not supported yet */ > - return 1; > - > cant_sleep(); > > if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) { > -- > 2.24.1 >
Hello: This patch was applied to bpf/bpf.git (refs/heads/master): On Tue, 2 Feb 2021 23:06:36 -0800 you wrote: > From: Alexei Starovoitov <ast@kernel.org> > > The commit 0d00449c7a28 ("x86: Replace ist_enter() with nmi_enter()") > converted do_int3 handler to be "NMI-like". > That made old if (in_nmi()) check abort execution of bpf programs > attached to kprobe when kprobe is firing via int3 > (For example when kprobe is placed in the middle of the function). > Remove the check to restore user visible behavior. > > [...] Here is the summary with links: - [bpf] bpf: Unbreak BPF_PROG_TYPE_KPROBE when kprobe is called via do_int3 https://git.kernel.org/bpf/bpf/c/548f1191d86c You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 6c0018abe68a..764400260eb6 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -96,9 +96,6 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx) { unsigned int ret; - if (in_nmi()) /* not supported yet */ - return 1; - cant_sleep(); if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {