linux-kernel - Re: [PATCH v15 05/27] riscv: usercfi state for task and save/restore of CSR

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 for Android: free password hash cracker in your pocket

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

Message-Id: <D9WLRSAB63M5.3DZD4ND3WVZ6F@ventanamicro.com>
Date: Thu, 15 May 2025 10:48:35 +0200
From: Radim Krčmář <rkrcmar@...tanamicro.com>
To: "Alexandre Ghiti" <alex@...ti.fr>, "Deepak Gupta" <debug@...osinc.com>,
 "Thomas Gleixner" <tglx@...utronix.de>, "Ingo Molnar" <mingo@...hat.com>,
 "Borislav Petkov" <bp@...en8.de>, "Dave Hansen"
 <dave.hansen@...ux.intel.com>, <x86@...nel.org>, "H. Peter Anvin"
 <hpa@...or.com>, "Andrew Morton" <akpm@...ux-foundation.org>, "Liam R.
 Howlett" <Liam.Howlett@...cle.com>, "Vlastimil Babka" <vbabka@...e.cz>,
 "Lorenzo Stoakes" <lorenzo.stoakes@...cle.com>, "Paul Walmsley"
 <paul.walmsley@...ive.com>, "Palmer Dabbelt" <palmer@...belt.com>, "Albert
 Ou" <aou@...s.berkeley.edu>, "Conor Dooley" <conor@...nel.org>, "Rob
 Herring" <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>,
 "Arnd Bergmann" <arnd@...db.de>, "Christian Brauner" <brauner@...nel.org>,
 "Peter Zijlstra" <peterz@...radead.org>, "Oleg Nesterov" <oleg@...hat.com>,
 "Eric Biederman" <ebiederm@...ssion.com>, "Kees Cook" <kees@...nel.org>,
 "Jonathan Corbet" <corbet@....net>, "Shuah Khan" <shuah@...nel.org>, "Jann
 Horn" <jannh@...gle.com>, "Conor Dooley" <conor+dt@...nel.org>, "Miguel
 Ojeda" <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun
 Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <benno.lossin@...ton.me>, "Andreas Hindborg" <a.hindborg@...nel.org>,
 "Alice Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>
Cc: <linux-kernel@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
 <linux-mm@...ck.org>, <linux-riscv@...ts.infradead.org>,
 <devicetree@...r.kernel.org>, <linux-arch@...r.kernel.org>,
 <linux-doc@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
 <alistair.francis@....com>, <richard.henderson@...aro.org>,
 <jim.shu@...ive.com>, <andybnac@...il.com>, <kito.cheng@...ive.com>,
 <charlie@...osinc.com>, <atishp@...osinc.com>, <evan@...osinc.com>,
 <cleger@...osinc.com>, <alexghiti@...osinc.com>, <samitolvanen@...gle.com>,
 <broonie@...nel.org>, <rick.p.edgecombe@...el.com>,
 <rust-for-linux@...r.kernel.org>, "Zong Li" <zong.li@...ive.com>,
 "linux-riscv" <linux-riscv-bounces@...ts.infradead.org>
Subject: Re: [PATCH v15 05/27] riscv: usercfi state for task and
 save/restore of CSR_SSP on trap entry/exit

2025-05-15T09:28:25+02:00, Alexandre Ghiti <alex@...ti.fr>:
> On 06/05/2025 12:10, Radim Krčmář wrote:
>> 2025-05-02T16:30:36-07:00, Deepak Gupta <debug@...osinc.com>:
>>> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
>>> @@ -91,6 +91,32 @@
>>> +.macro restore_userssp tmp
>>> +	ALTERNATIVE("nops(2)",
>>> +		__stringify(				\
>>> +		REG_L \tmp, TASK_TI_USER_SSP(tp);	\
>>> +		csrw CSR_SSP, \tmp),
>>> +		0,
>>> +		RISCV_ISA_EXT_ZICFISS,
>>> +		CONFIG_RISCV_USER_CFI)
>>> +.endm
>> Do we need to emit the nops when CONFIG_RISCV_USER_CFI isn't selected?
>>
>> (Why not put #ifdef CONFIG_RISCV_USER_CFI around the ALTERNATIVES?)
>
> The alternatives are used to create a generic kernel that contains the 
> code for a large number of extensions and only enable it at runtime 
> depending on the platform capabilities. This way distros can ship a 
> single kernel that works on all platforms.

Yup, and if a kernel is compiled without CONFIG_RISCV_USER_CFI, the nops
will only enlarge the binary and potentially slow down execution.
In other words, why we don't do something like this

 (!CONFIG_RISCV_USER_CFI ? "" :
   (RISCV_ISA_EXT_ZICFISS ? __stringify(...) : "nops(x)"))

instead of the current

 (CONFIG_RISCV_USER_CFI &&
    RISCV_ISA_EXT_ZICFISS ? __stringify(...) : "nops(x)")

It could be a new preprocessor macro in case we wanted to make it nice,
but it's probably not a common case, so an ifdef could work as well.

Do we just generally not care about such minor optimizations?

(If we wanted to go an extra mile, we could also keep the nops when both
 CONFIG_RISCV_USER_CFI and RISCV_ISA_EXT_ZICFISS are present, but
 command line riscv_nousercfi disabled backward cfi.)

Thanks.