This patch was send by "Sebastian Andrzej Siewior <[email protected]>"
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/[email protected]/
[2]:
https://lkml.org/lkml/2019/6/3/426
Fixes: f5fe12b1eaee2 ("ARM: spectre-v2: harden user aborts in kernel space")
Reported-by: Bernd Edlinger <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Lecopzer Chen <[email protected]>
Cc: YJ Chiang <[email protected]>
---
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
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!
> 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
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!
> 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
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
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