2018-05-04 04:16:30

by Masami Hiramatsu

[permalink] [raw]
Subject: [BUGFIX PATCH v3 0/4] arm: kprobes: Fix to prohibit probing on unsafe functions

Hi,

This is the 3rd version of bugfix series for kprobes on arm.
This series fixes 4 different issues which I found.

- Fix to use smp_processor_id() after disabling preemption.
- Prohibit probing on optimized_callback() for avoiding
recursive probe.
- Prohibit kprobes on do_undefinstr() by same reason.
- Prohibit kprobes on get_user() by same reason.

>From v2, I included another 2 bugfixes (1/4 and 2/4)
which are not merged yet, and added "Cc: [email protected]",
since there are obvious bugs.

Thanks,

---

Masami Hiramatsu (4):
arm: kprobes: Fix to use get_kprobe_ctlblk after irq-disabed
arm: kprobes: Prohibit probing on optimized_callback
arm: kprobes: Prohibit kprobes on do_undefinstr
arm: kprobes: Prohibit kprobes on get_user functions


arch/arm/include/asm/assembler.h | 10 ++++++++++
arch/arm/kernel/traps.c | 5 ++++-
arch/arm/lib/getuser.S | 10 ++++++++++
arch/arm/probes/kprobes/opt-arm.c | 4 +++-
4 files changed, 27 insertions(+), 2 deletions(-)

--
Masami Hiramatsu (Linaro) <[email protected]>


2018-05-04 04:16:00

by Masami Hiramatsu

[permalink] [raw]
Subject: [BUGFIX PATCH v3 1/4] arm: kprobes: Fix to use get_kprobe_ctlblk after irq-disabed

Since get_kprobe_ctlblk() uses smp_processor_id() to access
per-cpu variable, it hits smp_processor_id sanity check as below.

[ 7.006928] BUG: using smp_processor_id() in preemptible [00000000] code: swapper/0/1
[ 7.007859] caller is debug_smp_processor_id+0x20/0x24
[ 7.008438] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0-rc1-00192-g4eb17253e4b5 #1
[ 7.008890] Hardware name: Generic DT based system
[ 7.009917] [<c0313f0c>] (unwind_backtrace) from [<c030e6d8>] (show_stack+0x20/0x24)
[ 7.010473] [<c030e6d8>] (show_stack) from [<c0c64694>] (dump_stack+0x84/0x98)
[ 7.010990] [<c0c64694>] (dump_stack) from [<c071ca5c>] (check_preemption_disabled+0x138/0x13c)
[ 7.011592] [<c071ca5c>] (check_preemption_disabled) from [<c071ca80>] (debug_smp_processor_id+0x20/0x24)
[ 7.012214] [<c071ca80>] (debug_smp_processor_id) from [<c03335e0>] (optimized_callback+0x2c/0xe4)
[ 7.013077] [<c03335e0>] (optimized_callback) from [<bf0021b0>] (0xbf0021b0)

To fix this issue, call get_kprobe_ctlblk() right after
irq-disabled since that disables preemption.

Fixes: 0dc016dbd820 ("ARM: kprobes: enable OPTPROBES for ARM 32")
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: [email protected]
---
arch/arm/probes/kprobes/opt-arm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/probes/kprobes/opt-arm.c b/arch/arm/probes/kprobes/opt-arm.c
index bcdecc25461b..ddc5a82eb10d 100644
--- a/arch/arm/probes/kprobes/opt-arm.c
+++ b/arch/arm/probes/kprobes/opt-arm.c
@@ -165,13 +165,14 @@ optimized_callback(struct optimized_kprobe *op, struct pt_regs *regs)
{
unsigned long flags;
struct kprobe *p = &op->kp;
- struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ struct kprobe_ctlblk *kcb;

/* Save skipped registers */
regs->ARM_pc = (unsigned long)op->kp.addr;
regs->ARM_ORIG_r0 = ~0UL;

local_irq_save(flags);
+ kcb = get_kprobe_ctlblk();

if (kprobe_running()) {
kprobes_inc_nmissed_count(&op->kp);


2018-05-04 04:16:32

by Masami Hiramatsu

[permalink] [raw]
Subject: [BUGFIX PATCH v3 2/4] arm: kprobes: Prohibit probing on optimized_callback

Prohibit probing on optimized_callback() because
it is called from kprobes itself. If we put a kprobes
on it, that will cause a recursive call loop.
Mark it NOKPROBE_SYMBOL.

Fixes: 0dc016dbd820 ("ARM: kprobes: enable OPTPROBES for ARM 32")
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: [email protected]
---
arch/arm/probes/kprobes/opt-arm.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/probes/kprobes/opt-arm.c b/arch/arm/probes/kprobes/opt-arm.c
index ddc5a82eb10d..b2aa9b32bff2 100644
--- a/arch/arm/probes/kprobes/opt-arm.c
+++ b/arch/arm/probes/kprobes/opt-arm.c
@@ -192,6 +192,7 @@ optimized_callback(struct optimized_kprobe *op, struct pt_regs *regs)

local_irq_restore(flags);
}
+NOKPROBE_SYMBOL(optimized_callback)

int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *orig)
{


2018-05-04 04:17:31

by Masami Hiramatsu

[permalink] [raw]
Subject: [BUGFIX PATCH v3 4/4] arm: kprobes: Prohibit kprobes on get_user functions

Since do_undefinstr() uses get_user to get the undefined
instruction, it can be called before kprobes processes
recursive check. This can cause an infinit recursive
exception.
Prohibit probing on get_user functions.

Fixes: 24ba613c9d6c ("ARM kprobes: core code")
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: [email protected]
---
Changes in v2:
- Fix to add _ASM_NOKPROBE() definition for
!CONFIG_KPROBES.
---
arch/arm/include/asm/assembler.h | 10 ++++++++++
arch/arm/lib/getuser.S | 10 ++++++++++
2 files changed, 20 insertions(+)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index bc8d4bbd82e2..9342904cccca 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -536,4 +536,14 @@ THUMB( orr \reg , \reg , #PSR_T_BIT )
#endif
.endm

+#ifdef CONFIG_KPROBES
+#define _ASM_NOKPROBE(entry) \
+ .pushsection "_kprobe_blacklist", "aw" ; \
+ .balign 4 ; \
+ .long entry; \
+ .popsection
+#else
+#define _ASM_NOKPROBE(entry)
+#endif
+
#endif /* __ASM_ASSEMBLER_H__ */
diff --git a/arch/arm/lib/getuser.S b/arch/arm/lib/getuser.S
index df73914e81c8..746e7801dcdf 100644
--- a/arch/arm/lib/getuser.S
+++ b/arch/arm/lib/getuser.S
@@ -38,6 +38,7 @@ ENTRY(__get_user_1)
mov r0, #0
ret lr
ENDPROC(__get_user_1)
+_ASM_NOKPROBE(__get_user_1)

ENTRY(__get_user_2)
check_uaccess r0, 2, r1, r2, __get_user_bad
@@ -58,6 +59,7 @@ rb .req r0
mov r0, #0
ret lr
ENDPROC(__get_user_2)
+_ASM_NOKPROBE(__get_user_2)

ENTRY(__get_user_4)
check_uaccess r0, 4, r1, r2, __get_user_bad
@@ -65,6 +67,7 @@ ENTRY(__get_user_4)
mov r0, #0
ret lr
ENDPROC(__get_user_4)
+_ASM_NOKPROBE(__get_user_4)

ENTRY(__get_user_8)
check_uaccess r0, 8, r1, r2, __get_user_bad8
@@ -78,6 +81,7 @@ ENTRY(__get_user_8)
mov r0, #0
ret lr
ENDPROC(__get_user_8)
+_ASM_NOKPROBE(__get_user_8)

#ifdef __ARMEB__
ENTRY(__get_user_32t_8)
@@ -91,6 +95,7 @@ ENTRY(__get_user_32t_8)
mov r0, #0
ret lr
ENDPROC(__get_user_32t_8)
+_ASM_NOKPROBE(__get_user_32t_8)

ENTRY(__get_user_64t_1)
check_uaccess r0, 1, r1, r2, __get_user_bad8
@@ -98,6 +103,7 @@ ENTRY(__get_user_64t_1)
mov r0, #0
ret lr
ENDPROC(__get_user_64t_1)
+_ASM_NOKPROBE(__get_user_64t_1)

ENTRY(__get_user_64t_2)
check_uaccess r0, 2, r1, r2, __get_user_bad8
@@ -114,6 +120,7 @@ rb .req r0
mov r0, #0
ret lr
ENDPROC(__get_user_64t_2)
+_ASM_NOKPROBE(__get_user_64t_2)

ENTRY(__get_user_64t_4)
check_uaccess r0, 4, r1, r2, __get_user_bad8
@@ -121,6 +128,7 @@ ENTRY(__get_user_64t_4)
mov r0, #0
ret lr
ENDPROC(__get_user_64t_4)
+_ASM_NOKPROBE(__get_user_64t_4)
#endif

__get_user_bad8:
@@ -131,6 +139,8 @@ __get_user_bad:
ret lr
ENDPROC(__get_user_bad)
ENDPROC(__get_user_bad8)
+_ASM_NOKPROBE(__get_user_bad)
+_ASM_NOKPROBE(__get_user_bad8)

.pushsection __ex_table, "a"
.long 1b, __get_user_bad


2018-05-04 04:18:04

by Masami Hiramatsu

[permalink] [raw]
Subject: [BUGFIX PATCH v3 3/4] arm: kprobes: Prohibit kprobes on do_undefinstr

Prohibit kprobes on do_undefinstr because kprobes on
arm is implemented by undefined instruction. This means
if we probe do_undefinstr(), it can cause infinit
recursive exception.

Fixes: 24ba613c9d6c ("ARM kprobes: core code")
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: [email protected]
---
arch/arm/kernel/traps.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 5e3633c24e63..2fe87109ae46 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -19,6 +19,7 @@
#include <linux/uaccess.h>
#include <linux/hardirq.h>
#include <linux/kdebug.h>
+#include <linux/kprobes.h>
#include <linux/module.h>
#include <linux/kexec.h>
#include <linux/bug.h>
@@ -417,7 +418,8 @@ void unregister_undef_hook(struct undef_hook *hook)
raw_spin_unlock_irqrestore(&undef_lock, flags);
}

-static int call_undef_hook(struct pt_regs *regs, unsigned int instr)
+static nokprobe_inline
+int call_undef_hook(struct pt_regs *regs, unsigned int instr)
{
struct undef_hook *hook;
unsigned long flags;
@@ -490,6 +492,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs)

arm_notify_die("Oops - undefined instruction", regs, &info, 0, 6);
}
+NOKPROBE_SYMBOL(do_undefinstr)

/*
* Handle FIQ similarly to NMI on x86 systems.


2018-05-08 11:26:26

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [BUGFIX PATCH v3 0/4] arm: kprobes: Fix to prohibit probing on unsafe functions

On Fri, May 04, 2018 at 01:14:31PM +0900, Masami Hiramatsu wrote:
> Hi,
>
> This is the 3rd version of bugfix series for kprobes on arm.
> This series fixes 4 different issues which I found.
>
> - Fix to use smp_processor_id() after disabling preemption.
> - Prohibit probing on optimized_callback() for avoiding
> recursive probe.
> - Prohibit kprobes on do_undefinstr() by same reason.
> - Prohibit kprobes on get_user() by same reason.
>
> >From v2, I included another 2 bugfixes (1/4 and 2/4)
> which are not merged yet, and added "Cc: [email protected]",
> since there are obvious bugs.

Please submit them to the patch system, thanks.

>
> Thanks,
>
> ---
>
> Masami Hiramatsu (4):
> arm: kprobes: Fix to use get_kprobe_ctlblk after irq-disabed
> arm: kprobes: Prohibit probing on optimized_callback
> arm: kprobes: Prohibit kprobes on do_undefinstr
> arm: kprobes: Prohibit kprobes on get_user functions
>
>
> arch/arm/include/asm/assembler.h | 10 ++++++++++
> arch/arm/kernel/traps.c | 5 ++++-
> arch/arm/lib/getuser.S | 10 ++++++++++
> arch/arm/probes/kprobes/opt-arm.c | 4 +++-
> 4 files changed, 27 insertions(+), 2 deletions(-)
>
> --
> Masami Hiramatsu (Linaro) <[email protected]>

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

2018-05-12 00:42:52

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [BUGFIX PATCH v3 0/4] arm: kprobes: Fix to prohibit probing on unsafe functions

Hi Greg,

Could you pick this series to stable?

Thank you,

On Tue, 8 May 2018 12:25:03 +0100
Russell King - ARM Linux <[email protected]> wrote:

> On Fri, May 04, 2018 at 01:14:31PM +0900, Masami Hiramatsu wrote:
> > Hi,
> >
> > This is the 3rd version of bugfix series for kprobes on arm.
> > This series fixes 4 different issues which I found.
> >
> > - Fix to use smp_processor_id() after disabling preemption.
> > - Prohibit probing on optimized_callback() for avoiding
> > recursive probe.
> > - Prohibit kprobes on do_undefinstr() by same reason.
> > - Prohibit kprobes on get_user() by same reason.
> >
> > >From v2, I included another 2 bugfixes (1/4 and 2/4)
> > which are not merged yet, and added "Cc: [email protected]",
> > since there are obvious bugs.
>
> Please submit them to the patch system, thanks.
>
> >
> > Thanks,
> >
> > ---
> >
> > Masami Hiramatsu (4):
> > arm: kprobes: Fix to use get_kprobe_ctlblk after irq-disabed
> > arm: kprobes: Prohibit probing on optimized_callback
> > arm: kprobes: Prohibit kprobes on do_undefinstr
> > arm: kprobes: Prohibit kprobes on get_user functions
> >
> >
> > arch/arm/include/asm/assembler.h | 10 ++++++++++
> > arch/arm/kernel/traps.c | 5 ++++-
> > arch/arm/lib/getuser.S | 10 ++++++++++
> > arch/arm/probes/kprobes/opt-arm.c | 4 +++-
> > 4 files changed, 27 insertions(+), 2 deletions(-)
> >
> > --
> > Masami Hiramatsu (Linaro) <[email protected]>
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up


--
Masami Hiramatsu <[email protected]>

2018-05-12 04:32:30

by Greg KH

[permalink] [raw]
Subject: Re: [BUGFIX PATCH v3 0/4] arm: kprobes: Fix to prohibit probing on unsafe functions

On Sat, May 12, 2018 at 09:42:21AM +0900, Masami Hiramatsu wrote:
> Hi Greg,
>
> Could you pick this series to stable?

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

thanks,

greg k-h

2018-05-12 09:07:55

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [BUGFIX PATCH v3 0/4] arm: kprobes: Fix to prohibit probing on unsafe functions

On Sat, May 12, 2018 at 09:42:21AM +0900, Masami Hiramatsu wrote:
> Hi Greg,
>
> Could you pick this series to stable?

You've added the Cc for stable, so when they get committed to mainline,
they will be automatically picked up without further need to ask.

However, they do _first_ need to end up in mainline before they can go
anywhere near stable trees as per stable tree rules. For more
information on stable trees and their rules, please see:

Documentation/process/stable-kernel-rules.rst

>
> Thank you,
>
> On Tue, 8 May 2018 12:25:03 +0100
> Russell King - ARM Linux <[email protected]> wrote:
>
> > On Fri, May 04, 2018 at 01:14:31PM +0900, Masami Hiramatsu wrote:
> > > Hi,
> > >
> > > This is the 3rd version of bugfix series for kprobes on arm.
> > > This series fixes 4 different issues which I found.
> > >
> > > - Fix to use smp_processor_id() after disabling preemption.
> > > - Prohibit probing on optimized_callback() for avoiding
> > > recursive probe.
> > > - Prohibit kprobes on do_undefinstr() by same reason.
> > > - Prohibit kprobes on get_user() by same reason.
> > >
> > > >From v2, I included another 2 bugfixes (1/4 and 2/4)
> > > which are not merged yet, and added "Cc: [email protected]",
> > > since there are obvious bugs.
> >
> > Please submit them to the patch system, thanks.
> >
> > >
> > > Thanks,
> > >
> > > ---
> > >
> > > Masami Hiramatsu (4):
> > > arm: kprobes: Fix to use get_kprobe_ctlblk after irq-disabed
> > > arm: kprobes: Prohibit probing on optimized_callback
> > > arm: kprobes: Prohibit kprobes on do_undefinstr
> > > arm: kprobes: Prohibit kprobes on get_user functions
> > >
> > >
> > > arch/arm/include/asm/assembler.h | 10 ++++++++++
> > > arch/arm/kernel/traps.c | 5 ++++-
> > > arch/arm/lib/getuser.S | 10 ++++++++++
> > > arch/arm/probes/kprobes/opt-arm.c | 4 +++-
> > > 4 files changed, 27 insertions(+), 2 deletions(-)
> > >
> > > --
> > > Masami Hiramatsu (Linaro) <[email protected]>
> >
> > --
> > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> > According to speedtest.net: 8.21Mbps down 510kbps up
>
>
> --
> Masami Hiramatsu <[email protected]>

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

2018-05-12 11:32:06

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [BUGFIX PATCH v3 0/4] arm: kprobes: Fix to prohibit probing on unsafe functions

Hi Russell,

On Tue, 8 May 2018 12:25:03 +0100
Russell King - ARM Linux <[email protected]> wrote:

> On Fri, May 04, 2018 at 01:14:31PM +0900, Masami Hiramatsu wrote:
> > Hi,
> >
> > This is the 3rd version of bugfix series for kprobes on arm.
> > This series fixes 4 different issues which I found.
> >
> > - Fix to use smp_processor_id() after disabling preemption.
> > - Prohibit probing on optimized_callback() for avoiding
> > recursive probe.
> > - Prohibit kprobes on do_undefinstr() by same reason.
> > - Prohibit kprobes on get_user() by same reason.
> >
> > >From v2, I included another 2 bugfixes (1/4 and 2/4)
> > which are not merged yet, and added "Cc: [email protected]",
> > since there are obvious bugs.
>
> Please submit them to the patch system, thanks.

Could you tell me what you mean the patch system?

Thank you,

>
> >
> > Thanks,
> >
> > ---
> >
> > Masami Hiramatsu (4):
> > arm: kprobes: Fix to use get_kprobe_ctlblk after irq-disabed
> > arm: kprobes: Prohibit probing on optimized_callback
> > arm: kprobes: Prohibit kprobes on do_undefinstr
> > arm: kprobes: Prohibit kprobes on get_user functions
> >
> >
> > arch/arm/include/asm/assembler.h | 10 ++++++++++
> > arch/arm/kernel/traps.c | 5 ++++-
> > arch/arm/lib/getuser.S | 10 ++++++++++
> > arch/arm/probes/kprobes/opt-arm.c | 4 +++-
> > 4 files changed, 27 insertions(+), 2 deletions(-)
> >
> > --
> > Masami Hiramatsu (Linaro) <[email protected]>
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up


--
Masami Hiramatsu <[email protected]>

2018-05-12 17:31:39

by Florian Fainelli

[permalink] [raw]
Subject: Re: [BUGFIX PATCH v3 0/4] arm: kprobes: Fix to prohibit probing on unsafe functions

On May 12, 2018 4:31:37 AM PDT, Masami Hiramatsu <[email protected]> wrote:
>Hi Russell,
>
>On Tue, 8 May 2018 12:25:03 +0100
>Russell King - ARM Linux <[email protected]> wrote:
>
>> On Fri, May 04, 2018 at 01:14:31PM +0900, Masami Hiramatsu wrote:
>> > Hi,
>> >
>> > This is the 3rd version of bugfix series for kprobes on arm.
>> > This series fixes 4 different issues which I found.
>> >
>> > - Fix to use smp_processor_id() after disabling preemption.
>> > - Prohibit probing on optimized_callback() for avoiding
>> > recursive probe.
>> > - Prohibit kprobes on do_undefinstr() by same reason.
>> > - Prohibit kprobes on get_user() by same reason.
>> >
>> > >From v2, I included another 2 bugfixes (1/4 and 2/4)
>> > which are not merged yet, and added "Cc: [email protected]",
>> > since there are obvious bugs.
>>
>> Please submit them to the patch system, thanks.
>
>Could you tell me what you mean the patch system?

This is in Russell's signature:

RMK's Patch system: http://www.armlinux.org.uk/developer/patches/

--
Florian

2018-05-13 01:56:05

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [BUGFIX PATCH v3 0/4] arm: kprobes: Fix to prohibit probing on unsafe functions

On Sat, 12 May 2018 10:29:57 -0700
Florian Fainelli <[email protected]> wrote:

> On May 12, 2018 4:31:37 AM PDT, Masami Hiramatsu <[email protected]> wrote:
> >Hi Russell,
> >
> >On Tue, 8 May 2018 12:25:03 +0100
> >Russell King - ARM Linux <[email protected]> wrote:
> >
> >> On Fri, May 04, 2018 at 01:14:31PM +0900, Masami Hiramatsu wrote:
> >> > Hi,
> >> >
> >> > This is the 3rd version of bugfix series for kprobes on arm.
> >> > This series fixes 4 different issues which I found.
> >> >
> >> > - Fix to use smp_processor_id() after disabling preemption.
> >> > - Prohibit probing on optimized_callback() for avoiding
> >> > recursive probe.
> >> > - Prohibit kprobes on do_undefinstr() by same reason.
> >> > - Prohibit kprobes on get_user() by same reason.
> >> >
> >> > >From v2, I included another 2 bugfixes (1/4 and 2/4)
> >> > which are not merged yet, and added "Cc: [email protected]",
> >> > since there are obvious bugs.
> >>
> >> Please submit them to the patch system, thanks.
> >
> >Could you tell me what you mean the patch system?
>
> This is in Russell's signature:
>
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/

Ah, Thanks!
OK, I'll submit via that system!


>
> --
> Florian


--
Masami Hiramatsu <[email protected]>