Re: [Qemu-devel] [PATCH v3 18/23] hw/misc/bcm2835_rng: Use qemu_guest_getrandom_nofail

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> The random number is intended for use by the guest.  As such, we should
> honor the -seed argument for reproducibility.  Use the *_nofail routine
> instead of rolling our own error handling locally.
> 
> Cc: address@hidden
> Cc: Andrew Baumann <address@hidden>
> Cc: Philippe Mathieu-Daudé <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  hw/misc/bcm2835_rng.c | 32 ++++++++++++++------------------
>  1 file changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/misc/bcm2835_rng.c b/hw/misc/bcm2835_rng.c
> index 4d62143b24..fe59c868f5 100644
> --- a/hw/misc/bcm2835_rng.c
> +++ b/hw/misc/bcm2835_rng.c
> @@ -9,30 +9,26 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/log.h"
> -#include "qapi/error.h"
> -#include "crypto/random.h"
> +#include "qemu/guest-random.h"
>  #include "hw/misc/bcm2835_rng.h"
>  
>  static uint32_t get_random_bytes(void)
>  {
>      uint32_t res;
> -    Error *err = NULL;
>  
> -    if (qcrypto_random_bytes((uint8_t *)&res, sizeof(res), &err) < 0) {
> -        /* On failure we don't want to return the guest a non-random
> -         * value in case they're really using it for cryptographic
> -         * purposes, so the best we can do is die here.
> -         * This shouldn't happen unless something's broken.
> -         * In theory we could implement this device's full FIFO
> -         * and interrupt semantics and then just stop filling the
> -         * FIFO. That's a lot of work, though, so we assume any
> -         * errors are systematic problems and trust that if we didn't
> -         * fail as the guest inited then we won't fail later on
> -         * mid-run.
> -         */
> -        error_report_err(err);
> -        exit(1);
> -    }
> +    /*
> +     * On failure we don't want to return the guest a non-random
> +     * value in case they're really using it for cryptographic
> +     * purposes, so the best we can do is die here.
> +     * This shouldn't happen unless something's broken.
> +     * In theory we could implement this device's full FIFO
> +     * and interrupt semantics and then just stop filling the
> +     * FIFO. That's a lot of work, though, so we assume any
> +     * errors are systematic problems and trust that if we didn't
> +     * fail as the guest inited then we won't fail later on
> +     * mid-run.
> +     */
> +    qemu_guest_getrandom_nofail(&res, sizeof(res));
>      return res;
>  }
>  
> 

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>