linux-kernel - Re: [PATCH v3 4/4] RISC-V: Implement archrandom when Zkr is available

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: <ZK2OjvchhqzEKZWR@vermeer>
Date:   Tue, 11 Jul 2023 19:17:02 +0200
From:   Samuel Ortiz <sameo@...osinc.com>
To:     Heiko Stuebner <heiko.stuebner@...ll.eu>
Cc:     Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        linux-riscv@...ts.infradead.org, linux@...osinc.com,
        Conor Dooley <conor.dooley@...rochip.com>,
        Andrew Jones <ajones@...tanamicro.com>,
        Anup Patel <apatel@...tanamicro.com>,
        linux-kernel@...r.kernel.org,
        "Hongren (Zenithal) Zheng" <i@...ithal.me>,
        Guo Ren <guoren@...nel.org>, Atish Patra <atishp@...osinc.com>,
        Björn Töpel <bjorn@...osinc.com>,
        Evan Green <evan@...osinc.com>, devicetree@...r.kernel.org
Subject: Re: [PATCH v3 4/4] RISC-V: Implement archrandom when Zkr is available

Hi Heiko,

On Sun, Jul 09, 2023 at 04:06:16PM +0200, Heiko Stuebner wrote:
> Am Sonntag, 9. Juli 2023, 13:55:46 CEST schrieb Samuel Ortiz:
> > The Zkr extension is ratified and provides 16 bits of entropy seed when
> > reading the SEED CSR.
> > 
> > We can implement arch_get_random_seed_longs() by doing multiple csrrw to
> > that CSR and filling an unsigned long with valid entropy bits.
> > 
> > Acked-by: Conor Dooley <conor.dooley@...rochip.com>
> > Signed-off-by: Samuel Ortiz <sameo@...osinc.com>
> > ---
> 
> > +static inline size_t __must_check arch_get_random_seed_longs(unsigned long *v, size_t max_longs)
> > +{
> > +	if (!max_longs)
> > +		return 0;
> > +
> > +	/*
> > +	 * If Zkr is supported and csr_seed_long succeeds, we return one long
> > +	 * worth of entropy.
> > +	 */
> > +	if (riscv_has_extension_likely(RISCV_ISA_EXT_ZKR) && csr_seed_long(v))
> 
> While this whole thing looks really nice, I don't think you can only
> check the ZKR existence though.
> 
> To access the seed csr from supervisor-mode, it looks like the SSEED
> bit in the mseccfg register also needs to be set by firmware.
> And in the kernel we will likely need to check this setting somehow
> before enabling access.

We can't check it as msseccfg is an M-mode only CSR. While reviewing v2
of this patchset, Stephen suggested to either document the SSEED
requirement with the dt-bindings documentation, use the SBI FWFEATURE
extension to ask firmware to set mseecfg properly, or trap seed access
and feed the caller with a virtual entropy source.
I'd like to go with the second proposed approach (FWFEATURE) but that
requires the corresponding pending patch to be merged first. So for now,
I will only document the SSEED requirement when passing the Zkr
extension, so that we at least have a contract definition for firmwares
that enable Zkr through DT. When they do, they're required to at least
set SSEED in MSSECFG.

I have a couple of pending patches ([1],[2]) related to that, so that an
OpenSBI+qemu+linux combination works as expected when enabling Zkr. I am
going to submit them upstream as well.

Cheers,
Samuel.

[1] https://github.com/qemu/qemu/commit/2a146057099ada946bf4a9c2e355a5a290c23c80
[2] https://github.com/riscv-software-src/opensbi/pull/315