Le 31/08/2022 à 15:10, Michael Ellerman a écrit :
> This reverts commit ef5b570d3700fbb8628a58da0487486ceeb713cd.
>
> Zhouyi reported that commit is causing crashes when running rcutorture
> with KASAN enabled:
>
> BUG: using smp_processor_id() in preemptible [00000000] code:
> rcu_torture_rea/100
> caller is rcu_preempt_deferred_qs_irqrestore+0x74/0xed0
> CPU: 4 PID: 100 Comm: rcu_torture_rea Tainted: G W
> 5.19.0-rc5-next-20220708-dirty #253
> Call Trace:
> dump_stack_lvl+0xbc/0x108 (unreliable)
> check_preemption_disabled+0x154/0x160
> rcu_preempt_deferred_qs_irqrestore+0x74/0xed0
> __rcu_read_unlock+0x290/0x3b0
> rcu_torture_read_unlock+0x30/0xb0
> rcutorture_one_extend+0x198/0x810
> rcu_torture_one_read+0x58c/0xc90
> rcu_torture_reader+0x12c/0x360
> kthread+0x1e8/0x220
> ret_from_kernel_thread+0x5c/0x64
>
> KASAN will generate instrumentation instructions around the
> WRITE_ONCE(local_paca->irq_soft_mask, mask):
>
> 0xc000000000295cb0 <+0>: addis r2,r12,774
> 0xc000000000295cb4 <+4>: addi r2,r2,16464
> 0xc000000000295cb8 <+8>: mflr r0
> 0xc000000000295cbc <+12>: bl 0xc00000000008bb4c <mcount>
> 0xc000000000295cc0 <+16>: mflr r0
> 0xc000000000295cc4 <+20>: std r31,-8(r1)
> 0xc000000000295cc8 <+24>: addi r3,r13,2354
> 0xc000000000295ccc <+28>: mr r31,r13
> 0xc000000000295cd0 <+32>: std r0,16(r1)
> 0xc000000000295cd4 <+36>: stdu r1,-48(r1)
> 0xc000000000295cd8 <+40>: bl 0xc000000000609b98 <__asan_store1+8>
> 0xc000000000295cdc <+44>: nop
> 0xc000000000295ce0 <+48>: li r9,1
> 0xc000000000295ce4 <+52>: stb r9,2354(r31)
> 0xc000000000295ce8 <+56>: addi r1,r1,48
> 0xc000000000295cec <+60>: ld r0,16(r1)
> 0xc000000000295cf0 <+64>: ld r31,-8(r1)
> 0xc000000000295cf4 <+68>: mtlr r0
>
> If there is a context switch before "stb r9,2354(r31)", r31 may
> not equal to r13, in such case, irq soft mask will not work.
>
> The usual solution of marking the code ineligible for instrumentation
> forces the code out-of-line, which we would prefer to avoid. Christophe
> proposed a partial revert, but Nick raised some concerns with that. So
> for now do a full revert.
>
> Reported-by: Zhouyi Zhou <zhouzho...@gmail.com>
> [mpe: Construct change log based on Zhouyi's original report]
> Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
> ---
> arch/powerpc/include/asm/hw_irq.h | 43 ++++++++++++++++++++++++++-----
> 1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h
> b/arch/powerpc/include/asm/hw_irq.h
> index 3c8cb48f88ae..983551859891 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -113,7 +113,14 @@ static inline void __hard_RI_enable(void)
>
> static inline notrace unsigned long irq_soft_mask_return(void)
> {
> - return READ_ONCE(local_paca->irq_soft_mask);
> + unsigned long flags;
> +
> + asm volatile(
> + "lbz %0,%1(13)"
> + : "=r" (flags)
> + : "i" (offsetof(struct paca_struct, irq_soft_mask)));
> +
> + return flags;
> }
>
> /*
> @@ -140,24 +147,46 @@ static inline notrace void irq_soft_mask_set(unsigned
> long mask)
> if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> WARN_ON(mask && !(mask & IRQS_DISABLED));
>
> - WRITE_ONCE(local_paca->irq_soft_mask, mask);
> - barrier();
> + asm volatile(
> + "stb %0,%1(13)"
> + :
> + : "r" (mask),
> + "i" (offsetof(struct paca_struct, irq_soft_mask))
> + : "memory");
> }
Only the above changes need to be reverted, below ones should remain as
they are.
>
> static inline notrace unsigned long irq_soft_mask_set_return(unsigned long
> mask)
> {
> - unsigned long flags = irq_soft_mask_return();
> + unsigned long flags;
>
> - irq_soft_mask_set(mask);
> +#ifdef CONFIG_PPC_IRQ_SOFT_MASK_DEBUG
> + WARN_ON(mask && !(mask & IRQS_DISABLED));
> +#endif
> +
> + asm volatile(
> + "lbz %0,%1(13); stb %2,%1(13)"
> + : "=&r" (flags)
> + : "i" (offsetof(struct paca_struct, irq_soft_mask)),
> + "r" (mask)
> + : "memory");
>
> return flags;
> }
>
> static inline notrace unsigned long irq_soft_mask_or_return(unsigned long
> mask)
> {
> - unsigned long flags = irq_soft_mask_return();
> + unsigned long flags, tmp;
> +
> + asm volatile(
> + "lbz %0,%2(13); or %1,%0,%3; stb %1,%2(13)"
> + : "=&r" (flags), "=r" (tmp)
> + : "i" (offsetof(struct paca_struct, irq_soft_mask)),
> + "r" (mask)
> + : "memory");
>
> - irq_soft_mask_set(flags | mask);
> +#ifdef CONFIG_PPC_IRQ_SOFT_MASK_DEBUG
> + WARN_ON((mask | flags) && !((mask | flags) & IRQS_DISABLED));
> +#endif
>
> return flags;
> }
