ACK/Cmnt + APPLIED[?]: [HIRSUTE][linux-riscv][PATCH v2] UBUNTU: SAUCE: RISC-V: prevent sbi_send_cpumask_ipi race with ftrace

[Stefan Bader]

On 12.08.21 11:10, Dimitri John Ledkov wrote:
> On Thu, Aug 12, 2021 at 9:47 AM Stefan Bader <stefan.bader at canonical.com> wrote:
>>
>> On 11.08.21 11:23, Dimitri John Ledkov wrote:
>>> From: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
>>>
>>> ftrace will patch instructions in sbi_send_cpumask_ipi, which is going to
>>> be used by flush_icache_range, leading to potential races and crashes like
>>> this:
>>>
>>> [    0.000000] ftrace: allocating 38893 entries in 152 pages
>>> [    0.000000] Oops - illegal instruction [#1]
>>> [    0.000000] Modules linked in:
>>> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.11.0-1014-generic #14-Ubuntu
>>> [    0.000000] epc: ffffffe00000920e ra : ffffffe000009384 sp : ffffffe001803d30
>>> [    0.000000]  gp : ffffffe001a14240 tp : ffffffe00180f440 t0 : ffffffe07fe38000
>>> [    0.000000]  t1 : ffffffe0019cd338 t2 : 0000000000000000 s0 : ffffffe001803d70
>>> [    0.000000]  s1 : 0000000000000000 a0 : ffffffe0000095aa a1 : 0000000000000001
>>> [    0.000000]  a2 : 0000000000000002 a3 : 0000000000000000 a4 : 0000000000000000
>>> [    0.000000]  a5 : 0000000000000000 a6 : 0000000000000004 a7 : 0000000052464e43
>>> [    0.000000]  s2 : 0000000000000002 s3 : 0000000000000001 s4 : 0000000000000000
>>> [    0.000000]  s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000
>>> [    0.000000]  s8 : ffffffe001a170c0 s9 : 0000000000000001 s10: 0000000000000001
>>> [    0.000000]  s11: 00000000fffcc5d0 t3 : 0000000000000068 t4 : 000000000000000b
>>> [    0.000000]  t5 : ffffffe0019cd3e0 t6 : ffffffe001803cd8
>>> [    0.000000] status: 0000000200000100 badaddr: 000000000513f187 cause: 0000000000000002
>>> [    0.000000] ---[ end trace f67eb9af4d8d492b ]---
>>> [    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
>>> [    0.000000] ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
>>>
>>> Where ffffffe00000920e lies in the middle of sbi_send_cpumask_ipi.
>>>
>>> BugLink: https://bugs.launchpad.net/bugs/1934548
>>> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
>>> Tested-by: Dimitri John Ledkov <dimitri.ledkov at canonical.com>
>>> Signed-off-by: Dimitri John Ledkov <dimitri.ledkov at canonical.com>
>>> ---
>>>
>>>    Patch can be applied to:
>>>    - Unstable
>>>    - Impish linux
>>>    - Impish linux-riscv
>>>    - Hirsute linux
>>>    - Hirsute linux-riscv
>>>    - Focal linux-riscv-5.11
>>>
>>>    Whichever are suitable for respins.
>>>
>>>    Changes since v1:
>>>    - Added BugLink, Tested-By, SOB
>>>
>>>    arch/riscv/kernel/sbi.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
>>> index b8f82c73de..9f85f0656f 100644
>>> --- a/arch/riscv/kernel/sbi.c
>>> +++ b/arch/riscv/kernel/sbi.c
>>> @@ -562,7 +562,7 @@ long sbi_get_impid(void)
>>>        return __sbi_base_ecall(SBI_EXT_BASE_GET_MIMPID);
>>>    }
>>>
>>> -static void sbi_send_cpumask_ipi(const struct cpumask *target)
>>> +static void notrace sbi_send_cpumask_ipi(const struct cpumask *target)
>>>    {
>>>        struct cpumask hartid_mask;
>>>
>>>
>>
>> The related bug shows this as applied already without second ACK and not even
>> sending the APPLIED email. Whoever did this should refresh memory about process.
>> Anyhow:
>>
>> Acked-by: Stefan Bader <stefan.bader at canonical.com>
>>
> 
> It is applied into hirsute:linux-riscv (and also
> hirsute:linux-riscv-5.11). But still need to apply into
> impish:unstable & impish:linux-riscv. Maybe just "impish:linux".
> 
> re:second Ack - cascardo is the author, I reviewed & tested the patch
> and also posted it to the mailing list, and got acked by Tim. In my
> mind the two Acks were from myself and Tim. Should I have sent an ACK
> to the mailing too? Or should I have asked cycle lead klebs to send an
> Ack when we agreed to add respin for the said kernels?
> 

You sending it to the mailing list would not count as ack. This is the same as 
sending other patches from upstream. And before you apply the patch you would 
add all ACKS plus an additional SOB for being the one who committed it. And then 
reply to the mailing list that this is applied.

-Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210812/5f77b51d/attachment.sig>