[PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Lecopzer Chen]

This patch was send by "Sebastian Andrzej Siewior <bigeasy@linutronix.de>"
in [1] and rebase in v5.10-rc6.

The original commit message:

> On non-LPAE systems a write to 0xbffffff0 (modules area) from userland
> results in:
> | BUG: using smp_processor_id() in preemptible [00000000] code: mem-tc/521
> | caller is __do_user_fault.constprop.2+0x4c/0x74
> | CPU: 1 PID: 521 Comm: mem-tc Not tainted 5.1.0-rc1 #4
> | [<c04614e4>] (debug_smp_processor_id) from [<c0116378>] (__do_user_fault.constprop.2+0x4c/0x74)
> | [<c0116378>] (__do_user_fault.constprop.2) from [<c011668c>] (do_page_fault+0x278/0x37c)
> | [<c011668c>] (do_page_fault) from [<c0116904>] (do_DataAbort+0x3c/0xa8)
> | [<c0116904>] (do_DataAbort) from [<c0101e1c>] (__dabt_usr+0x3c/0x40)
>
> Move harden_branch_predictor() from __do_user_fault() to its both
> callers (do_bad_area() and do_page_fault()). The invocation in
> do_page_fault() is added before interrupst are enabled. The invocation
> in do_bad_area() is added just before __do_user_fault() is invoked.

The BUG still exists in v5.10-rc, and the previous disscussion was [2].
This issue can be easily reproduced in ARM with CONFIG_DEBUG_PREEMPT and
CONFIG_HARDEN_BRANCH_PREDICTOR by the following userspace program:

  int *p = 0xffff3ff4;
  *p = 123;

[1]:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20190319203239.gl46fxnfz6gzeeic@linutronix.de/
[2]:
https://lkml.org/lkml/2019/6/3/426

Fixes: f5fe12b1eaee2 ("ARM: spectre-v2: harden user aborts in kernel space")
Reported-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Cc: YJ Chiang <yj.chiang@mediatek.com>
---
 arch/arm/mm/fault.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index efa402025031..f1b57b7d5a0c 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -138,9 +138,6 @@ __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,
 {
 	struct task_struct *tsk = current;
 
-	if (addr > TASK_SIZE)
-		harden_branch_predictor();
-
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {
@@ -173,8 +170,11 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	 * If we are in kernel mode at this point, we
 	 * have no context to handle this fault with.
 	 */
-	if (user_mode(regs))
+	if (user_mode(regs)) {
+		if (addr > TASK_SIZE)
+			harden_branch_predictor();
 		__do_user_fault(addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
+	}
 	else
 		__do_kernel_fault(mm, addr, fsr, regs);
 }
@@ -251,6 +251,9 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	tsk = current;
 	mm  = tsk->mm;
 
+	if (addr > TASK_SIZE && user_mode(regs))
+		harden_branch_predictor();
+
 	/* Enable interrupts if they were enabled in the parent context. */
 	if (interrupts_enabled(regs))
 		local_irq_enable();
-- 
2.18.0

[PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Lecopzer Chen]

This patch was send by "Sebastian Andrzej Siewior <bigeasy@linutronix.de>"
in [1] and rebase in v5.10-rc6.

The original commit message:

> On non-LPAE systems a write to 0xbffffff0 (modules area) from userland
> results in:
> | BUG: using smp_processor_id() in preemptible [00000000] code: mem-tc/521
> | caller is __do_user_fault.constprop.2+0x4c/0x74
> | CPU: 1 PID: 521 Comm: mem-tc Not tainted 5.1.0-rc1 #4
> | [<c04614e4>] (debug_smp_processor_id) from [<c0116378>] (__do_user_fault.constprop.2+0x4c/0x74)
> | [<c0116378>] (__do_user_fault.constprop.2) from [<c011668c>] (do_page_fault+0x278/0x37c)
> | [<c011668c>] (do_page_fault) from [<c0116904>] (do_DataAbort+0x3c/0xa8)
> | [<c0116904>] (do_DataAbort) from [<c0101e1c>] (__dabt_usr+0x3c/0x40)
>
> Move harden_branch_predictor() from __do_user_fault() to its both
> callers (do_bad_area() and do_page_fault()). The invocation in
> do_page_fault() is added before interrupst are enabled. The invocation
> in do_bad_area() is added just before __do_user_fault() is invoked.

The BUG still exists in v5.10-rc, and the previous disscussion was [2].
This issue can be easily reproduced in ARM with CONFIG_DEBUG_PREEMPT and
CONFIG_HARDEN_BRANCH_PREDICTOR by the following userspace program:

  int *p = 0xffff3ff4;
  *p = 123;

[1]:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20190319203239.gl46fxnfz6gzeeic@linutronix.de/
[2]:
https://lkml.org/lkml/2019/6/3/426

Fixes: f5fe12b1eaee2 ("ARM: spectre-v2: harden user aborts in kernel space")
Reported-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Cc: YJ Chiang <yj.chiang@mediatek.com>
---
 arch/arm/mm/fault.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index efa402025031..f1b57b7d5a0c 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -138,9 +138,6 @@ __do_user_fault(unsigned long addr, unsigned int fsr, unsigned int sig,
 {
 	struct task_struct *tsk = current;
 
-	if (addr > TASK_SIZE)
-		harden_branch_predictor();
-
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {
@@ -173,8 +170,11 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	 * If we are in kernel mode at this point, we
 	 * have no context to handle this fault with.
 	 */
-	if (user_mode(regs))
+	if (user_mode(regs)) {
+		if (addr > TASK_SIZE)
+			harden_branch_predictor();
 		__do_user_fault(addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
+	}
 	else
 		__do_kernel_fault(mm, addr, fsr, regs);
 }
@@ -251,6 +251,9 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	tsk = current;
 	mm  = tsk->mm;
 
+	if (addr > TASK_SIZE && user_mode(regs))
+		harden_branch_predictor();
+
 	/* Enable interrupts if they were enabled in the parent context. */
 	if (interrupts_enabled(regs))
 		local_irq_enable();
-- 
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Lecopzer Chen]

Hi all,

I don't see any fix for this issue now(maybe I missed it..?),
could we fix this if there is better solution?
This issue exists almost two years.

Thanks!

BRs,
Lecopzer

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Lecopzer Chen]

Hi all,

I don't see any fix for this issue now(maybe I missed it..?),
could we fix this if there is better solution?
This issue exists almost two years.

Thanks!

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

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Russell King - ARM Linux admin]

On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> Hi all,
> 
> I don't see any fix for this issue now(maybe I missed it..?),
> could we fix this if there is better solution?
> This issue exists almost two years.

I don't think anyone provided an acceptable patch.

The first patch moved the hardening out of the translation/section
fault handling. Since the kernel is mapped with sections, these
are above TASK_SIZE, and the whole point of the branch prediction
hardening is to prevent the prediction in the kernel being exploited,
missing the hardening effectively makes the mitigation useless.

The discussion in February 2019 never concluded from what I can see.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Russell King - ARM Linux admin]

On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> Hi all,
> 
> I don't see any fix for this issue now(maybe I missed it..?),
> could we fix this if there is better solution?
> This issue exists almost two years.

I don't think anyone provided an acceptable patch.

The first patch moved the hardening out of the translation/section
fault handling. Since the kernel is mapped with sections, these
are above TASK_SIZE, and the whole point of the branch prediction
hardening is to prevent the prediction in the kernel being exploited,
missing the hardening effectively makes the mitigation useless.

The discussion in February 2019 never concluded from what I can see.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Sebastian Andrzej Siewior]

On 2021-01-26 10:59:32 [+0000], Russell King - ARM Linux admin wrote:
> On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> > Hi all,
> > 
> > I don't see any fix for this issue now(maybe I missed it..?),
> > could we fix this if there is better solution?
> > This issue exists almost two years.
> 
> I don't think anyone provided an acceptable patch.
> 
> The first patch moved the hardening out of the translation/section
> fault handling. Since the kernel is mapped with sections, these
> are above TASK_SIZE, and the whole point of the branch prediction
> hardening is to prevent the prediction in the kernel being exploited,
> missing the hardening effectively makes the mitigation useless.
> 
> The discussion in February 2019 never concluded from what I can see.

My memory is that I never got a reply which I understood.
Let me try again this week with the information above.

Sebastian

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Sebastian Andrzej Siewior]

On 2021-01-26 10:59:32 [+0000], Russell King - ARM Linux admin wrote:
> On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> > Hi all,
> > 
> > I don't see any fix for this issue now(maybe I missed it..?),
> > could we fix this if there is better solution?
> > This issue exists almost two years.
> 
> I don't think anyone provided an acceptable patch.
> 
> The first patch moved the hardening out of the translation/section
> fault handling. Since the kernel is mapped with sections, these
> are above TASK_SIZE, and the whole point of the branch prediction
> hardening is to prevent the prediction in the kernel being exploited,
> missing the hardening effectively makes the mitigation useless.
> 
> The discussion in February 2019 never concluded from what I can see.

My memory is that I never got a reply which I understood.
Let me try again this week with the information above.

Sebastian

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

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Lecopzer Chen]

> On 2021-01-26 10:59:32 [+0000], Russell King - ARM Linux admin wrote:
> > On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> > > Hi all,
> > > 
> > > I don't see any fix for this issue now(maybe I missed it..?),
> > > could we fix this if there is better solution?
> > > This issue exists almost two years.
> > 
> > I don't think anyone provided an acceptable patch.
> > 
> > The first patch moved the hardening out of the translation/section
> > fault handling. Since the kernel is mapped with sections, these
> > are above TASK_SIZE, and the whole point of the branch prediction
> > hardening is to prevent the prediction in the kernel being exploited,
> > missing the hardening effectively makes the mitigation useless.
> > 
> > The discussion in February 2019 never concluded from what I can see.
> 
> My memory is that I never got a reply which I understood.
> Let me try again this week with the information above.


NOTE:
Before sending this mail, I had searched the relative threads and
there are two solutions in general:
    1. Add get_pcpu()/put_cpu() https://lkml.org/lkml/2019/6/3/426
       Reject by Marc:
       > The right fix would be to move the call to a point where we haven't
       > enabled preemption yet.

    2. Move out like the patch from Sebastian:
       This seems follow the concept of 1.
       (move the call to a point where we haven't enabled preemption yet).
       But I can't find any reply in the thread.

Now the CONFIG_HARDEN_BRANCH_PREDICTOR has already backported to LTS,
and after upgrading ARM CONFIG_CPU_V7 products to latest LTS, the
CONFIG_HARDEN_BRANCH_PREDICTOR will be default y and this issue makes
our devices panic and we have to either disable HARDEN_BRANCH_PREDICTOR
or hack in-house to avoid the kernel panic.


I'll wait for anyone have acceptable idea and patching(maybe Sebastian could? :)),
and I'll also take some time to find other solution too.


Thanks,
Lecopzer

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Lecopzer Chen]

> On 2021-01-26 10:59:32 [+0000], Russell King - ARM Linux admin wrote:
> > On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> > > Hi all,
> > > 
> > > I don't see any fix for this issue now(maybe I missed it..?),
> > > could we fix this if there is better solution?
> > > This issue exists almost two years.
> > 
> > I don't think anyone provided an acceptable patch.
> > 
> > The first patch moved the hardening out of the translation/section
> > fault handling. Since the kernel is mapped with sections, these
> > are above TASK_SIZE, and the whole point of the branch prediction
> > hardening is to prevent the prediction in the kernel being exploited,
> > missing the hardening effectively makes the mitigation useless.
> > 
> > The discussion in February 2019 never concluded from what I can see.
> 
> My memory is that I never got a reply which I understood.
> Let me try again this week with the information above.


NOTE:
Before sending this mail, I had searched the relative threads and
there are two solutions in general:
    1. Add get_pcpu()/put_cpu() https://lkml.org/lkml/2019/6/3/426
       Reject by Marc:
       > The right fix would be to move the call to a point where we haven't
       > enabled preemption yet.

    2. Move out like the patch from Sebastian:
       This seems follow the concept of 1.
       (move the call to a point where we haven't enabled preemption yet).
       But I can't find any reply in the thread.

Now the CONFIG_HARDEN_BRANCH_PREDICTOR has already backported to LTS,
and after upgrading ARM CONFIG_CPU_V7 products to latest LTS, the
CONFIG_HARDEN_BRANCH_PREDICTOR will be default y and this issue makes
our devices panic and we have to either disable HARDEN_BRANCH_PREDICTOR
or hack in-house to avoid the kernel panic.


I'll wait for anyone have acceptable idea and patching(maybe Sebastian could? :)),
and I'll also take some time to find other solution too.


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

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Russell King - ARM Linux admin]

On Tue, Jan 26, 2021 at 11:01:50PM +0800, Lecopzer Chen wrote:
> > On 2021-01-26 10:59:32 [+0000], Russell King - ARM Linux admin wrote:
> > > On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> > > > Hi all,
> > > > 
> > > > I don't see any fix for this issue now(maybe I missed it..?),
> > > > could we fix this if there is better solution?
> > > > This issue exists almost two years.
> > > 
> > > I don't think anyone provided an acceptable patch.
> > > 
> > > The first patch moved the hardening out of the translation/section
> > > fault handling. Since the kernel is mapped with sections, these
> > > are above TASK_SIZE, and the whole point of the branch prediction
> > > hardening is to prevent the prediction in the kernel being exploited,
> > > missing the hardening effectively makes the mitigation useless.
> > > 
> > > The discussion in February 2019 never concluded from what I can see.
> > 
> > My memory is that I never got a reply which I understood.
> > Let me try again this week with the information above.
> 
> 
> NOTE:
> Before sending this mail, I had searched the relative threads and
> there are two solutions in general:
>     1. Add get_pcpu()/put_cpu() https://lkml.org/lkml/2019/6/3/426
>        Reject by Marc:
>        > The right fix would be to move the call to a point where we haven't
>        > enabled preemption yet.
> 
>     2. Move out like the patch from Sebastian:
>        This seems follow the concept of 1.
>        (move the call to a point where we haven't enabled preemption yet).
>        But I can't find any reply in the thread.
> 
> Now the CONFIG_HARDEN_BRANCH_PREDICTOR has already backported to LTS,
> and after upgrading ARM CONFIG_CPU_V7 products to latest LTS, the
> CONFIG_HARDEN_BRANCH_PREDICTOR will be default y and this issue makes
> our devices panic and we have to either disable HARDEN_BRANCH_PREDICTOR
> or hack in-house to avoid the kernel panic.

It does _not_ cause the kernel to panic, ever. A kernel panic takes
out the system. This is not the case here.

It merely causes a noisy message to be emitted in the kernel log, and
the system survives. That is way more preferable than breaking the
effect of branch predictor hardening.

If it is taking out your kernel with a real panic, then there is
something wrong elsewhere - and this is _not_ something that should
be happening during normal system operation.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Russell King - ARM Linux admin]

On Tue, Jan 26, 2021 at 11:01:50PM +0800, Lecopzer Chen wrote:
> > On 2021-01-26 10:59:32 [+0000], Russell King - ARM Linux admin wrote:
> > > On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> > > > Hi all,
> > > > 
> > > > I don't see any fix for this issue now(maybe I missed it..?),
> > > > could we fix this if there is better solution?
> > > > This issue exists almost two years.
> > > 
> > > I don't think anyone provided an acceptable patch.
> > > 
> > > The first patch moved the hardening out of the translation/section
> > > fault handling. Since the kernel is mapped with sections, these
> > > are above TASK_SIZE, and the whole point of the branch prediction
> > > hardening is to prevent the prediction in the kernel being exploited,
> > > missing the hardening effectively makes the mitigation useless.
> > > 
> > > The discussion in February 2019 never concluded from what I can see.
> > 
> > My memory is that I never got a reply which I understood.
> > Let me try again this week with the information above.
> 
> 
> NOTE:
> Before sending this mail, I had searched the relative threads and
> there are two solutions in general:
>     1. Add get_pcpu()/put_cpu() https://lkml.org/lkml/2019/6/3/426
>        Reject by Marc:
>        > The right fix would be to move the call to a point where we haven't
>        > enabled preemption yet.
> 
>     2. Move out like the patch from Sebastian:
>        This seems follow the concept of 1.
>        (move the call to a point where we haven't enabled preemption yet).
>        But I can't find any reply in the thread.
> 
> Now the CONFIG_HARDEN_BRANCH_PREDICTOR has already backported to LTS,
> and after upgrading ARM CONFIG_CPU_V7 products to latest LTS, the
> CONFIG_HARDEN_BRANCH_PREDICTOR will be default y and this issue makes
> our devices panic and we have to either disable HARDEN_BRANCH_PREDICTOR
> or hack in-house to avoid the kernel panic.

It does _not_ cause the kernel to panic, ever. A kernel panic takes
out the system. This is not the case here.

It merely causes a noisy message to be emitted in the kernel log, and
the system survives. That is way more preferable than breaking the
effect of branch predictor hardening.

If it is taking out your kernel with a real panic, then there is
something wrong elsewhere - and this is _not_ something that should
be happening during normal system operation.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Lecopzer Chen]

> On Tue, Jan 26, 2021 at 11:01:50PM +0800, Lecopzer Chen wrote:
> > > On 2021-01-26 10:59:32 [+0000], Russell King - ARM Linux admin wrote:
> > > > On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> > > > > Hi all,
> > > > > 
> > > > > I don't see any fix for this issue now(maybe I missed it..?),
> > > > > could we fix this if there is better solution?
> > > > > This issue exists almost two years.
> > > > 
> > > > I don't think anyone provided an acceptable patch.
> > > > 
> > > > The first patch moved the hardening out of the translation/section
> > > > fault handling. Since the kernel is mapped with sections, these
> > > > are above TASK_SIZE, and the whole point of the branch prediction
> > > > hardening is to prevent the prediction in the kernel being exploited,
> > > > missing the hardening effectively makes the mitigation useless.
> > > > 
> > > > The discussion in February 2019 never concluded from what I can see.
> > > 
> > > My memory is that I never got a reply which I understood.
> > > Let me try again this week with the information above.
> > 
> > 
> > NOTE:
> > Before sending this mail, I had searched the relative threads and
> > there are two solutions in general:
> >     1. Add get_pcpu()/put_cpu() https://lkml.org/lkml/2019/6/3/426
> >        Reject by Marc:
> >        > The right fix would be to move the call to a point where we haven't
> >        > enabled preemption yet.
> > 
> >     2. Move out like the patch from Sebastian:
> >        This seems follow the concept of 1.
> >        (move the call to a point where we haven't enabled preemption yet).
> >        But I can't find any reply in the thread.
> > 
> > Now the CONFIG_HARDEN_BRANCH_PREDICTOR has already backported to LTS,
> > and after upgrading ARM CONFIG_CPU_V7 products to latest LTS, the
> > CONFIG_HARDEN_BRANCH_PREDICTOR will be default y and this issue makes
> > our devices panic and we have to either disable HARDEN_BRANCH_PREDICTOR
> > or hack in-house to avoid the kernel panic.
> 
> It does _not_ cause the kernel to panic, ever. A kernel panic takes
> out the system. This is not the case here.
> 
> It merely causes a noisy message to be emitted in the kernel log, and
> the system survives. That is way more preferable than breaking the
> effect of branch predictor hardening.
> 
> If it is taking out your kernel with a real panic, then there is
> something wrong elsewhere - and this is _not_ something that should
> be happening during normal system operation.

Oh, yes, you're right;

After reread the panic log, our panic happened because
-> invalid userspace memory access
-> debug_preempt log
-> the program seg fault
-> main service need the program but it crash
-> panic

Sorry for wrong information and thanks a lot for the correctness.
I think I have to see why the in-house hacking is working...

Thanks!!

BRs,
Lecopzer

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Lecopzer Chen]

> On Tue, Jan 26, 2021 at 11:01:50PM +0800, Lecopzer Chen wrote:
> > > On 2021-01-26 10:59:32 [+0000], Russell King - ARM Linux admin wrote:
> > > > On Tue, Jan 26, 2021 at 05:17:08PM +0800, Lecopzer Chen wrote:
> > > > > Hi all,
> > > > > 
> > > > > I don't see any fix for this issue now(maybe I missed it..?),
> > > > > could we fix this if there is better solution?
> > > > > This issue exists almost two years.
> > > > 
> > > > I don't think anyone provided an acceptable patch.
> > > > 
> > > > The first patch moved the hardening out of the translation/section
> > > > fault handling. Since the kernel is mapped with sections, these
> > > > are above TASK_SIZE, and the whole point of the branch prediction
> > > > hardening is to prevent the prediction in the kernel being exploited,
> > > > missing the hardening effectively makes the mitigation useless.
> > > > 
> > > > The discussion in February 2019 never concluded from what I can see.
> > > 
> > > My memory is that I never got a reply which I understood.
> > > Let me try again this week with the information above.
> > 
> > 
> > NOTE:
> > Before sending this mail, I had searched the relative threads and
> > there are two solutions in general:
> >     1. Add get_pcpu()/put_cpu() https://lkml.org/lkml/2019/6/3/426
> >        Reject by Marc:
> >        > The right fix would be to move the call to a point where we haven't
> >        > enabled preemption yet.
> > 
> >     2. Move out like the patch from Sebastian:
> >        This seems follow the concept of 1.
> >        (move the call to a point where we haven't enabled preemption yet).
> >        But I can't find any reply in the thread.
> > 
> > Now the CONFIG_HARDEN_BRANCH_PREDICTOR has already backported to LTS,
> > and after upgrading ARM CONFIG_CPU_V7 products to latest LTS, the
> > CONFIG_HARDEN_BRANCH_PREDICTOR will be default y and this issue makes
> > our devices panic and we have to either disable HARDEN_BRANCH_PREDICTOR
> > or hack in-house to avoid the kernel panic.
> 
> It does _not_ cause the kernel to panic, ever. A kernel panic takes
> out the system. This is not the case here.
> 
> It merely causes a noisy message to be emitted in the kernel log, and
> the system survives. That is way more preferable than breaking the
> effect of branch predictor hardening.
> 
> If it is taking out your kernel with a real panic, then there is
> something wrong elsewhere - and this is _not_ something that should
> be happening during normal system operation.

Oh, yes, you're right;

After reread the panic log, our panic happened because
-> invalid userspace memory access
-> debug_preempt log
-> the program seg fault
-> main service need the program but it crash
-> panic

Sorry for wrong information and thanks a lot for the correctness.
I think I have to see why the in-house hacking is working...

Thanks!!

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

[PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Sebastian Andrzej Siewior]

On non-LPAE systems a write to 0xbffffff0 (modules area) from userland
results in:
| BUG: using smp_processor_id() in preemptible [00000000] code: mem-tc/521
| caller is __do_user_fault.constprop.2+0x4c/0x74
| CPU: 1 PID: 521 Comm: mem-tc Not tainted 5.1.0-rc1 #4
| [<c04614e4>] (debug_smp_processor_id) from [<c0116378>] (__do_user_fault.constprop.2+0x4c/0x74)
| [<c0116378>] (__do_user_fault.constprop.2) from [<c011668c>] (do_page_fault+0x278/0x37c)
| [<c011668c>] (do_page_fault) from [<c0116904>] (do_DataAbort+0x3c/0xa8)
| [<c0116904>] (do_DataAbort) from [<c0101e1c>] (__dabt_usr+0x3c/0x40)

Move harden_branch_predictor() from __do_user_fault() to its both
callers (do_bad_area() and do_page_fault()). The invocation in
do_page_fault() is added before interrupst are enabled. The invocation
in do_bad_area() is added just before __do_user_fault() is invoked.

Fixes: f5fe12b1eaee2 ("ARM: spectre-v2: harden user aborts in kernel space")
Reported-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm/mm/fault.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 58f69fa07df95..7adff8eb8f3d2 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -161,9 +161,6 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
 		unsigned int fsr, unsigned int sig, int code,
 		struct pt_regs *regs)
 {
-	if (addr > TASK_SIZE)
-		harden_branch_predictor();
-
 #ifdef CONFIG_DEBUG_USER
 	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
 	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {
@@ -195,10 +192,13 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	 * If we are in kernel mode at this point, we
 	 * have no context to handle this fault with.
 	 */
-	if (user_mode(regs))
+	if (user_mode(regs)) {
+		if (addr > TASK_SIZE)
+			harden_branch_predictor();
 		__do_user_fault(tsk, addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
-	else
+	} else {
 		__do_kernel_fault(mm, addr, fsr, regs);
+	}
 }
 
 #ifdef CONFIG_MMU
@@ -272,6 +272,8 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	tsk = current;
 	mm  = tsk->mm;
 
+	if (addr > TASK_SIZE && user_mode(regs))
+		harden_branch_predictor();
 	/* Enable interrupts if they were enabled in the parent context. */
 	if (interrupts_enabled(regs))
 		local_irq_enable();
-- 
2.20.1


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

Re: [PATCH] ARM: mm: harden branch predictor before opening interrupts during fault

[Sebastian Andrzej Siewior]

On 2019-03-19 21:32:39 [+0100], To linux-arm-kernel@lists.infradead.org wrote:
> On non-LPAE systems a write to 0xbffffff0 (modules area) from userland
> results in:
> | BUG: using smp_processor_id() in preemptible [00000000] code: mem-tc/521
> | caller is __do_user_fault.constprop.2+0x4c/0x74
> | CPU: 1 PID: 521 Comm: mem-tc Not tainted 5.1.0-rc1 #4
> | [<c04614e4>] (debug_smp_processor_id) from [<c0116378>] (__do_user_fault.constprop.2+0x4c/0x74)
> | [<c0116378>] (__do_user_fault.constprop.2) from [<c011668c>] (do_page_fault+0x278/0x37c)
> | [<c011668c>] (do_page_fault) from [<c0116904>] (do_DataAbort+0x3c/0xa8)
> | [<c0116904>] (do_DataAbort) from [<c0101e1c>] (__dabt_usr+0x3c/0x40)
> 
> Move harden_branch_predictor() from __do_user_fault() to its both
> callers (do_bad_area() and do_page_fault()). The invocation in
> do_page_fault() is added before interrupst are enabled. The invocation
> in do_bad_area() is added just before __do_user_fault() is invoked.

In 20190216113338.irr5j4ukhpwngval@shell.armlinux.org.uk Russel
complained that I am opening a window for branch predictor attacks that
he tried to close. This is no longer the case because
harden_branch_predictor() is now in do_page_fault() and do_bad_area().

So is this still obviously wrong and I don't see it?

> Fixes: f5fe12b1eaee2 ("ARM: spectre-v2: harden user aborts in kernel space")
> Reported-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/arm/mm/fault.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index 58f69fa07df95..7adff8eb8f3d2 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -161,9 +161,6 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
>  		unsigned int fsr, unsigned int sig, int code,
>  		struct pt_regs *regs)
>  {
> -	if (addr > TASK_SIZE)
> -		harden_branch_predictor();
> -
>  #ifdef CONFIG_DEBUG_USER
>  	if (((user_debug & UDBG_SEGV) && (sig == SIGSEGV)) ||
>  	    ((user_debug & UDBG_BUS)  && (sig == SIGBUS))) {
> @@ -195,10 +192,13 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>  	 * If we are in kernel mode at this point, we
>  	 * have no context to handle this fault with.
>  	 */
> -	if (user_mode(regs))
> +	if (user_mode(regs)) {
> +		if (addr > TASK_SIZE)
> +			harden_branch_predictor();
>  		__do_user_fault(tsk, addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
> -	else
> +	} else {
>  		__do_kernel_fault(mm, addr, fsr, regs);
> +	}
>  }
>  
>  #ifdef CONFIG_MMU
> @@ -272,6 +272,8 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>  	tsk = current;
>  	mm  = tsk->mm;
>  
> +	if (addr > TASK_SIZE && user_mode(regs))
> +		harden_branch_predictor();
>  	/* Enable interrupts if they were enabled in the parent context. */
>  	if (interrupts_enabled(regs))
>  		local_irq_enable();

Sebastian

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