linux-kernel - Re: [PATCH v3 2/6] RISC-V: add vector crypto extension validation checks

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]

Message-ID: <20250211-implode-mute-8ff31f44bdf9@spud>
Date: Tue, 11 Feb 2025 12:34:57 +0000
From: Conor Dooley <conor@...nel.org>
To: Clément Léger <cleger@...osinc.com>
Cc: linux-riscv@...ts.infradead.org,
	Conor Dooley <conor.dooley@...rochip.com>,
	Eric Biggers <ebiggers@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>, Andy Chiu <andybnac@...il.com>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/6] RISC-V: add vector crypto extension validation
 checks

On Tue, Feb 11, 2025 at 09:45:44AM +0100, Clément Léger wrote:
> 
> 
> On 05/02/2025 17:05, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@...rochip.com>
> > 
> > Using Clement's new validation callbacks, support checking that
> > dependencies have been satisfied for the vector crpyto extensions.
> > Currently riscv_isa_extension_available(<vector crypto>) will return
> > true on systems that support the extensions but vector itself has been
> > disabled by the kernel, adding validation callbacks will prevent such a
> > scenario from occuring and make the behaviour of the extension detection
> > functions more consistent with user expectations - it's not expected to
> > have to check for vector AND the specific crypto extension.
> > 
> > The 1.0.0 Vector crypto spec states:
> > 	The Zvknhb and Zvbc Vector Crypto Extensions --and accordingly
> > 	the composite extensions Zvkn and Zvks-- require a Zve64x base,
> > 	or application ("V") base Vector Extension. All of the other
> > 	Vector Crypto Extensions can be built on any embedded (Zve*) or
> > 	application ("V") base Vector Extension.
> > and this could be used as the basis for checking that the correct base
> > for individual crypto extensions, but that's not really the kernel's job
> > in my opinion and it is sufficient to leave that sort of precision to
> > the dt-bindings. The kernel only needs to make sure that vector, in some
> > form, is available.
> > 
> > Since vector will now be disabled proactively, there's no need to clear
> > the bit in elf_hwcap in riscv_fill_hwcap() any longer.
> > 
> > Link: https://github.com/riscv/riscv-crypto/releases/tag/v1.0.0
> > Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
> > ---
> >  arch/riscv/kernel/cpufeature.c | 49 +++++++++++++++++++++++-----------
> >  1 file changed, 33 insertions(+), 16 deletions(-)
> > 
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 40a24b08d905..1c148ecea612 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -138,6 +138,23 @@ static int riscv_ext_vector_float_validate(const struct riscv_isa_ext_data *data
> >  	return 0;
> >  }
> >  
> > +static int riscv_ext_vector_crypto_validate(const struct riscv_isa_ext_data *data,
> > +					    const unsigned long *isa_bitmap)
> > +{
> > +	if (!IS_ENABLED(CONFIG_RISCV_ISA_V))
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * It isn't the kernel's job to check that the binding is correct, so
> > +	 * it should be enough to check that any of the vector extensions are
> > +	 * enabled, which in-turn means that vector is usable in this kernel
> > +	 */
> > +	if (!__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZVE32X))
> > +		return -EINVAL;
> 
> After a second thought, I think it should be this:
> 
> if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZVE32X))
> 	return 0;
> 
> return -EPROBEDEFER;
> 
> Extensions can be enabled later (but can not be "reverted") so check for
> the extension to be present (in which case it's ok), or wait for it to
> be (potentially) enabled.

Ah, of course it is operating on the /resolved/ isa, not the source one.
Makes me thing the parameter of all the validate callbacks should be
"resolved_isa_bitmap" instead of "isa_bitmap" to make things clearer?


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)