* [PATCH bpf-next] bpf: trace jit code when enable BPF_JIT_ALWAYS_ON
@ 2021-03-26 12:40 Jianlin Lv
2021-03-26 14:24 ` Alexei Starovoitov
0 siblings, 1 reply; 5+ messages in thread
From: Jianlin Lv @ 2021-03-26 12:40 UTC (permalink / raw)
To: bpf
Cc: davem, kuba, ast, daniel, andrii, kafai, songliubraving, yhs,
john.fastabend, kpsingh, viro, rdna, dvyukov, nicolas.dichtel,
keescook, masahiroy, maheshb, linux-kernel, netdev, Jianlin.Lv,
iecedge, nd
When CONFIG_BPF_JIT_ALWAYS_ON is enabled, the value of bpf_jit_enable in
/proc/sys is limited to SYSCTL_ONE. This is not convenient for debugging.
This patch modifies the value of extra2 (max) to 2 that support developers
to emit traces on kernel log.
Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
---
net/core/sysctl_net_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index d84c8a1b280e..aa16883ac445 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -386,7 +386,7 @@ static struct ctl_table net_core_table[] = {
.proc_handler = proc_dointvec_minmax_bpf_enable,
# ifdef CONFIG_BPF_JIT_ALWAYS_ON
.extra1 = SYSCTL_ONE,
- .extra2 = SYSCTL_ONE,
+ .extra2 = &two,
# else
.extra1 = SYSCTL_ZERO,
.extra2 = &two,
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: trace jit code when enable BPF_JIT_ALWAYS_ON
2021-03-26 12:40 [PATCH bpf-next] bpf: trace jit code when enable BPF_JIT_ALWAYS_ON Jianlin Lv
@ 2021-03-26 14:24 ` Alexei Starovoitov
2021-03-27 8:19 ` Jianlin Lv
0 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2021-03-26 14:24 UTC (permalink / raw)
To: Jianlin Lv
Cc: bpf, David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Alexander Viro,
Andrey Ignatov, Dmitry Vyukov, Nicolas Dichtel, Kees Cook,
Masahiro Yamada, Mahesh Bandewar, LKML, Network Development,
iecedge, nd
On Fri, Mar 26, 2021 at 5:40 AM Jianlin Lv <Jianlin.Lv@arm.com> wrote:
>
> When CONFIG_BPF_JIT_ALWAYS_ON is enabled, the value of bpf_jit_enable in
> /proc/sys is limited to SYSCTL_ONE. This is not convenient for debugging.
> This patch modifies the value of extra2 (max) to 2 that support developers
> to emit traces on kernel log.
>
> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> ---
> net/core/sysctl_net_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index d84c8a1b280e..aa16883ac445 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -386,7 +386,7 @@ static struct ctl_table net_core_table[] = {
> .proc_handler = proc_dointvec_minmax_bpf_enable,
> # ifdef CONFIG_BPF_JIT_ALWAYS_ON
> .extra1 = SYSCTL_ONE,
> - .extra2 = SYSCTL_ONE,
> + .extra2 = &two,
"bpftool prog dump jited" is much better way to examine JITed dumps.
I'd rather remove bpf_jit_enable=2 altogether.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH bpf-next] bpf: trace jit code when enable BPF_JIT_ALWAYS_ON
2021-03-26 14:24 ` Alexei Starovoitov
@ 2021-03-27 8:19 ` Jianlin Lv
2021-03-27 15:19 ` Alexei Starovoitov
0 siblings, 1 reply; 5+ messages in thread
From: Jianlin Lv @ 2021-03-27 8:19 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Alexander Viro,
Andrey Ignatov, Dmitry Vyukov, Nicolas Dichtel, Kees Cook,
Masahiro Yamada, Mahesh Bandewar, LKML, Network Development,
iecedge, nd
> -----Original Message-----
> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Sent: Friday, March 26, 2021 10:25 PM
> To: Jianlin Lv <Jianlin.Lv@arm.com>
> Cc: bpf <bpf@vger.kernel.org>; David S. Miller <davem@davemloft.net>;
> Jakub Kicinski <kuba@kernel.org>; Alexei Starovoitov <ast@kernel.org>;
> Daniel Borkmann <daniel@iogearbox.net>; Andrii Nakryiko
> <andrii@kernel.org>; Martin KaFai Lau <kafai@fb.com>; Song Liu
> <songliubraving@fb.com>; Yonghong Song <yhs@fb.com>; John Fastabend
> <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>; Alexander
> Viro <viro@zeniv.linux.org.uk>; Andrey Ignatov <rdna@fb.com>; Dmitry
> Vyukov <dvyukov@google.com>; Nicolas Dichtel
> <nicolas.dichtel@6wind.com>; Kees Cook <keescook@chromium.org>;
> Masahiro Yamada <masahiroy@kernel.org>; Mahesh Bandewar
> <maheshb@google.com>; LKML <linux-kernel@vger.kernel.org>; Network
> Development <netdev@vger.kernel.org>; iecedge@gmail.com; nd
> <nd@arm.com>
> Subject: Re: [PATCH bpf-next] bpf: trace jit code when enable
> BPF_JIT_ALWAYS_ON
>
> On Fri, Mar 26, 2021 at 5:40 AM Jianlin Lv <Jianlin.Lv@arm.com> wrote:
> >
> > When CONFIG_BPF_JIT_ALWAYS_ON is enabled, the value of
> bpf_jit_enable
> > in /proc/sys is limited to SYSCTL_ONE. This is not convenient for debugging.
> > This patch modifies the value of extra2 (max) to 2 that support
> > developers to emit traces on kernel log.
> >
> > Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> > ---
> > net/core/sysctl_net_core.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> > index d84c8a1b280e..aa16883ac445 100644
> > --- a/net/core/sysctl_net_core.c
> > +++ b/net/core/sysctl_net_core.c
> > @@ -386,7 +386,7 @@ static struct ctl_table net_core_table[] = {
> > .proc_handler = proc_dointvec_minmax_bpf_enable,
> > # ifdef CONFIG_BPF_JIT_ALWAYS_ON
> > .extra1 = SYSCTL_ONE,
> > - .extra2 = SYSCTL_ONE,
> > + .extra2 = &two,
>
> "bpftool prog dump jited" is much better way to examine JITed dumps.
> I'd rather remove bpf_jit_enable=2 altogether.
In my case, I introduced a bug when I made some adjustments to the arm64
jit macro A64_MOV(), which caused the SP register to be replaced by the
XZR register when building prologue, and the wrong value was stored in fp,
which triggered a crash.
Test case:
modprobe test_bpf test_name="SPILL_FILL"
jited code:
0: stp x29, x30, [sp, #-16]!
fd 7b bf a9
4: mov x29, xzr//Err, should be 'mov x29, sp'
fd 03 1f aa
8: stp x19, x20, [sp, #-16]!
f3 53 bf a9
c: stp x21, x22, [sp, #-16]!
f5 5b bf a9
10: stp x25, x26, [sp, #-16]!
f9 6b bf a9
14: mov x25, xzr//Err, should be 'mov x25, sp'
f9 03 1f aa
...
3c: mov x10, #0xfffffffffffffff8 // #-8
ea 00 80 92
40: str w7, [x25, x10]// Crash
27 6b 2a b8
This bug is likely to cause the instruction to access the BPF stack in
jited prog to trigger a crash.
I tried to use bpftool to debug, but bpftool crashed when I executed the
"bpftool prog show" command.
The syslog shown that bpftool is loading and running some bpf prog.
because of the bug in the JIT compiler, the bpftool execution failed.
bpf_jit_disasm saved me, it helped me dump the jited image:
echo 2> /proc/sys/net/core/bpf_jit_enable
modprobe test_bpf test_name="SPILL_FILL"
./bpf_jit_disasm -o
So keeping bpf_jit_enable=2 is still very meaningful for developers who
try to modify the JIT compiler.
Jianlin
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: trace jit code when enable BPF_JIT_ALWAYS_ON
2021-03-27 8:19 ` Jianlin Lv
@ 2021-03-27 15:19 ` Alexei Starovoitov
2021-03-28 1:59 ` Jianlin Lv
0 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2021-03-27 15:19 UTC (permalink / raw)
To: Jianlin Lv
Cc: bpf, David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Alexander Viro,
Andrey Ignatov, Dmitry Vyukov, Nicolas Dichtel, Kees Cook,
Masahiro Yamada, Mahesh Bandewar, LKML, Network Development,
iecedge, nd
On Sat, Mar 27, 2021 at 1:19 AM Jianlin Lv <Jianlin.Lv@arm.com> wrote:
>
> > On Fri, Mar 26, 2021 at 5:40 AM Jianlin Lv <Jianlin.Lv@arm.com> wrote:
> > >
> > > When CONFIG_BPF_JIT_ALWAYS_ON is enabled, the value of
> > bpf_jit_enable
> > > in /proc/sys is limited to SYSCTL_ONE. This is not convenient for debugging.
> > > This patch modifies the value of extra2 (max) to 2 that support
> > > developers to emit traces on kernel log.
> > >
> > > Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> > > ---
> > > net/core/sysctl_net_core.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> > > index d84c8a1b280e..aa16883ac445 100644
> > > --- a/net/core/sysctl_net_core.c
> > > +++ b/net/core/sysctl_net_core.c
> > > @@ -386,7 +386,7 @@ static struct ctl_table net_core_table[] = {
> > > .proc_handler = proc_dointvec_minmax_bpf_enable,
> > > # ifdef CONFIG_BPF_JIT_ALWAYS_ON
> > > .extra1 = SYSCTL_ONE,
> > > - .extra2 = SYSCTL_ONE,
> > > + .extra2 = &two,
> >
> > "bpftool prog dump jited" is much better way to examine JITed dumps.
> > I'd rather remove bpf_jit_enable=2 altogether.
>
> In my case, I introduced a bug when I made some adjustments to the arm64
> jit macro A64_MOV(), which caused the SP register to be replaced by the
> XZR register when building prologue, and the wrong value was stored in fp,
> which triggered a crash.
>
> This bug is likely to cause the instruction to access the BPF stack in
> jited prog to trigger a crash.
> I tried to use bpftool to debug, but bpftool crashed when I executed the
> "bpftool prog show" command.
> The syslog shown that bpftool is loading and running some bpf prog.
> because of the bug in the JIT compiler, the bpftool execution failed.
Right 'bpftool prog show' command is loading a bpf iterator prog,
but you didn't need to use it to dump JITed code.
"bpftool prog dump jited name my_prog"
would have dumped it even when JIT is all buggy.
> bpf_jit_disasm saved me, it helped me dump the jited image:
>
> echo 2> /proc/sys/net/core/bpf_jit_enable
> modprobe test_bpf test_name="SPILL_FILL"
> ./bpf_jit_disasm -o
>
> So keeping bpf_jit_enable=2 is still very meaningful for developers who
> try to modify the JIT compiler.
sure and such JIT developers can compile the kernel
without BPF_JIT_ALWAYS_ON just like you did.
They can also insert printk, etc.
bpf_jit_enable=2 was done long ago when there was no other way
to see JITed code. Now we have proper apis.
That =2 mode can and should be removed.
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
please fix your email server/client/whatever. No patches will ever be
accepted with
such disclaimer.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: trace jit code when enable BPF_JIT_ALWAYS_ON
2021-03-27 15:19 ` Alexei Starovoitov
@ 2021-03-28 1:59 ` Jianlin Lv
0 siblings, 0 replies; 5+ messages in thread
From: Jianlin Lv @ 2021-03-28 1:59 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jianlin Lv, bpf, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Alexander Viro, Andrey Ignatov, Dmitry Vyukov,
Nicolas Dichtel, Kees Cook, Masahiro Yamada, Mahesh Bandewar,
LKML, Network Development, nd
On Sat, Mar 27, 2021 at 11:19 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Mar 27, 2021 at 1:19 AM Jianlin Lv <Jianlin.Lv@arm.com> wrote:
> >
> > > On Fri, Mar 26, 2021 at 5:40 AM Jianlin Lv <Jianlin.Lv@arm.com> wrote:
> > > >
> > > > When CONFIG_BPF_JIT_ALWAYS_ON is enabled, the value of
> > > bpf_jit_enable
> > > > in /proc/sys is limited to SYSCTL_ONE. This is not convenient for debugging.
> > > > This patch modifies the value of extra2 (max) to 2 that support
> > > > developers to emit traces on kernel log.
> > > >
> > > > Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> > > > ---
> > > > net/core/sysctl_net_core.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> > > > index d84c8a1b280e..aa16883ac445 100644
> > > > --- a/net/core/sysctl_net_core.c
> > > > +++ b/net/core/sysctl_net_core.c
> > > > @@ -386,7 +386,7 @@ static struct ctl_table net_core_table[] = {
> > > > .proc_handler = proc_dointvec_minmax_bpf_enable,
> > > > # ifdef CONFIG_BPF_JIT_ALWAYS_ON
> > > > .extra1 = SYSCTL_ONE,
> > > > - .extra2 = SYSCTL_ONE,
> > > > + .extra2 = &two,
> > >
> > > "bpftool prog dump jited" is much better way to examine JITed dumps.
> > > I'd rather remove bpf_jit_enable=2 altogether.
> >
> > In my case, I introduced a bug when I made some adjustments to the arm64
> > jit macro A64_MOV(), which caused the SP register to be replaced by the
> > XZR register when building prologue, and the wrong value was stored in fp,
> > which triggered a crash.
> >
> > This bug is likely to cause the instruction to access the BPF stack in
> > jited prog to trigger a crash.
> > I tried to use bpftool to debug, but bpftool crashed when I executed the
> > "bpftool prog show" command.
> > The syslog shown that bpftool is loading and running some bpf prog.
> > because of the bug in the JIT compiler, the bpftool execution failed.
>
> Right 'bpftool prog show' command is loading a bpf iterator prog,
> but you didn't need to use it to dump JITed code.
> "bpftool prog dump jited name my_prog"
> would have dumped it even when JIT is all buggy.
>
> > bpf_jit_disasm saved me, it helped me dump the jited image:
> >
> > echo 2> /proc/sys/net/core/bpf_jit_enable
> > modprobe test_bpf test_name="SPILL_FILL"
> > ./bpf_jit_disasm -o
> >
> > So keeping bpf_jit_enable=2 is still very meaningful for developers who
> > try to modify the JIT compiler.
>
> sure and such JIT developers can compile the kernel
> without BPF_JIT_ALWAYS_ON just like you did.
> They can also insert printk, etc.
> bpf_jit_enable=2 was done long ago when there was no other way
> to see JITed code. Now we have proper apis.
> That =2 mode can and should be removed.
Thanks for your reply, I will prepare another patch to remove =2mode.
>
> > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
> please fix your email server/client/whatever. No patches will ever be
> accepted with
> such disclaimer.
Apologize for this.
Jianlin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-28 2:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26 12:40 [PATCH bpf-next] bpf: trace jit code when enable BPF_JIT_ALWAYS_ON Jianlin Lv
2021-03-26 14:24 ` Alexei Starovoitov
2021-03-27 8:19 ` Jianlin Lv
2021-03-27 15:19 ` Alexei Starovoitov
2021-03-28 1:59 ` Jianlin Lv
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).