linux-kernel - Re: [PATCH v3 2/4] riscv: add support for SBI Supervisor Software Events extension

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] [day] [month] [year] [list]

Message-ID: <03594835-6f12-4582-a4f7-cbb1630dba91@ghiti.fr>
Date: Thu, 30 Jan 2025 11:01:37 +0100
From: Alexandre Ghiti <alex@...ti.fr>
To: Clément Léger <cleger@...osinc.com>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, linux-riscv@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Cc: Himanshu Chauhan <hchauhan@...tanamicro.com>,
 Anup Patel <apatel@...tanamicro.com>, Xu Lu <luxu.kernel@...edance.com>,
 Atish Patra <atishp@...shpatra.org>
Subject: Re: [PATCH v3 2/4] riscv: add support for SBI Supervisor Software
 Events extension

Hi Clément,

On 28/01/2025 09:10, Clément Léger wrote:
>
> On 27/01/2025 09:09, Alexandre Ghiti wrote:
>>> I believe the goal is not the same. Using CONFIG_VMAP_STACK allows the
>>> kernel exception handling to catch any stack overflow when entering the
>>> kernel and thus using vmalloc is required to allocate twice the page
>>> size (overflow is when sp is located in the upper half of the allocated
>>> vmalloc stack. So basically, this is two distinct purposes.
>>>
>>> AFAIU, kvmalloc allows to fallback to vmalloc if kmalloc fails. This is
>>> not what we are looking for here since our allocation size is always
>>> quite small and known (STACK_SIZE basically).
>>>
>>> But I might be missing something.
>>
>> arch_alloc_vmap_stack() only vmalloc the stack and does not implement
>> any stack overflow mechanism, so I'm still unsure we need the define.
> Hi Alex,
>
> So actually, the stack overflow check itself is done in the exception
> entry. It check if the stack pointer did passed in the upper part of the
> vmalloc allocation (see entry.S:122). In this allocation, the stack size
> is actually * 2:
>
> #ifdef CONFIG_VMAP_STACK
> #define THREAD_ALIGN            (2 * THREAD_SIZE)
> #else
> #define THREAD_ALIGN            THREAD_SIZE
> #endif
>
> So even though it does nothing special by itself, it centralize the
> allocation size/method. And size the size is larger, using vamlloc makes
> sense I guess. The same mechanism is used to allocate irq stack as well.


You're right, it makes sense! Nit: we can avoid the ifdef by using 
IS_ENABLED() but do as you prefer.

Thanks for the explanation,

Alex


>
> Thanks,
>
> Clément
>
>> Thanks,
>>
>> Alex