[PATCH 1/3] rng: add OP-TEE based Random Number Generator

Heinrich Schuchardt xypron.glpk at gmx.de
Thu Mar 24 13:06:52 CET 2022 

On 3/24/22 12:03, Patrick DELAUNAY wrote:
> Hi,
>
> On 3/22/22 15:38, Heinrich Schuchardt wrote:
>> On 3/22/22 15:06, Patrick Delaunay wrote:
>>> Add driver for OP-TEE based Random Number Generator on ARM SoCs
>>> where hardware entropy sources are not accessible to normal world
>>> and the RNG service is provided by a HWRNG Trusted Application (TA).
>>>
>>> This driver is based on the linux driver: char/hw_random/optee-rng.c
>>>
>>> Signed-off-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
>>> ---
>>>
>>>   MAINTAINERS             |   1 +
>>>   drivers/rng/Kconfig     |   8 +++
>>>   drivers/rng/Makefile    |   1 +
>>>   drivers/rng/optee_rng.c | 156 ++++++++++++++++++++++++++++++++++++++++
>>>   4 files changed, 166 insertions(+)
>>>   create mode 100644 drivers/rng/optee_rng.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index f25ca7cd00..3356c65dd0 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -481,6 +481,7 @@ F:    drivers/power/regulator/stpmic1.c
>>>   F:    drivers/ram/stm32mp1/
>>>   F:    drivers/remoteproc/stm32_copro.c
>>>   F:    drivers/reset/stm32-reset.c
>>> +F:    drivers/rng/optee_rng.c
>>>   F:    drivers/rng/stm32mp1_rng.c
>>>   F:    drivers/rtc/stm32_rtc.c
>>>   F:    drivers/serial/serial_stm32.*
>>> diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
>>> index b1c5ab93d1..a02c585f61 100644
>>> --- a/drivers/rng/Kconfig
>>> +++ b/drivers/rng/Kconfig
>>> @@ -31,6 +31,14 @@ config RNG_MSM
>>>         This driver provides support for the Random Number
>>>         Generator hardware found on Qualcomm SoCs.
>>>
>>> +config RNG_OPTEE
>>> +    bool "OP-TEE based Random Number Generator support"
>>> +    depends on DM_RNG && OPTEE
>>> +    help
>>> +      This driver provides support for OP-TEE based Random Number
>>> +      Generator on ARM SoCs where hardware entropy sources are not
>>> +      accessible to normal world.
>>
>>
>> https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html
>>
>> has:
>>
>> "By default OP-TEE is configured with a software PRNG. The entropy is
>> added to software PRNG at various places, but unfortunately it is still
>> quite easy to predict the data added as entropy. As a consequence,
>> unless the RNG is based on hardware the generated random will be quite
>> weak."
>>
>> Having a similiar warning in the long text for the CONFIG_RNG_OPTEE
>> symbol would be helpful.
>>
>
> I propose something as :
>
>
> +config RNG_OPTEE
> +    bool "OP-TEE based Random Number Generator support"
> +    depends on DM_RNG && OPTEE
> +    help
> +      This driver provides support for OP-TEE based Random Number

%s/for/for the/

> +      Generator on ARM SoCs where hardware entropy sources are not
> +      accessible to normal world but reserved and used by the OP-TEE
>

%s/\n//

> +      to avoid weakness of the software PRNG.

%s/avoid weakness of the/avoid the weakness of a/

Looks ok to me.

Best regards

Heinrich

>
>
>>> +
>>>   config RNG_STM32MP1
>>>       bool "Enable random number generator for STM32MP1"
>>>       depends on ARCH_STM32MP
>>> diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
>>> index 39f7ee3f03..435b3b965a 100644
>>> --- a/drivers/rng/Makefile
>>> +++ b/drivers/rng/Makefile
>>> @@ -7,6 +7,7 @@ obj-$(CONFIG_DM_RNG) += rng-uclass.o
>>>   obj-$(CONFIG_RNG_MESON) += meson-rng.o
>>>   obj-$(CONFIG_RNG_SANDBOX) += sandbox_rng.o
>>>   obj-$(CONFIG_RNG_MSM) += msm_rng.o
>>> +obj-$(CONFIG_RNG_OPTEE) += optee_rng.o
>>>   obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
>>>   obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o
>>>   obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o
>>> diff --git a/drivers/rng/optee_rng.c b/drivers/rng/optee_rng.c
>>> new file mode 100644
>>> index 0000000000..a0833d0862
>>> --- /dev/null
>>> +++ b/drivers/rng/optee_rng.c
>>> @@ -0,0 +1,156 @@
>>> +// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
>>> +/*
>>> + * Copyright (C) 2022, STMicroelectronics - All Rights Reserved
>>> + */
>>> +#define LOG_CATEGORY UCLASS_RNG
>>> +
>>> +#include <common.h>
>>> +
>>> +#include <rng.h>
>>> +#include <tee.h>
>>> +#include <dm/device.h>
>>> +#include <dm/device_compat.h>
>>> +#include <linux/sizes.h>
>>> +
>>> +#define TEE_ERROR_HEALTH_TEST_FAIL    0x00000001
>>> +
>>> +/*
>>> + * TA_CMD_GET_ENTROPY - Get Entropy from RNG
>>> + *
>>> + * param[0] (inout memref) - Entropy buffer memory reference
>>> + * param[1] unused
>>> + * param[2] unused
>>> + * param[3] unused
>>> + *
>>> + * Result:
>>> + * TEE_SUCCESS - Invoke command success
>>> + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
>>> + * TEE_ERROR_NOT_SUPPORTED - Requested entropy size greater than
>>> size of pool
>>> + * TEE_ERROR_HEALTH_TEST_FAIL - Continuous health testing failed
>>> + */
>>> +#define TA_CMD_GET_ENTROPY        0x0
>>> +
>>> +#define MAX_ENTROPY_REQ_SZ        SZ_4K
>>> +
>>> +#define TA_HWRNG_UUID { 0xab7a617c, 0xb8e7, 0x4d8f, \
>>> +            { 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64 } }
>>> +
>>> +/**
>>> + * struct optee_rng_priv - OP-TEE Random Number Generator private data
>>> + * @session_id:        RNG TA session identifier.
>>> + */
>>> +struct optee_rng_priv {
>>> +    u32 session_id;
>>> +};
>>> +
>>
>> Please, provide as Sphinx style function description.
>>
>
> OK
>
>
>>> +static int get_optee_rng_data(struct udevice *dev,
>>> +                  struct tee_shm *entropy_shm_pool,
>>> +                  void *buf, size_t *size)
>>> +{
>>> +    struct optee_rng_priv *priv = dev_get_priv(dev);
>>> +    int ret = 0;
>>> +    struct tee_invoke_arg arg;
>>
>> Wouldn't it be preferable to use
>>
>> struct tee_invoke_arg arg = { 0 }; ?
>>
>> This way the compiler can optimize the initialization and set all 0
>> initialized variables with a single memset() call.
>>
>
> I wasn't sure of the portability for struct zero-initialisation.
>
> But I will change it in V2.
>
>
>> Best regards
>>
>> Heinrich
>>
>>> +    struct tee_param param;
>>> +
>>> +    memset(&arg, 0, sizeof(arg));
>>> +    memset(&param, 0, sizeof(param));
>>> +
>>> +    /* Invoke TA_CMD_GET_ENTROPY function of Trusted App */
>>> +    arg.func = TA_CMD_GET_ENTROPY;
>>> +    arg.session = priv->session_id;
>>> +
>>> +    /* Fill invoke cmd params */
>>> +    param.attr = TEE_PARAM_ATTR_TYPE_MEMREF_INOUT;
>>> +    param.u.memref.shm = entropy_shm_pool;
>>> +    param.u.memref.size = *size;
>>> +
>>> +    ret = tee_invoke_func(dev->parent, &arg, 1, &param);
>>> +    if (ret || arg.ret) {
>>> +        if (!ret)
>>> +            ret = -EPROTO;
>>> +        dev_err(dev, "TA_CMD_GET_ENTROPY invoke err: %d 0x%x\n",
>>> ret, arg.ret);
>>> +        *size = 0;
>>> +
>>> +        return ret;
>>> +    }
>>> +
>>> +    memcpy(buf, param.u.memref.shm->addr, param.u.memref.size);
>>> +    *size = param.u.memref.size;
>>> +
>>> +    return 0;
>>> +}
>>> +
>
>
> (...)
>
> Regards
>
>
> Patrick
>

More information about the U-Boot mailing list