linux-kernel - Re: [PATCH v3 5/6] hwrng: exynos: Add SMC based TRNG operation

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: <oypijdplsaruia.fsf%l.stelmach@samsung.com>
Date: Fri, 21 Jun 2024 21:00:29 +0200
From: Lukasz Stelmach <l.stelmach@...sung.com>
To: Sam Protsenko <semen.protsenko@...aro.org>
Cc: Krzysztof Kozlowski <krzk+dt@...nel.org>,  Rob Herring
	<robh@...nel.org>,  Conor Dooley <conor+dt@...nel.org>,  Anand Moon
	<linux.amoon@...il.com>,  Olivia Mackall <olivia@...enic.com>,  Herbert Xu
	<herbert@...dor.apana.org.au>,  Alim Akhtar <alim.akhtar@...sung.com>,
	linux-samsung-soc@...r.kernel.org,  linux-crypto@...r.kernel.org,
	devicetree@...r.kernel.org,  linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH v3 5/6] hwrng: exynos: Add SMC based TRNG operation

It was <2024-06-20 czw 18:13>, when Sam Protsenko wrote:
> On some Exynos chips like Exynos850 the access to Security Sub System
> (SSS) registers is protected with TrustZone, and therefore only possible
> from EL3 monitor software. The Linux kernel is running in EL1, so the
> only way for the driver to obtain TRNG data is via SMC calls to EL3
> monitor. Implement such SMC operation and use it when EXYNOS_SMC flag is
> set in the corresponding chip driver data.
>
> Signed-off-by: Sam Protsenko <semen.protsenko@...aro.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> ---
> Changes in v3:
>   - Added appropriate error messages for the case when init SMC command fails
>
> Changes in v2:
>   - Used the "reversed Christmas tree" style in the variable declaration
>     block in exynos_trng_do_read_smc()
>   - Renamed .quirks to .flags in the driver structure
>   - Added Krzysztof's R-b tag
>
>  drivers/char/hw_random/exynos-trng.c | 140 +++++++++++++++++++++++++--
>  1 file changed, 130 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
> index 6ef2ee6c9804..9fa30583cc86 100644
> --- a/drivers/char/hw_random/exynos-trng.c
> +++ b/drivers/char/hw_random/exynos-trng.c

[...]


> @@ -103,6 +163,24 @@ static int exynos_trng_init(struct hwrng *rng)
>  	return 0;
>  }
>  
> +static int exynos_trng_init_smc(struct hwrng *rng)
> +{
> +	struct exynos_trng_dev *trng = (struct exynos_trng_dev *)rng->priv;
> +	struct arm_smccc_res res;
> +	int ret = 0;
> +
> +	arm_smccc_smc(SMC_CMD_RANDOM, HWRNG_INIT, 0, 0, 0, 0, 0, 0, &res);
> +	if (res.a0 != HWRNG_RET_OK) {
> +		dev_err(trng->dev, "SMC command for TRNG init failed (%d)\n",
> +			(int)res.a0);
> +		ret = -EIO;
> +	}
> +	if ((int)res.a0 == -1)
> +		dev_info(trng->dev, "Make sure LDFW is loaded by your BL\n");

This is good, thank you for adding it. It can be even better though, if
you don't skimp on message length (-; I mean, I know what BL is, I can
fingure what LDFW is because you have explained to me and I can see the
source code, but somewone who sees it for the first time will be only
slightly less surprised than with v2 error message only. Come on, you
can make this message twice as long and it will still fit in 80 characters (-;

Don't change it if v3 is the last. If not, please, make it more verbose.

> +
> +	return ret;
> +}
> +


[...]


Kind regards,
-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

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