linux-kernel - Re: [PATCH] riscv: Fix SMP when shadow call stacks are enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 25 Nov 2023 18:06:38 -0600
From:   Samuel Holland <samuel.holland@...ive.com>
To:     Conor Dooley <conor.dooley@...rochip.com>
Cc:     Palmer Dabbelt <palmer@...belt.com>,
        linux-riscv@...ts.infradead.org, Albert Ou <aou@...s.berkeley.edu>,
        Andy Chiu <andy.chiu@...ive.com>,
        Clément Léger <cleger@...osinc.com>,
        Greentime Hu <greentime.hu@...ive.com>,
        Guo Ren <guoren@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Nam Cao <namcaov@...il.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] riscv: Fix SMP when shadow call stacks are enabled

Hi Conor,

On 2023-11-23 8:06 AM, Conor Dooley wrote:
> On Tue, Nov 21, 2023 at 01:19:29PM -0800, Samuel Holland wrote:
>> This fixes two bugs in SCS initialization for secondary CPUs. First,
>> the SCS was not initialized at all in the spinwait boot path. Second,
>> the code for the SBI HSM path attempted to initialize the SCS before
>> enabling the MMU. However, that involves dereferencing the thread
>> pointer, which requires the MMU to be enabled.
>>
>> Fix both issues by setting up the SCS in the common secondary entry
>> path, after enabling the MMU.
> 
> I'm curious, mostly because I do not know much about the implemtnation
> of the shadow call stack, but does it actually work correctly when the
> kernel is built without mmu support?

I imagine it would work. The SCS implementation is purely software; it stores
the return address in a stack at `gp` instead of with the rest of local
variables at `sp`.

The problem here is that we are passing a pointer between CPUs with different
views of the virtual address space (i.e. the boot CPU sees the kernel at
0xffffffff80000000 while the CPU being brought up sees it at its physical
address), and then dereferencing it. If there is no MMU support, then the
virtual address space is identity mapped on all CPUs, and there is no problem.

Regards,
Samuel

>> Fixes: d1584d791a29 ("riscv: Implement Shadow Call Stack")
>> Signed-off-by: Samuel Holland <samuel.holland@...ive.com>
>> ---
>>
>>  arch/riscv/kernel/head.S | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>> index b77397432403..76ace1e0b46f 100644
>> --- a/arch/riscv/kernel/head.S
>> +++ b/arch/riscv/kernel/head.S
>> @@ -154,7 +154,6 @@ secondary_start_sbi:
>>  	XIP_FIXUP_OFFSET a3
>>  	add a3, a3, a1
>>  	REG_L sp, (a3)
>> -	scs_load_current
>>  
>>  .Lsecondary_start_common:
>>  
>> @@ -165,6 +164,7 @@ secondary_start_sbi:
>>  	call relocate_enable_mmu
>>  #endif
>>  	call .Lsetup_trap_vector
>> +	scs_load_current
>>  	tail smp_callin
>>  #endif /* CONFIG_SMP */
>>  
>> -- 
>> 2.42.0
>>