Re: [PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode

2020-03-26 Thread Michael Ellerman
On Tue, 2020-02-18 at 19:38:27 UTC, Christophe Leroy wrote:
> When a program check exception happens while MMU translation is
> disabled, following Oops happens in kprobe_handler() in the following
> code:
> 
>   } else if (*addr != BREAKPOINT_INSTRUCTION) {
> 
> [   33.098554] BUG: Unable to handle kernel data access on read at 0xe268
> [   33.105091] Faulting instruction address: 0xc000ec34
> [   33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
> [   33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
> [   33.119540] Modules linked in:
> [   33.122591] CPU: 0 PID: 429 Comm: cat Not tainted 
> 5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
> [   33.131005] NIP:  c000ec34 LR: c000ecd8 CTR: c019cab8
> [   33.136002] REGS: ca4d3b58 TRAP: 0300   Not tainted  
> (5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
> [   33.144324] MSR:  1032   CR: 2a4d3c52  XER: 
> [   33.150699] DAR: e268 DSISR: c000
> [   33.150699] GPR00: c000b09c ca4d3c10 c66d0620  ca4d3c60  
> 9032 
> [   33.150699] GPR08: 0002  c087de44 c000afe0 c66d0ad0 100d3dd6 
> fff3 
> [   33.150699] GPR16:  0041  ca4d3d70   
> 416d 
> [   33.150699] GPR24: 0004 c53b6128  e268  c07c 
> c07bb6fc ca4d3c60
> [   33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
> [   33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
> [   33.197854] Call Trace:
> [   33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
> [   33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
> [   33.212392] --- interrupt: 700 at 0xe268
> [   33.270401] Instruction dump:
> [   33.273335] 913e0008 8122 3861 3929 9122 80010024 bb410008 
> 7c0803a6
> [   33.280992] 38210020 4e800020 3860 4e800020 <813b> 6d2a7fe0 
> 2f8a0008 419e0154
> [   33.288841] ---[ end trace 5b9152d4cdadd06d ]---
> 
> kprobe is not prepared to handle events in real mode and functions
> running in real mode should have been blacklisted, so kprobe_handler()
> can safely bail out telling 'this trap is not mine' for any trap that
> happened while in real-mode.
> 
> If the trap happened with MSR_IR or MSR_DR cleared, return 0 immediately.
> 
> Reported-by: Larry Finger 
> Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
> Cc: sta...@vger.kernel.org
> Cc: Naveen N. Rao 
> Cc: Masami Hiramatsu 
> Signed-off-by: Christophe Leroy 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/21f8b2fa3ca5b01f7a2b51b89ce97a3705a15aa0

cheers


Re: [PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode

2020-03-23 Thread Michael Ellerman
Christophe Leroy  writes:
> ping
>
>
> Le 18/02/2020 à 20:38, Christophe Leroy a écrit :
>> When a program check exception happens while MMU translation is
>> disabled, following Oops happens in kprobe_handler() in the following
>> code:
>
> Michael, we have several traps in assembly while MMU is still disabled 
> (TRACE_IRQFLAGS, KUAP DEBUG, syscall from kernel, machine check in RTAS, 
> ...).
> Without this fix, all of them trigger an Oops when CONFIG_KPROBE is set.

Only on 32-bit.

But I guess this fix is good, if someone really wants to handle kprobes
in real mode they can tell us and do the work to make it solid.

cheers

>>  } else if (*addr != BREAKPOINT_INSTRUCTION) {
>> 
>> [   33.098554] BUG: Unable to handle kernel data access on read at 0xe268
>> [   33.105091] Faulting instruction address: 0xc000ec34
>> [   33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
>> [   33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
>> [   33.119540] Modules linked in:
>> [   33.122591] CPU: 0 PID: 429 Comm: cat Not tainted 
>> 5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
>> [   33.131005] NIP:  c000ec34 LR: c000ecd8 CTR: c019cab8
>> [   33.136002] REGS: ca4d3b58 TRAP: 0300   Not tainted  
>> (5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
>> [   33.144324] MSR:  1032   CR: 2a4d3c52  XER: 
>> [   33.150699] DAR: e268 DSISR: c000
>> [   33.150699] GPR00: c000b09c ca4d3c10 c66d0620  ca4d3c60  
>> 9032 
>> [   33.150699] GPR08: 0002  c087de44 c000afe0 c66d0ad0 100d3dd6 
>> fff3 
>> [   33.150699] GPR16:  0041  ca4d3d70   
>> 416d 
>> [   33.150699] GPR24: 0004 c53b6128  e268  c07c 
>> c07bb6fc ca4d3c60
>> [   33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
>> [   33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
>> [   33.197854] Call Trace:
>> [   33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
>> [   33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
>> [   33.212392] --- interrupt: 700 at 0xe268
>> [   33.270401] Instruction dump:
>> [   33.273335] 913e0008 8122 3861 3929 9122 80010024 
>> bb410008 7c0803a6
>> [   33.280992] 38210020 4e800020 3860 4e800020 <813b> 6d2a7fe0 
>> 2f8a0008 419e0154
>> [   33.288841] ---[ end trace 5b9152d4cdadd06d ]---
>> 
>> kprobe is not prepared to handle events in real mode and functions
>> running in real mode should have been blacklisted, so kprobe_handler()
>> can safely bail out telling 'this trap is not mine' for any trap that
>> happened while in real-mode.
>> 
>> If the trap happened with MSR_IR or MSR_DR cleared, return 0 immediately.
>> 
>> Reported-by: Larry Finger 
>> Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
>> Cc: sta...@vger.kernel.org
>> Cc: Naveen N. Rao 
>> Cc: Masami Hiramatsu 
>> Signed-off-by: Christophe Leroy 
>> 
>> ---
>> v3: Also bail out if MSR_DR is cleared.
>> 
>> Resending v2 with a more appropriate name
>> 
>> v2: bailing out instead of converting real-time address to virtual and 
>> continuing.
>> 
>> The bug might have existed even before that commit from Naveen.
>> 
>> Signed-off-by: Christophe Leroy 
>> ---
>>   arch/powerpc/kernel/kprobes.c | 3 +++
>>   1 file changed, 3 insertions(+)
>> 
>> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
>> index 2d27ec4feee4..9b340af02c38 100644
>> --- a/arch/powerpc/kernel/kprobes.c
>> +++ b/arch/powerpc/kernel/kprobes.c
>> @@ -264,6 +264,9 @@ int kprobe_handler(struct pt_regs *regs)
>>  if (user_mode(regs))
>>  return 0;
>>   
>> +if (!(regs->msr & MSR_IR) || !(regs->msr & MSR_DR))
>> +return 0;
>> +
>>  /*
>>   * We don't want to be preempted for the entire
>>   * duration of kprobe processing
>> 


Re: [PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode

2020-03-22 Thread Naveen N. Rao

Christophe Leroy wrote:

ping


Le 18/02/2020 à 20:38, Christophe Leroy a écrit :

When a program check exception happens while MMU translation is
disabled, following Oops happens in kprobe_handler() in the following
code:


Michael, we have several traps in assembly while MMU is still disabled 
(TRACE_IRQFLAGS, KUAP DEBUG, syscall from kernel, machine check in RTAS, 
...).

Without this fix, all of them trigger an Oops when CONFIG_KPROBE is set.


For this patch:
Reviewed-by: Naveen N. Rao 


- Naveen



Re: [PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode

2020-03-22 Thread Christophe Leroy

ping


Le 18/02/2020 à 20:38, Christophe Leroy a écrit :

When a program check exception happens while MMU translation is
disabled, following Oops happens in kprobe_handler() in the following
code:


Michael, we have several traps in assembly while MMU is still disabled 
(TRACE_IRQFLAGS, KUAP DEBUG, syscall from kernel, machine check in RTAS, 
...).

Without this fix, all of them trigger an Oops when CONFIG_KPROBE is set.

Christophe



} else if (*addr != BREAKPOINT_INSTRUCTION) {

[   33.098554] BUG: Unable to handle kernel data access on read at 0xe268
[   33.105091] Faulting instruction address: 0xc000ec34
[   33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
[   33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
[   33.119540] Modules linked in:
[   33.122591] CPU: 0 PID: 429 Comm: cat Not tainted 
5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
[   33.131005] NIP:  c000ec34 LR: c000ecd8 CTR: c019cab8
[   33.136002] REGS: ca4d3b58 TRAP: 0300   Not tainted  
(5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
[   33.144324] MSR:  1032   CR: 2a4d3c52  XER: 
[   33.150699] DAR: e268 DSISR: c000
[   33.150699] GPR00: c000b09c ca4d3c10 c66d0620  ca4d3c60  
9032 
[   33.150699] GPR08: 0002  c087de44 c000afe0 c66d0ad0 100d3dd6 
fff3 
[   33.150699] GPR16:  0041  ca4d3d70   
416d 
[   33.150699] GPR24: 0004 c53b6128  e268  c07c 
c07bb6fc ca4d3c60
[   33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
[   33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
[   33.197854] Call Trace:
[   33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
[   33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
[   33.212392] --- interrupt: 700 at 0xe268
[   33.270401] Instruction dump:
[   33.273335] 913e0008 8122 3861 3929 9122 80010024 bb410008 
7c0803a6
[   33.280992] 38210020 4e800020 3860 4e800020 <813b> 6d2a7fe0 2f8a0008 
419e0154
[   33.288841] ---[ end trace 5b9152d4cdadd06d ]---

kprobe is not prepared to handle events in real mode and functions
running in real mode should have been blacklisted, so kprobe_handler()
can safely bail out telling 'this trap is not mine' for any trap that
happened while in real-mode.

If the trap happened with MSR_IR or MSR_DR cleared, return 0 immediately.

Reported-by: Larry Finger 
Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
Cc: sta...@vger.kernel.org
Cc: Naveen N. Rao 
Cc: Masami Hiramatsu 
Signed-off-by: Christophe Leroy 

---
v3: Also bail out if MSR_DR is cleared.

Resending v2 with a more appropriate name

v2: bailing out instead of converting real-time address to virtual and 
continuing.

The bug might have existed even before that commit from Naveen.

Signed-off-by: Christophe Leroy 
---
  arch/powerpc/kernel/kprobes.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 2d27ec4feee4..9b340af02c38 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -264,6 +264,9 @@ int kprobe_handler(struct pt_regs *regs)
if (user_mode(regs))
return 0;
  
+	if (!(regs->msr & MSR_IR) || !(regs->msr & MSR_DR))

+   return 0;
+
/*
 * We don't want to be preempted for the entire
 * duration of kprobe processing



Re: [PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode

2020-02-18 Thread Masami Hiramatsu
On Tue, 18 Feb 2020 19:38:27 + (UTC)
Christophe Leroy  wrote:

> When a program check exception happens while MMU translation is
> disabled, following Oops happens in kprobe_handler() in the following
> code:
> 
>   } else if (*addr != BREAKPOINT_INSTRUCTION) {
> 
> [   33.098554] BUG: Unable to handle kernel data access on read at 0xe268
> [   33.105091] Faulting instruction address: 0xc000ec34
> [   33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
> [   33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
> [   33.119540] Modules linked in:
> [   33.122591] CPU: 0 PID: 429 Comm: cat Not tainted 
> 5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
> [   33.131005] NIP:  c000ec34 LR: c000ecd8 CTR: c019cab8
> [   33.136002] REGS: ca4d3b58 TRAP: 0300   Not tainted  
> (5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
> [   33.144324] MSR:  1032   CR: 2a4d3c52  XER: 
> [   33.150699] DAR: e268 DSISR: c000
> [   33.150699] GPR00: c000b09c ca4d3c10 c66d0620  ca4d3c60  
> 9032 
> [   33.150699] GPR08: 0002  c087de44 c000afe0 c66d0ad0 100d3dd6 
> fff3 
> [   33.150699] GPR16:  0041  ca4d3d70   
> 416d 
> [   33.150699] GPR24: 0004 c53b6128  e268  c07c 
> c07bb6fc ca4d3c60
> [   33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
> [   33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
> [   33.197854] Call Trace:
> [   33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
> [   33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
> [   33.212392] --- interrupt: 700 at 0xe268
> [   33.270401] Instruction dump:
> [   33.273335] 913e0008 8122 3861 3929 9122 80010024 bb410008 
> 7c0803a6
> [   33.280992] 38210020 4e800020 3860 4e800020 <813b> 6d2a7fe0 
> 2f8a0008 419e0154
> [   33.288841] ---[ end trace 5b9152d4cdadd06d ]---
> 
> kprobe is not prepared to handle events in real mode and functions
> running in real mode should have been blacklisted, so kprobe_handler()
> can safely bail out telling 'this trap is not mine' for any trap that
> happened while in real-mode.
> 
> If the trap happened with MSR_IR or MSR_DR cleared, return 0 immediately.
> 

Looks good to me.

Reviewed-by: Masami Hiramatsu 

Thank you!


> Reported-by: Larry Finger 
> Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
> Cc: sta...@vger.kernel.org
> Cc: Naveen N. Rao 
> Cc: Masami Hiramatsu 
> Signed-off-by: Christophe Leroy 
> 
> ---
> v3: Also bail out if MSR_DR is cleared.
> 
> Resending v2 with a more appropriate name
> 
> v2: bailing out instead of converting real-time address to virtual and 
> continuing.
> 
> The bug might have existed even before that commit from Naveen.
> 
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/kernel/kprobes.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 2d27ec4feee4..9b340af02c38 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -264,6 +264,9 @@ int kprobe_handler(struct pt_regs *regs)
>   if (user_mode(regs))
>   return 0;
>  
> + if (!(regs->msr & MSR_IR) || !(regs->msr & MSR_DR))
> + return 0;
> +
>   /*
>* We don't want to be preempted for the entire
>* duration of kprobe processing
> -- 
> 2.25.0
> 


-- 
Masami Hiramatsu 


[PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode

2020-02-18 Thread Christophe Leroy
When a program check exception happens while MMU translation is
disabled, following Oops happens in kprobe_handler() in the following
code:

} else if (*addr != BREAKPOINT_INSTRUCTION) {

[   33.098554] BUG: Unable to handle kernel data access on read at 0xe268
[   33.105091] Faulting instruction address: 0xc000ec34
[   33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
[   33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
[   33.119540] Modules linked in:
[   33.122591] CPU: 0 PID: 429 Comm: cat Not tainted 
5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
[   33.131005] NIP:  c000ec34 LR: c000ecd8 CTR: c019cab8
[   33.136002] REGS: ca4d3b58 TRAP: 0300   Not tainted  
(5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
[   33.144324] MSR:  1032   CR: 2a4d3c52  XER: 
[   33.150699] DAR: e268 DSISR: c000
[   33.150699] GPR00: c000b09c ca4d3c10 c66d0620  ca4d3c60  
9032 
[   33.150699] GPR08: 0002  c087de44 c000afe0 c66d0ad0 100d3dd6 
fff3 
[   33.150699] GPR16:  0041  ca4d3d70   
416d 
[   33.150699] GPR24: 0004 c53b6128  e268  c07c 
c07bb6fc ca4d3c60
[   33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
[   33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
[   33.197854] Call Trace:
[   33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
[   33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
[   33.212392] --- interrupt: 700 at 0xe268
[   33.270401] Instruction dump:
[   33.273335] 913e0008 8122 3861 3929 9122 80010024 bb410008 
7c0803a6
[   33.280992] 38210020 4e800020 3860 4e800020 <813b> 6d2a7fe0 2f8a0008 
419e0154
[   33.288841] ---[ end trace 5b9152d4cdadd06d ]---

kprobe is not prepared to handle events in real mode and functions
running in real mode should have been blacklisted, so kprobe_handler()
can safely bail out telling 'this trap is not mine' for any trap that
happened while in real-mode.

If the trap happened with MSR_IR or MSR_DR cleared, return 0 immediately.

Reported-by: Larry Finger 
Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
Cc: sta...@vger.kernel.org
Cc: Naveen N. Rao 
Cc: Masami Hiramatsu 
Signed-off-by: Christophe Leroy 

---
v3: Also bail out if MSR_DR is cleared.

Resending v2 with a more appropriate name

v2: bailing out instead of converting real-time address to virtual and 
continuing.

The bug might have existed even before that commit from Naveen.

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/kernel/kprobes.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 2d27ec4feee4..9b340af02c38 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -264,6 +264,9 @@ int kprobe_handler(struct pt_regs *regs)
if (user_mode(regs))
return 0;
 
+   if (!(regs->msr & MSR_IR) || !(regs->msr & MSR_DR))
+   return 0;
+
/*
 * We don't want to be preempted for the entire
 * duration of kprobe processing
-- 
2.25.0