[PATCH] arm64: Fix early single-stepping

[PATCH] arm64: Fix early single-stepping

[Jean-Philippe Brucker]

To use debug features such as single-step, the OS lock must be unlocked
in the debug registers. Currently this is done in postcore_initcall
which is now too late.

Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
using kprobes from early_initcall, when OS lock is still locked. So when
kprobe attempts to single-step a patched instruction, instead of
trapping, execution continues until it throws an undef exception:

[    0.064233] Kprobe smoke test: started
[    0.151133] ------------[ cut here ]------------
[    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
[    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
              ...
[    0.162689] Call trace:
[    0.163014]  do_undefinstr+0x1d4/0x1f4
[    0.163336]  el1_sync_handler+0xbc/0x140
[    0.163839]  el1_sync+0x80/0x100
[    0.164154]  0xffffffc01001d004
[    0.164527]  init_kprobes+0x13c/0x154
[    0.164968]  do_one_initcall+0x54/0x2e0
[    0.165322]  kernel_init_freeable+0xf4/0x258
[    0.165783]  kernel_init+0x20/0x12c
[    0.166117]  ret_from_fork+0x10/0x30
[    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
[    0.167084] ---[ end trace 36778fdf576e9a79 ]---

To fix this, unlock the OS lock as early as possible. Do it in
traps_init() for CPU0, since KGDB wants to use single-step from that
point on according to commit b322c65f8ca3 ("arm64: Call
debug_traps_init() from trap_init() to help early kgdb").
For secondary CPUs, setup the CPU hotplug handler at early_initcall.

Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 arch/arm64/kernel/debug-monitors.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 75a423c3336a..80f082021234 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -135,7 +135,7 @@ static int __init debug_monitors_init(void)
 				 "arm64/debug_monitors:starting",
 				 clear_os_lock, NULL);
 }
-postcore_initcall(debug_monitors_init);
+early_initcall(debug_monitors_init);
 
 /*
  * Single step API and exception handling.
@@ -380,6 +380,7 @@ NOKPROBE_SYMBOL(aarch32_break_handler);
 
 void __init debug_traps_init(void)
 {
+	clear_os_lock(0);
 	hook_debug_fault_code(DBG_ESR_EVT_HWSS, single_step_handler, SIGTRAP,
 			      TRAP_TRACE, "single-step handler");
 	hook_debug_fault_code(DBG_ESR_EVT_BRK, brk_handler, SIGTRAP,
-- 
2.29.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Will Deacon]

On Mon, Oct 26, 2020 at 06:29:09PM +0100, Jean-Philippe Brucker wrote:
> To use debug features such as single-step, the OS lock must be unlocked
> in the debug registers. Currently this is done in postcore_initcall
> which is now too late.
> 
> Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> using kprobes from early_initcall, when OS lock is still locked. So when
> kprobe attempts to single-step a patched instruction, instead of
> trapping, execution continues until it throws an undef exception:
> 
> [    0.064233] Kprobe smoke test: started
> [    0.151133] ------------[ cut here ]------------
> [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>               ...
> [    0.162689] Call trace:
> [    0.163014]  do_undefinstr+0x1d4/0x1f4
> [    0.163336]  el1_sync_handler+0xbc/0x140
> [    0.163839]  el1_sync+0x80/0x100
> [    0.164154]  0xffffffc01001d004
> [    0.164527]  init_kprobes+0x13c/0x154
> [    0.164968]  do_one_initcall+0x54/0x2e0
> [    0.165322]  kernel_init_freeable+0xf4/0x258
> [    0.165783]  kernel_init+0x20/0x12c
> [    0.166117]  ret_from_fork+0x10/0x30
> [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> 
> To fix this, unlock the OS lock as early as possible. Do it in
> traps_init() for CPU0, since KGDB wants to use single-step from that
> point on according to commit b322c65f8ca3 ("arm64: Call
> debug_traps_init() from trap_init() to help early kgdb").
> For secondary CPUs, setup the CPU hotplug handler at early_initcall.

Hmm, does this mean we end up setting MDSCR_EL1.KDE before we've reset the
hardware breakpoint/watchpoint registers? Why do we need kprobes so early?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Masami Hiramatsu]

On Mon, 26 Oct 2020 17:38:37 +0000
Will Deacon <will@kernel.org> wrote:

> On Mon, Oct 26, 2020 at 06:29:09PM +0100, Jean-Philippe Brucker wrote:
> > To use debug features such as single-step, the OS lock must be unlocked
> > in the debug registers. Currently this is done in postcore_initcall
> > which is now too late.
> > 
> > Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> > using kprobes from early_initcall, when OS lock is still locked. So when
> > kprobe attempts to single-step a patched instruction, instead of
> > trapping, execution continues until it throws an undef exception:
> > 
> > [    0.064233] Kprobe smoke test: started
> > [    0.151133] ------------[ cut here ]------------
> > [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> > [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> >               ...
> > [    0.162689] Call trace:
> > [    0.163014]  do_undefinstr+0x1d4/0x1f4
> > [    0.163336]  el1_sync_handler+0xbc/0x140
> > [    0.163839]  el1_sync+0x80/0x100
> > [    0.164154]  0xffffffc01001d004
> > [    0.164527]  init_kprobes+0x13c/0x154
> > [    0.164968]  do_one_initcall+0x54/0x2e0
> > [    0.165322]  kernel_init_freeable+0xf4/0x258
> > [    0.165783]  kernel_init+0x20/0x12c
> > [    0.166117]  ret_from_fork+0x10/0x30
> > [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> > [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> > 
> > To fix this, unlock the OS lock as early as possible. Do it in
> > traps_init() for CPU0, since KGDB wants to use single-step from that
> > point on according to commit b322c65f8ca3 ("arm64: Call
> > debug_traps_init() from trap_init() to help early kgdb").
> > For secondary CPUs, setup the CPU hotplug handler at early_initcall.

Oops, thanks for the fix! I missed it.

> Hmm, does this mean we end up setting MDSCR_EL1.KDE before we've reset the
> hardware breakpoint/watchpoint registers? Why do we need kprobes so early?

This is for boot-time tracing. To enable kprobes events in core_initcall(),
we need to enable kprobes itself in early_initcall().(or early_initcall_sync())
With this, we can trace postcore functions with kprobes(it includes some platform
initializations), which is making boot-time ftrace more useful.
For example, we can trace function-calls in specific code area as I posted
an example;

https://lore.kernel.org/linux-doc/159887792384.1330989.5993224243767476896.stgit@devnote2/

So this expands the feature to the earlier stages.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Masami Hiramatsu]

On Mon, 26 Oct 2020 18:29:09 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> To use debug features such as single-step, the OS lock must be unlocked
> in the debug registers. Currently this is done in postcore_initcall
> which is now too late.
> 
> Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> using kprobes from early_initcall, when OS lock is still locked. So when
> kprobe attempts to single-step a patched instruction, instead of
> trapping, execution continues until it throws an undef exception:
> 
> [    0.064233] Kprobe smoke test: started
> [    0.151133] ------------[ cut here ]------------
> [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>               ...
> [    0.162689] Call trace:
> [    0.163014]  do_undefinstr+0x1d4/0x1f4
> [    0.163336]  el1_sync_handler+0xbc/0x140
> [    0.163839]  el1_sync+0x80/0x100
> [    0.164154]  0xffffffc01001d004
> [    0.164527]  init_kprobes+0x13c/0x154
> [    0.164968]  do_one_initcall+0x54/0x2e0
> [    0.165322]  kernel_init_freeable+0xf4/0x258
> [    0.165783]  kernel_init+0x20/0x12c
> [    0.166117]  ret_from_fork+0x10/0x30
> [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> 
> To fix this, unlock the OS lock as early as possible. Do it in
> traps_init() for CPU0, since KGDB wants to use single-step from that
> point on according to commit b322c65f8ca3 ("arm64: Call
> debug_traps_init() from trap_init() to help early kgdb").
> For secondary CPUs, setup the CPU hotplug handler at early_initcall.
> 
> Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Hi Jean,

How have you confirmed this fixes the issue?
On my environment, this doesn't fix the issue.
We need to call debug_monitors_init() before calling init_kprobe(). 
But since both are early_initcall, it doesn't guarantee that the
debug_monitors_init() is called before init_kprobes().

I think there are 2 ways to fix this (in addition to this patch),
- split kprobe selftest from init_kprobe() and call the selftest
  in core_initcall()
- call clear_os_lock(0) from arch_init_kprobes()

Thank you,

> ---
>  arch/arm64/kernel/debug-monitors.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index 75a423c3336a..80f082021234 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -135,7 +135,7 @@ static int __init debug_monitors_init(void)
>  				 "arm64/debug_monitors:starting",
>  				 clear_os_lock, NULL);
>  }
> -postcore_initcall(debug_monitors_init);
> +early_initcall(debug_monitors_init);
>  
>  /*
>   * Single step API and exception handling.
> @@ -380,6 +380,7 @@ NOKPROBE_SYMBOL(aarch32_break_handler);
>  
>  void __init debug_traps_init(void)
>  {
> +	clear_os_lock(0);
>  	hook_debug_fault_code(DBG_ESR_EVT_HWSS, single_step_handler, SIGTRAP,
>  			      TRAP_TRACE, "single-step handler");
>  	hook_debug_fault_code(DBG_ESR_EVT_BRK, brk_handler, SIGTRAP,
> -- 
> 2.29.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Masami Hiramatsu]

On Tue, 27 Oct 2020 19:13:18 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Mon, 26 Oct 2020 18:29:09 +0100
> Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> 
> > To use debug features such as single-step, the OS lock must be unlocked
> > in the debug registers. Currently this is done in postcore_initcall
> > which is now too late.
> > 
> > Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> > using kprobes from early_initcall, when OS lock is still locked. So when
> > kprobe attempts to single-step a patched instruction, instead of
> > trapping, execution continues until it throws an undef exception:
> > 
> > [    0.064233] Kprobe smoke test: started
> > [    0.151133] ------------[ cut here ]------------
> > [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> > [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> >               ...
> > [    0.162689] Call trace:
> > [    0.163014]  do_undefinstr+0x1d4/0x1f4
> > [    0.163336]  el1_sync_handler+0xbc/0x140
> > [    0.163839]  el1_sync+0x80/0x100
> > [    0.164154]  0xffffffc01001d004
> > [    0.164527]  init_kprobes+0x13c/0x154
> > [    0.164968]  do_one_initcall+0x54/0x2e0
> > [    0.165322]  kernel_init_freeable+0xf4/0x258
> > [    0.165783]  kernel_init+0x20/0x12c
> > [    0.166117]  ret_from_fork+0x10/0x30
> > [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> > [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> > 
> > To fix this, unlock the OS lock as early as possible. Do it in
> > traps_init() for CPU0, since KGDB wants to use single-step from that
> > point on according to commit b322c65f8ca3 ("arm64: Call
> > debug_traps_init() from trap_init() to help early kgdb").
> > For secondary CPUs, setup the CPU hotplug handler at early_initcall.
> > 
> > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> 
> Hi Jean,
> 
> How have you confirmed this fixes the issue?
> On my environment, this doesn't fix the issue.

Oops, it was my mistake. I missed to boot up with Xen. (so I find another bug...)
Anyway this works for me too.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you!


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Jean-Philippe Brucker]

On Tue, Oct 27, 2020 at 07:42:58PM +0900, Masami Hiramatsu wrote:
> On Tue, 27 Oct 2020 19:13:18 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Mon, 26 Oct 2020 18:29:09 +0100
> > Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> > 
> > > To use debug features such as single-step, the OS lock must be unlocked
> > > in the debug registers. Currently this is done in postcore_initcall
> > > which is now too late.
> > > 
> > > Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> > > using kprobes from early_initcall, when OS lock is still locked. So when
> > > kprobe attempts to single-step a patched instruction, instead of
> > > trapping, execution continues until it throws an undef exception:
> > > 
> > > [    0.064233] Kprobe smoke test: started
> > > [    0.151133] ------------[ cut here ]------------
> > > [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> > > [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > >               ...
> > > [    0.162689] Call trace:
> > > [    0.163014]  do_undefinstr+0x1d4/0x1f4
> > > [    0.163336]  el1_sync_handler+0xbc/0x140
> > > [    0.163839]  el1_sync+0x80/0x100
> > > [    0.164154]  0xffffffc01001d004
> > > [    0.164527]  init_kprobes+0x13c/0x154
> > > [    0.164968]  do_one_initcall+0x54/0x2e0
> > > [    0.165322]  kernel_init_freeable+0xf4/0x258
> > > [    0.165783]  kernel_init+0x20/0x12c
> > > [    0.166117]  ret_from_fork+0x10/0x30
> > > [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> > > [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> > > 
> > > To fix this, unlock the OS lock as early as possible. Do it in
> > > traps_init() for CPU0, since KGDB wants to use single-step from that
> > > point on according to commit b322c65f8ca3 ("arm64: Call
> > > debug_traps_init() from trap_init() to help early kgdb").
> > > For secondary CPUs, setup the CPU hotplug handler at early_initcall.
> > > 
> > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > 
> > Hi Jean,
> > 
> > How have you confirmed this fixes the issue?
> > On my environment, this doesn't fix the issue.
> 
> Oops, it was my mistake. I missed to boot up with Xen. (so I find another bug...)
> Anyway this works for me too.

No worries :)  Although now I've been wondering whether it would be better
to just disable the OS lock lazily, on the first call to
enable_debug_monitors(). It might add a tiny performance penalty but would
avoid this problem reappearing if one of the debugger needs to start even
earlier in the future.

> Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
> Tested-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!
Jean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Will Deacon]

On Tue, Oct 27, 2020 at 12:59:09PM +0100, Jean-Philippe Brucker wrote:
> On Tue, Oct 27, 2020 at 07:42:58PM +0900, Masami Hiramatsu wrote:
> > On Tue, 27 Oct 2020 19:13:18 +0900
> > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > 
> > > On Mon, 26 Oct 2020 18:29:09 +0100
> > > Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> > > 
> > > > To use debug features such as single-step, the OS lock must be unlocked
> > > > in the debug registers. Currently this is done in postcore_initcall
> > > > which is now too late.
> > > > 
> > > > Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> > > > using kprobes from early_initcall, when OS lock is still locked. So when
> > > > kprobe attempts to single-step a patched instruction, instead of
> > > > trapping, execution continues until it throws an undef exception:
> > > > 
> > > > [    0.064233] Kprobe smoke test: started
> > > > [    0.151133] ------------[ cut here ]------------
> > > > [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> > > > [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > > >               ...
> > > > [    0.162689] Call trace:
> > > > [    0.163014]  do_undefinstr+0x1d4/0x1f4
> > > > [    0.163336]  el1_sync_handler+0xbc/0x140
> > > > [    0.163839]  el1_sync+0x80/0x100
> > > > [    0.164154]  0xffffffc01001d004
> > > > [    0.164527]  init_kprobes+0x13c/0x154
> > > > [    0.164968]  do_one_initcall+0x54/0x2e0
> > > > [    0.165322]  kernel_init_freeable+0xf4/0x258
> > > > [    0.165783]  kernel_init+0x20/0x12c
> > > > [    0.166117]  ret_from_fork+0x10/0x30
> > > > [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> > > > [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> > > > 
> > > > To fix this, unlock the OS lock as early as possible. Do it in
> > > > traps_init() for CPU0, since KGDB wants to use single-step from that
> > > > point on according to commit b322c65f8ca3 ("arm64: Call
> > > > debug_traps_init() from trap_init() to help early kgdb").
> > > > For secondary CPUs, setup the CPU hotplug handler at early_initcall.
> > > > 
> > > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > 
> > > Hi Jean,
> > > 
> > > How have you confirmed this fixes the issue?
> > > On my environment, this doesn't fix the issue.
> > 
> > Oops, it was my mistake. I missed to boot up with Xen. (so I find another bug...)
> > Anyway this works for me too.
> 
> No worries :)  Although now I've been wondering whether it would be better
> to just disable the OS lock lazily, on the first call to
> enable_debug_monitors(). It might add a tiny performance penalty but would
> avoid this problem reappearing if one of the debugger needs to start even
> earlier in the future.

I'm still uneasy about enabling KDE with the watchpoint registers in an
unknown state, so I think this needs more work.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Masami Hiramatsu]

On Tue, 27 Oct 2020 12:33:18 +0000
Will Deacon <will@kernel.org> wrote:

> On Tue, Oct 27, 2020 at 12:59:09PM +0100, Jean-Philippe Brucker wrote:
> > On Tue, Oct 27, 2020 at 07:42:58PM +0900, Masami Hiramatsu wrote:
> > > On Tue, 27 Oct 2020 19:13:18 +0900
> > > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > 
> > > > On Mon, 26 Oct 2020 18:29:09 +0100
> > > > Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> > > > 
> > > > > To use debug features such as single-step, the OS lock must be unlocked
> > > > > in the debug registers. Currently this is done in postcore_initcall
> > > > > which is now too late.
> > > > > 
> > > > > Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> > > > > using kprobes from early_initcall, when OS lock is still locked. So when
> > > > > kprobe attempts to single-step a patched instruction, instead of
> > > > > trapping, execution continues until it throws an undef exception:
> > > > > 
> > > > > [    0.064233] Kprobe smoke test: started
> > > > > [    0.151133] ------------[ cut here ]------------
> > > > > [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> > > > > [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > > > >               ...
> > > > > [    0.162689] Call trace:
> > > > > [    0.163014]  do_undefinstr+0x1d4/0x1f4
> > > > > [    0.163336]  el1_sync_handler+0xbc/0x140
> > > > > [    0.163839]  el1_sync+0x80/0x100
> > > > > [    0.164154]  0xffffffc01001d004
> > > > > [    0.164527]  init_kprobes+0x13c/0x154
> > > > > [    0.164968]  do_one_initcall+0x54/0x2e0
> > > > > [    0.165322]  kernel_init_freeable+0xf4/0x258
> > > > > [    0.165783]  kernel_init+0x20/0x12c
> > > > > [    0.166117]  ret_from_fork+0x10/0x30
> > > > > [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> > > > > [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> > > > > 
> > > > > To fix this, unlock the OS lock as early as possible. Do it in
> > > > > traps_init() for CPU0, since KGDB wants to use single-step from that
> > > > > point on according to commit b322c65f8ca3 ("arm64: Call
> > > > > debug_traps_init() from trap_init() to help early kgdb").
> > > > > For secondary CPUs, setup the CPU hotplug handler at early_initcall.
> > > > > 
> > > > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > > 
> > > > Hi Jean,
> > > > 
> > > > How have you confirmed this fixes the issue?
> > > > On my environment, this doesn't fix the issue.
> > > 
> > > Oops, it was my mistake. I missed to boot up with Xen. (so I find another bug...)
> > > Anyway this works for me too.
> > 
> > No worries :)  Although now I've been wondering whether it would be better
> > to just disable the OS lock lazily, on the first call to
> > enable_debug_monitors(). It might add a tiny performance penalty but would
> > avoid this problem reappearing if one of the debugger needs to start even
> > earlier in the future.
> 
> I'm still uneasy about enabling KDE with the watchpoint registers in an
> unknown state, so I think this needs more work.

Hmm, how we reset it in the early stage? reset watchpoint registers first?

Thanks,

> 
> Will


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Jean-Philippe Brucker]

On Tue, Oct 27, 2020 at 10:49:22PM +0900, Masami Hiramatsu wrote:
> On Tue, 27 Oct 2020 12:33:18 +0000
> Will Deacon <will@kernel.org> wrote:
> 
> > On Tue, Oct 27, 2020 at 12:59:09PM +0100, Jean-Philippe Brucker wrote:
> > > On Tue, Oct 27, 2020 at 07:42:58PM +0900, Masami Hiramatsu wrote:
> > > > On Tue, 27 Oct 2020 19:13:18 +0900
> > > > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > > 
> > > > > On Mon, 26 Oct 2020 18:29:09 +0100
> > > > > Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> > > > > 
> > > > > > To use debug features such as single-step, the OS lock must be unlocked
> > > > > > in the debug registers. Currently this is done in postcore_initcall
> > > > > > which is now too late.
> > > > > > 
> > > > > > Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> > > > > > using kprobes from early_initcall, when OS lock is still locked. So when
> > > > > > kprobe attempts to single-step a patched instruction, instead of
> > > > > > trapping, execution continues until it throws an undef exception:
> > > > > > 
> > > > > > [    0.064233] Kprobe smoke test: started
> > > > > > [    0.151133] ------------[ cut here ]------------
> > > > > > [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> > > > > > [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > > > > >               ...
> > > > > > [    0.162689] Call trace:
> > > > > > [    0.163014]  do_undefinstr+0x1d4/0x1f4
> > > > > > [    0.163336]  el1_sync_handler+0xbc/0x140
> > > > > > [    0.163839]  el1_sync+0x80/0x100
> > > > > > [    0.164154]  0xffffffc01001d004
> > > > > > [    0.164527]  init_kprobes+0x13c/0x154
> > > > > > [    0.164968]  do_one_initcall+0x54/0x2e0
> > > > > > [    0.165322]  kernel_init_freeable+0xf4/0x258
> > > > > > [    0.165783]  kernel_init+0x20/0x12c
> > > > > > [    0.166117]  ret_from_fork+0x10/0x30
> > > > > > [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> > > > > > [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> > > > > > 
> > > > > > To fix this, unlock the OS lock as early as possible. Do it in
> > > > > > traps_init() for CPU0, since KGDB wants to use single-step from that
> > > > > > point on according to commit b322c65f8ca3 ("arm64: Call
> > > > > > debug_traps_init() from trap_init() to help early kgdb").
> > > > > > For secondary CPUs, setup the CPU hotplug handler at early_initcall.
> > > > > > 
> > > > > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > > > 
> > > > > Hi Jean,
> > > > > 
> > > > > How have you confirmed this fixes the issue?
> > > > > On my environment, this doesn't fix the issue.
> > > > 
> > > > Oops, it was my mistake. I missed to boot up with Xen. (so I find another bug...)
> > > > Anyway this works for me too.
> > > 
> > > No worries :)  Although now I've been wondering whether it would be better
> > > to just disable the OS lock lazily, on the first call to
> > > enable_debug_monitors(). It might add a tiny performance penalty but would
> > > avoid this problem reappearing if one of the debugger needs to start even
> > > earlier in the future.
> > 
> > I'm still uneasy about enabling KDE with the watchpoint registers in an
> > unknown state, so I think this needs more work.
> 
> Hmm, how we reset it in the early stage? reset watchpoint registers first?

Yes, I think so. Same order problem as the OS lock, they need to be reset
before enable_debug_monitors(). On CPU0 that would be before
early_initcall and for secondaries the hotplug notifier needs to be
installed earlier as well. I'll send a v2.

Thanks,
Jean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Will Deacon]

On Wed, Oct 28, 2020 at 09:28:20AM +0100, Jean-Philippe Brucker wrote:
> On Tue, Oct 27, 2020 at 10:49:22PM +0900, Masami Hiramatsu wrote:
> > On Tue, 27 Oct 2020 12:33:18 +0000
> > Will Deacon <will@kernel.org> wrote:
> > 
> > > On Tue, Oct 27, 2020 at 12:59:09PM +0100, Jean-Philippe Brucker wrote:
> > > > On Tue, Oct 27, 2020 at 07:42:58PM +0900, Masami Hiramatsu wrote:
> > > > > On Tue, 27 Oct 2020 19:13:18 +0900
> > > > > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > > > 
> > > > > > On Mon, 26 Oct 2020 18:29:09 +0100
> > > > > > Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> > > > > > 
> > > > > > > To use debug features such as single-step, the OS lock must be unlocked
> > > > > > > in the debug registers. Currently this is done in postcore_initcall
> > > > > > > which is now too late.
> > > > > > > 
> > > > > > > Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> > > > > > > using kprobes from early_initcall, when OS lock is still locked. So when
> > > > > > > kprobe attempts to single-step a patched instruction, instead of
> > > > > > > trapping, execution continues until it throws an undef exception:
> > > > > > > 
> > > > > > > [    0.064233] Kprobe smoke test: started
> > > > > > > [    0.151133] ------------[ cut here ]------------
> > > > > > > [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> > > > > > > [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > > > > > >               ...
> > > > > > > [    0.162689] Call trace:
> > > > > > > [    0.163014]  do_undefinstr+0x1d4/0x1f4
> > > > > > > [    0.163336]  el1_sync_handler+0xbc/0x140
> > > > > > > [    0.163839]  el1_sync+0x80/0x100
> > > > > > > [    0.164154]  0xffffffc01001d004
> > > > > > > [    0.164527]  init_kprobes+0x13c/0x154
> > > > > > > [    0.164968]  do_one_initcall+0x54/0x2e0
> > > > > > > [    0.165322]  kernel_init_freeable+0xf4/0x258
> > > > > > > [    0.165783]  kernel_init+0x20/0x12c
> > > > > > > [    0.166117]  ret_from_fork+0x10/0x30
> > > > > > > [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> > > > > > > [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> > > > > > > 
> > > > > > > To fix this, unlock the OS lock as early as possible. Do it in
> > > > > > > traps_init() for CPU0, since KGDB wants to use single-step from that
> > > > > > > point on according to commit b322c65f8ca3 ("arm64: Call
> > > > > > > debug_traps_init() from trap_init() to help early kgdb").
> > > > > > > For secondary CPUs, setup the CPU hotplug handler at early_initcall.
> > > > > > > 
> > > > > > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > > > > 
> > > > > > Hi Jean,
> > > > > > 
> > > > > > How have you confirmed this fixes the issue?
> > > > > > On my environment, this doesn't fix the issue.
> > > > > 
> > > > > Oops, it was my mistake. I missed to boot up with Xen. (so I find another bug...)
> > > > > Anyway this works for me too.
> > > > 
> > > > No worries :)  Although now I've been wondering whether it would be better
> > > > to just disable the OS lock lazily, on the first call to
> > > > enable_debug_monitors(). It might add a tiny performance penalty but would
> > > > avoid this problem reappearing if one of the debugger needs to start even
> > > > earlier in the future.
> > > 
> > > I'm still uneasy about enabling KDE with the watchpoint registers in an
> > > unknown state, so I think this needs more work.
> > 
> > Hmm, how we reset it in the early stage? reset watchpoint registers first?
> 
> Yes, I think so. Same order problem as the OS lock, they need to be reset
> before enable_debug_monitors(). On CPU0 that would be before
> early_initcall and for secondaries the hotplug notifier needs to be
> installed earlier as well. I'll send a v2.

Cheers. An alternative (which I think would be better in the long run
anyway) would be to avoid using hardware step in kprobes and instead rely
on a BRK instruction to trap after running the trampoline.

Then we just need to ensure that the BRK handlers are up and running in
time  (which might require an early handler, like we have for KASAN).

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Masami Hiramatsu]

On Wed, 28 Oct 2020 08:36:44 +0000
Will Deacon <will@kernel.org> wrote:

> On Wed, Oct 28, 2020 at 09:28:20AM +0100, Jean-Philippe Brucker wrote:
> > On Tue, Oct 27, 2020 at 10:49:22PM +0900, Masami Hiramatsu wrote:
> > > On Tue, 27 Oct 2020 12:33:18 +0000
> > > Will Deacon <will@kernel.org> wrote:
> > > 
> > > > On Tue, Oct 27, 2020 at 12:59:09PM +0100, Jean-Philippe Brucker wrote:
> > > > > On Tue, Oct 27, 2020 at 07:42:58PM +0900, Masami Hiramatsu wrote:
> > > > > > On Tue, 27 Oct 2020 19:13:18 +0900
> > > > > > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > > > > 
> > > > > > > On Mon, 26 Oct 2020 18:29:09 +0100
> > > > > > > Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> > > > > > > 
> > > > > > > > To use debug features such as single-step, the OS lock must be unlocked
> > > > > > > > in the debug registers. Currently this is done in postcore_initcall
> > > > > > > > which is now too late.
> > > > > > > > 
> > > > > > > > Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> > > > > > > > using kprobes from early_initcall, when OS lock is still locked. So when
> > > > > > > > kprobe attempts to single-step a patched instruction, instead of
> > > > > > > > trapping, execution continues until it throws an undef exception:
> > > > > > > > 
> > > > > > > > [    0.064233] Kprobe smoke test: started
> > > > > > > > [    0.151133] ------------[ cut here ]------------
> > > > > > > > [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> > > > > > > > [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > > > > > > >               ...
> > > > > > > > [    0.162689] Call trace:
> > > > > > > > [    0.163014]  do_undefinstr+0x1d4/0x1f4
> > > > > > > > [    0.163336]  el1_sync_handler+0xbc/0x140
> > > > > > > > [    0.163839]  el1_sync+0x80/0x100
> > > > > > > > [    0.164154]  0xffffffc01001d004
> > > > > > > > [    0.164527]  init_kprobes+0x13c/0x154
> > > > > > > > [    0.164968]  do_one_initcall+0x54/0x2e0
> > > > > > > > [    0.165322]  kernel_init_freeable+0xf4/0x258
> > > > > > > > [    0.165783]  kernel_init+0x20/0x12c
> > > > > > > > [    0.166117]  ret_from_fork+0x10/0x30
> > > > > > > > [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> > > > > > > > [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> > > > > > > > 
> > > > > > > > To fix this, unlock the OS lock as early as possible. Do it in
> > > > > > > > traps_init() for CPU0, since KGDB wants to use single-step from that
> > > > > > > > point on according to commit b322c65f8ca3 ("arm64: Call
> > > > > > > > debug_traps_init() from trap_init() to help early kgdb").
> > > > > > > > For secondary CPUs, setup the CPU hotplug handler at early_initcall.
> > > > > > > > 
> > > > > > > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > > > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > > > > > 
> > > > > > > Hi Jean,
> > > > > > > 
> > > > > > > How have you confirmed this fixes the issue?
> > > > > > > On my environment, this doesn't fix the issue.
> > > > > > 
> > > > > > Oops, it was my mistake. I missed to boot up with Xen. (so I find another bug...)
> > > > > > Anyway this works for me too.
> > > > > 
> > > > > No worries :)  Although now I've been wondering whether it would be better
> > > > > to just disable the OS lock lazily, on the first call to
> > > > > enable_debug_monitors(). It might add a tiny performance penalty but would
> > > > > avoid this problem reappearing if one of the debugger needs to start even
> > > > > earlier in the future.
> > > > 
> > > > I'm still uneasy about enabling KDE with the watchpoint registers in an
> > > > unknown state, so I think this needs more work.
> > > 
> > > Hmm, how we reset it in the early stage? reset watchpoint registers first?
> > 
> > Yes, I think so. Same order problem as the OS lock, they need to be reset
> > before enable_debug_monitors(). On CPU0 that would be before
> > early_initcall and for secondaries the hotplug notifier needs to be
> > installed earlier as well. I'll send a v2.
> 
> Cheers. An alternative (which I think would be better in the long run
> anyway) would be to avoid using hardware step in kprobes and instead rely
> on a BRK instruction to trap after running the trampoline.

But how we trap the instruction which can change pc? (like br?)
Are all those instruction emulated now?

Thank you,


> Then we just need to ensure that the BRK handlers are up and running in
> time  (which might require an early handler, like we have for KASAN).
> 
> Will


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Jean-Philippe Brucker]

On Wed, Oct 28, 2020 at 06:07:31PM +0900, Masami Hiramatsu wrote:
> > > Yes, I think so. Same order problem as the OS lock, they need to be reset
> > > before enable_debug_monitors(). On CPU0 that would be before
> > > early_initcall and for secondaries the hotplug notifier needs to be
> > > installed earlier as well. I'll send a v2.
> > 
> > Cheers. An alternative (which I think would be better in the long run
> > anyway) would be to avoid using hardware step in kprobes and instead rely
> > on a BRK instruction to trap after running the trampoline.
> 
> But how we trap the instruction which can change pc? (like br?)
> Are all those instruction emulated now?

According to aarch64_insn_is_steppable() anything that changes the PC is
emulated. I'm also checking whether there is a change of behavior with
synchronous exceptions taken while single-stepping (page faults).

Thanks,
Jean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Masami Hiramatsu]

On Wed, 28 Oct 2020 10:48:27 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> On Wed, Oct 28, 2020 at 06:07:31PM +0900, Masami Hiramatsu wrote:
> > > > Yes, I think so. Same order problem as the OS lock, they need to be reset
> > > > before enable_debug_monitors(). On CPU0 that would be before
> > > > early_initcall and for secondaries the hotplug notifier needs to be
> > > > installed earlier as well. I'll send a v2.
> > > 
> > > Cheers. An alternative (which I think would be better in the long run
> > > anyway) would be to avoid using hardware step in kprobes and instead rely
> > > on a BRK instruction to trap after running the trampoline.
> > 
> > But how we trap the instruction which can change pc? (like br?)
> > Are all those instruction emulated now?
> 
> According to aarch64_insn_is_steppable() anything that changes the PC is
> emulated.

OK, that sounds good. Then we can put the BRK right after the copied instruction.

> I'm also checking whether there is a change of behavior with
> synchronous exceptions taken while single-stepping (page faults).

Thanks! From the kprobe_fault_handler(), it seems the page faults handled
before the single-stepping exception and the fault handler disables single
steping explicitly. So if we use BRK, that code will not be needed.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Masami Hiramatsu]

Hi Will,

On Wed, 28 Oct 2020 08:36:44 +0000
Will Deacon <will@kernel.org> wrote:

> On Wed, Oct 28, 2020 at 09:28:20AM +0100, Jean-Philippe Brucker wrote:
> > On Tue, Oct 27, 2020 at 10:49:22PM +0900, Masami Hiramatsu wrote:
> > > On Tue, 27 Oct 2020 12:33:18 +0000
> > > Will Deacon <will@kernel.org> wrote:
> > > 
> > > > On Tue, Oct 27, 2020 at 12:59:09PM +0100, Jean-Philippe Brucker wrote:
> > > > > On Tue, Oct 27, 2020 at 07:42:58PM +0900, Masami Hiramatsu wrote:
> > > > > > On Tue, 27 Oct 2020 19:13:18 +0900
> > > > > > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > > > > 
> > > > > > > On Mon, 26 Oct 2020 18:29:09 +0100
> > > > > > > Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> > > > > > > 
> > > > > > > > To use debug features such as single-step, the OS lock must be unlocked
> > > > > > > > in the debug registers. Currently this is done in postcore_initcall
> > > > > > > > which is now too late.
> > > > > > > > 
> > > > > > > > Commit 36dadef23fcc ("kprobes: Init kprobes in early_initcall") enabled
> > > > > > > > using kprobes from early_initcall, when OS lock is still locked. So when
> > > > > > > > kprobe attempts to single-step a patched instruction, instead of
> > > > > > > > trapping, execution continues until it throws an undef exception:
> > > > > > > > 
> > > > > > > > [    0.064233] Kprobe smoke test: started
> > > > > > > > [    0.151133] ------------[ cut here ]------------
> > > > > > > > [    0.151458] kernel BUG at arch/arm64/kernel/traps.c:406!
> > > > > > > > [    0.151812] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > > > > > > >               ...
> > > > > > > > [    0.162689] Call trace:
> > > > > > > > [    0.163014]  do_undefinstr+0x1d4/0x1f4
> > > > > > > > [    0.163336]  el1_sync_handler+0xbc/0x140
> > > > > > > > [    0.163839]  el1_sync+0x80/0x100
> > > > > > > > [    0.164154]  0xffffffc01001d004
> > > > > > > > [    0.164527]  init_kprobes+0x13c/0x154
> > > > > > > > [    0.164968]  do_one_initcall+0x54/0x2e0
> > > > > > > > [    0.165322]  kernel_init_freeable+0xf4/0x258
> > > > > > > > [    0.165783]  kernel_init+0x20/0x12c
> > > > > > > > [    0.166117]  ret_from_fork+0x10/0x30
> > > > > > > > [    0.166595] Code: 97ffff53 a9425bf5 17ffff9b f9001bf7 (d4210000)
> > > > > > > > [    0.167084] ---[ end trace 36778fdf576e9a79 ]---
> > > > > > > > 
> > > > > > > > To fix this, unlock the OS lock as early as possible. Do it in
> > > > > > > > traps_init() for CPU0, since KGDB wants to use single-step from that
> > > > > > > > point on according to commit b322c65f8ca3 ("arm64: Call
> > > > > > > > debug_traps_init() from trap_init() to help early kgdb").
> > > > > > > > For secondary CPUs, setup the CPU hotplug handler at early_initcall.
> > > > > > > > 
> > > > > > > > Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
> > > > > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > > > > > 
> > > > > > > Hi Jean,
> > > > > > > 
> > > > > > > How have you confirmed this fixes the issue?
> > > > > > > On my environment, this doesn't fix the issue.
> > > > > > 
> > > > > > Oops, it was my mistake. I missed to boot up with Xen. (so I find another bug...)
> > > > > > Anyway this works for me too.
> > > > > 
> > > > > No worries :)  Although now I've been wondering whether it would be better
> > > > > to just disable the OS lock lazily, on the first call to
> > > > > enable_debug_monitors(). It might add a tiny performance penalty but would
> > > > > avoid this problem reappearing if one of the debugger needs to start even
> > > > > earlier in the future.
> > > > 
> > > > I'm still uneasy about enabling KDE with the watchpoint registers in an
> > > > unknown state, so I think this needs more work.
> > > 
> > > Hmm, how we reset it in the early stage? reset watchpoint registers first?
> > 
> > Yes, I think so. Same order problem as the OS lock, they need to be reset
> > before enable_debug_monitors(). On CPU0 that would be before
> > early_initcall and for secondaries the hotplug notifier needs to be
> > installed earlier as well. I'll send a v2.
> 
> Cheers. An alternative (which I think would be better in the long run
> anyway) would be to avoid using hardware step in kprobes and instead rely
> on a BRK instruction to trap after running the trampoline.

We started working on using the BRK instead of hardware step in kprobes
in other threads. However, there still be a bug in the kernel.
I would like to fix or at least mitigate this issue until this is released
(since it's a bug)

Would you think we can push the BRK only kprobes until it or in stable kernel?
Or, we should add a mitigation patch for this bug?
For the mitigation, I think we can introduce a kconfig flag which indicates
the arch doesn't support early kprobes, in that case we defer the kprobe and
boot-time trace later stage. This flag will be removed after we introduce the
BRK-only kprobes.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Will Deacon]

Hi Masami,

On Thu, Nov 26, 2020 at 01:09:06AM +0900, Masami Hiramatsu wrote:
> On Wed, 28 Oct 2020 08:36:44 +0000
> Will Deacon <will@kernel.org> wrote:
> > Cheers. An alternative (which I think would be better in the long run
> > anyway) would be to avoid using hardware step in kprobes and instead rely
> > on a BRK instruction to trap after running the trampoline.
> 
> We started working on using the BRK instead of hardware step in kprobes
> in other threads. However, there still be a bug in the kernel.
> I would like to fix or at least mitigate this issue until this is released
> (since it's a bug)
> 
> Would you think we can push the BRK only kprobes until it or in stable kernel?
> Or, we should add a mitigation patch for this bug?
> For the mitigation, I think we can introduce a kconfig flag which indicates
> the arch doesn't support early kprobes, in that case we defer the kprobe and
> boot-time trace later stage. This flag will be removed after we introduce the
> BRK-only kprobes.

The BRK stuff is merged upstream:

http://git.kernel.org/linus/7ee31a3aa8f49

Are you saying that this isn't sufficient to fix the problem?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Fix early single-stepping

[Masami Hiramatsu]

On Wed, 25 Nov 2020 16:11:34 +0000
Will Deacon <will@kernel.org> wrote:

> Hi Masami,
> 
> On Thu, Nov 26, 2020 at 01:09:06AM +0900, Masami Hiramatsu wrote:
> > On Wed, 28 Oct 2020 08:36:44 +0000
> > Will Deacon <will@kernel.org> wrote:
> > > Cheers. An alternative (which I think would be better in the long run
> > > anyway) would be to avoid using hardware step in kprobes and instead rely
> > > on a BRK instruction to trap after running the trampoline.
> > 
> > We started working on using the BRK instead of hardware step in kprobes
> > in other threads. However, there still be a bug in the kernel.
> > I would like to fix or at least mitigate this issue until this is released
> > (since it's a bug)
> > 
> > Would you think we can push the BRK only kprobes until it or in stable kernel?
> > Or, we should add a mitigation patch for this bug?
> > For the mitigation, I think we can introduce a kconfig flag which indicates
> > the arch doesn't support early kprobes, in that case we defer the kprobe and
> > boot-time trace later stage. This flag will be removed after we introduce the
> > BRK-only kprobes.
> 
> The BRK stuff is merged upstream:
> 
> http://git.kernel.org/linus/7ee31a3aa8f49
> 
> Are you saying that this isn't sufficient to fix the problem?

Oops, No, it should be enough.
I thought we were still in discussion on the other thread...

Anyway, thank you for merging! 

-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel