Re: [PATCH v2 1/2] hwrng: fetch randomness only after device init

[Kees Cook]

On Fri, Jul 4, 2014 at 10:34 PM, Amit Shah <amit.shah@xxxxxxxxxx> wrote:
> Commit d9e7972619334 "hwrng: add randomness to system from rng sources"
> added a call to rng_get_data() from the hwrng_register() function.
> However, some rng devices need initialization before data can be read
> from them.
>
> This commit makes the call to rng_get_data() depend on no init fn
> pointer being registered by the device.  If an init function is
> registered, this call is made after device init.

Thanks, this seems pretty reasonable. One side-effect is that cycling
between hwrngs via sysfs (when they have init functions) will cause
them to add more entropy. I don't think this is a problem, but it is
kind of a weird side-effect.

Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>

-Kees

>
> CC: Kees Cook <keescook@xxxxxxxxxxxx>
> CC: Jason Cooper <jason@xxxxxxxxxxxxxx>
> CC: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> CC: <stable@xxxxxxxxxxxxxxx> # For v3.15+
> Signed-off-by: Amit Shah <amit.shah@xxxxxxxxxx>
> ---
>  drivers/char/hw_random/core.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 334601c..df95e2f 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -55,16 +55,37 @@ static DEFINE_MUTEX(rng_mutex);
>  static int data_avail;
>  static u8 *rng_buffer;
>
> +static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
> +                              int wait);
> +
>  static size_t rng_buffer_size(void)
>  {
>         return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES;
>  }
>
> +static void add_early_randomness(struct hwrng *rng)
> +{
> +       unsigned char bytes[16];
> +       int bytes_read;
> +
> +       bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1);
> +       if (bytes_read > 0)
> +               add_device_randomness(bytes, bytes_read);
> +}
> +
>  static inline int hwrng_init(struct hwrng *rng)
>  {
> +       int ret;
> +
>         if (!rng->init)
>                 return 0;
> -       return rng->init(rng);
> +
> +       ret =  rng->init(rng);
> +       if (ret)
> +               return ret;
> +
> +       add_early_randomness(rng);
> +       return ret;
>  }
>
>  static inline void hwrng_cleanup(struct hwrng *rng)
> @@ -304,8 +325,6 @@ int hwrng_register(struct hwrng *rng)
>  {
>         int err = -EINVAL;
>         struct hwrng *old_rng, *tmp;
> -       unsigned char bytes[16];
> -       int bytes_read;
>
>         if (rng->name == NULL ||
>             (rng->data_read == NULL && rng->read == NULL))
> @@ -347,9 +366,9 @@ int hwrng_register(struct hwrng *rng)
>         INIT_LIST_HEAD(&rng->list);
>         list_add_tail(&rng->list, &rng_list);
>
> -       bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1);
> -       if (bytes_read > 0)
> -               add_device_randomness(bytes, bytes_read);
> +       if (!rng->init)
> +               add_early_randomness(rng);
> +
>  out_unlock:
>         mutex_unlock(&rng_mutex);
>  out:
> --
> 1.9.3
>



-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/