[Qemu-arm] [PATCH v3 16/23] aspeed/scu: Use qemu_guest_getrandom_nofail

[Qemu-arm] [PATCH v3 16/23] aspeed/scu: Use qemu_guest_getrandom_nofail

[Richard Henderson]

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: qemu-arm@nongnu.org
Cc: Cédric Le Goater <clg@kaod.org>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Joel Stanley <joel@jms.id.au>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/misc/aspeed_scu.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index c8217740ef..ab1e18ed4b 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -16,7 +16,7 @@
 #include "qapi/visitor.h"
 #include "qemu/bitops.h"
 #include "qemu/log.h"
-#include "crypto/random.h"
+#include "qemu/guest-random.h"
 #include "trace.h"
 
 #define TO_REG(offset) ((offset) >> 2)
@@ -157,14 +157,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
 
 static uint32_t aspeed_scu_get_random(void)
 {
-    Error *err = NULL;
     uint32_t num;
-
-    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
-        error_report_err(err);
-        exit(1);
-    }
-
+    qemu_guest_getrandom_nofail(&num, sizeof(num));
     return num;
 }
 
-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 16/23] aspeed/scu: Use qemu_guest_getrandom_nofail

[Joel Stanley]

On Fri, 15 Mar 2019 at 03:26, Richard Henderson
<richard.henderson@linaro.org> 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.

Nice!

Reviewed-by: Joel Stanley <joel@jms.id.au>

>
> Cc: qemu-arm@nongnu.org
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  hw/misc/aspeed_scu.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index c8217740ef..ab1e18ed4b 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -16,7 +16,7 @@
>  #include "qapi/visitor.h"
>  #include "qemu/bitops.h"
>  #include "qemu/log.h"
> -#include "crypto/random.h"
> +#include "qemu/guest-random.h"
>  #include "trace.h"
>
>  #define TO_REG(offset) ((offset) >> 2)
> @@ -157,14 +157,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>
>  static uint32_t aspeed_scu_get_random(void)
>  {
> -    Error *err = NULL;
>      uint32_t num;
> -
> -    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> -
> +    qemu_guest_getrandom_nofail(&num, sizeof(num));
>      return num;
>  }
>
> --
> 2.17.2
>

Re: [Qemu-devel] [PATCH v3 16/23] aspeed/scu: Use qemu_guest_getrandom_nofail

[Cédric Le Goater]

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: qemu-arm@nongnu.org
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Thanks

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/misc/aspeed_scu.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index c8217740ef..ab1e18ed4b 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -16,7 +16,7 @@
>  #include "qapi/visitor.h"
>  #include "qemu/bitops.h"
>  #include "qemu/log.h"
> -#include "crypto/random.h"
> +#include "qemu/guest-random.h"
>  #include "trace.h"
>  
>  #define TO_REG(offset) ((offset) >> 2)
> @@ -157,14 +157,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>  
>  static uint32_t aspeed_scu_get_random(void)
>  {
> -    Error *err = NULL;
>      uint32_t num;
> -
> -    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> -
> +    qemu_guest_getrandom_nofail(&num, sizeof(num));
>      return num;
>  }
>  
> 

Re: [Qemu-arm] [Qemu-devel] [PATCH v3 16/23] aspeed/scu: 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: qemu-arm@nongnu.org
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  hw/misc/aspeed_scu.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index c8217740ef..ab1e18ed4b 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -16,7 +16,7 @@
>  #include "qapi/visitor.h"
>  #include "qemu/bitops.h"
>  #include "qemu/log.h"
> -#include "crypto/random.h"
> +#include "qemu/guest-random.h"
>  #include "trace.h"
>  
>  #define TO_REG(offset) ((offset) >> 2)
> @@ -157,14 +157,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>  
>  static uint32_t aspeed_scu_get_random(void)
>  {
> -    Error *err = NULL;
>      uint32_t num;
> -
> -    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> -
> +    qemu_guest_getrandom_nofail(&num, sizeof(num));
>      return num;
>  }
>  
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH v3 16/23] aspeed/scu: 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: qemu-arm@nongnu.org
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  hw/misc/aspeed_scu.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index c8217740ef..ab1e18ed4b 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -16,7 +16,7 @@
>  #include "qapi/visitor.h"
>  #include "qemu/bitops.h"
>  #include "qemu/log.h"
> -#include "crypto/random.h"
> +#include "qemu/guest-random.h"
>  #include "trace.h"
>  
>  #define TO_REG(offset) ((offset) >> 2)
> @@ -157,14 +157,8 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>  
>  static uint32_t aspeed_scu_get_random(void)
>  {
> -    Error *err = NULL;
>      uint32_t num;
> -
> -    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
> -        error_report_err(err);
> -        exit(1);
> -    }
> -
> +    qemu_guest_getrandom_nofail(&num, sizeof(num));
>      return num;
>  }
>  
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[Qemu-devel] [PATCH v3 17/23] hw/misc/nrf51_rng: Use qemu_guest_getrandom_nofail

[Richard Henderson]

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 error_abort directly.

Cc: qemu-arm@nongnu.org
Cc: Joel Stanley <joel@jms.id.au>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/misc/nrf51_rng.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/nrf51_rng.c b/hw/misc/nrf51_rng.c
index d188f044f4..3400e90a9b 100644
--- a/hw/misc/nrf51_rng.c
+++ b/hw/misc/nrf51_rng.c
@@ -14,7 +14,7 @@
 #include "qapi/error.h"
 #include "hw/arm/nrf51.h"
 #include "hw/misc/nrf51_rng.h"
-#include "crypto/random.h"
+#include "qemu/guest-random.h"
 
 static void update_irq(NRF51RNGState *s)
 {
@@ -145,7 +145,7 @@ static void nrf51_rng_timer_expire(void *opaque)
 {
     NRF51RNGState *s = NRF51_RNG(opaque);
 
-    qcrypto_random_bytes(&s->value, 1, &error_abort);
+    qemu_guest_getrandom_nofail(&s->value, 1);
 
     s->event_valrdy = 1;
     qemu_set_irq(s->eep_valrdy, 1);
-- 
2.17.2

Re: [Qemu-arm] [PATCH v3 17/23] hw/misc/nrf51_rng: Use qemu_guest_getrandom_nofail

[Joel Stanley]

On Fri, 15 Mar 2019 at 03:26, Richard Henderson
<richard.henderson@linaro.org> 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 error_abort directly.
>
> Cc: qemu-arm@nongnu.org
> Cc: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Joel Stanley <joel@jms.id.au>

> ---
>  hw/misc/nrf51_rng.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/misc/nrf51_rng.c b/hw/misc/nrf51_rng.c
> index d188f044f4..3400e90a9b 100644
> --- a/hw/misc/nrf51_rng.c
> +++ b/hw/misc/nrf51_rng.c
> @@ -14,7 +14,7 @@
>  #include "qapi/error.h"
>  #include "hw/arm/nrf51.h"
>  #include "hw/misc/nrf51_rng.h"
> -#include "crypto/random.h"
> +#include "qemu/guest-random.h"
>
>  static void update_irq(NRF51RNGState *s)
>  {
> @@ -145,7 +145,7 @@ static void nrf51_rng_timer_expire(void *opaque)
>  {
>      NRF51RNGState *s = NRF51_RNG(opaque);
>
> -    qcrypto_random_bytes(&s->value, 1, &error_abort);
> +    qemu_guest_getrandom_nofail(&s->value, 1);
>
>      s->event_valrdy = 1;
>      qemu_set_irq(s->eep_valrdy, 1);
> --
> 2.17.2
>

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

[Richard Henderson]

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: qemu-arm@nongnu.org
Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 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;
 }
 
-- 
2.17.2

Re: [Qemu-arm] [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: qemu-arm@nongnu.org
> Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  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é <philmd@redhat.com>

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: qemu-arm@nongnu.org
> Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  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é <philmd@redhat.com>

[Qemu-arm] [PATCH v3 19/23] hw/misc/exynos4210_rng: Use qemu_guest_getrandom

[Richard Henderson]

The random number is intended for use by the guest.  As such, we should
honor the -seed argument for reproducibility.

Cc: qemu-arm@nongnu.org
Cc: Igor Mitsyanko <i.mitsyanko@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/misc/exynos4210_rng.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/misc/exynos4210_rng.c b/hw/misc/exynos4210_rng.c
index 4ecbebd2d7..0e70ffb404 100644
--- a/hw/misc/exynos4210_rng.c
+++ b/hw/misc/exynos4210_rng.c
@@ -18,10 +18,10 @@
  */
 
 #include "qemu/osdep.h"
-#include "crypto/random.h"
 #include "hw/sysbus.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
+#include "qemu/guest-random.h"
 
 #define DEBUG_EXYNOS_RNG 0
 
@@ -109,7 +109,6 @@ static void exynos4210_rng_set_seed(Exynos4210RngState *s, unsigned int i,
 static void exynos4210_rng_run_engine(Exynos4210RngState *s)
 {
     Error *err = NULL;
-    int ret;
 
     /* Seed set? */
     if ((s->reg_status & EXYNOS4210_RNG_STATUS_SEED_SETTING_DONE) == 0) {
@@ -127,13 +126,11 @@ static void exynos4210_rng_run_engine(Exynos4210RngState *s)
     }
 
     /* Get randoms */
-    ret = qcrypto_random_bytes((uint8_t *)s->randr_value,
-                               sizeof(s->randr_value), &err);
-    if (!ret) {
+    if (qemu_guest_getrandom(s->randr_value, sizeof(s->randr_value), &err)) {
+        error_report_err(err);
+    } else {
         /* Notify that PRNG is ready */
         s->reg_status |= EXYNOS4210_RNG_STATUS_PRNG_DONE;
-    } else {
-        error_report_err(err);
     }
 
 out:
-- 
2.17.2

Re: [Qemu-arm] [Qemu-devel] [PATCH v3 19/23] hw/misc/exynos4210_rng: Use qemu_guest_getrandom

[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.
> 
> Cc: qemu-arm@nongnu.org
> Cc: Igor Mitsyanko <i.mitsyanko@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  hw/misc/exynos4210_rng.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/misc/exynos4210_rng.c b/hw/misc/exynos4210_rng.c
> index 4ecbebd2d7..0e70ffb404 100644
> --- a/hw/misc/exynos4210_rng.c
> +++ b/hw/misc/exynos4210_rng.c
> @@ -18,10 +18,10 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "crypto/random.h"
>  #include "hw/sysbus.h"
>  #include "qapi/error.h"
>  #include "qemu/log.h"
> +#include "qemu/guest-random.h"
>  
>  #define DEBUG_EXYNOS_RNG 0
>  
> @@ -109,7 +109,6 @@ static void exynos4210_rng_set_seed(Exynos4210RngState *s, unsigned int i,
>  static void exynos4210_rng_run_engine(Exynos4210RngState *s)
>  {
>      Error *err = NULL;
> -    int ret;
>  
>      /* Seed set? */
>      if ((s->reg_status & EXYNOS4210_RNG_STATUS_SEED_SETTING_DONE) == 0) {
> @@ -127,13 +126,11 @@ static void exynos4210_rng_run_engine(Exynos4210RngState *s)
>      }
>  
>      /* Get randoms */
> -    ret = qcrypto_random_bytes((uint8_t *)s->randr_value,
> -                               sizeof(s->randr_value), &err);
> -    if (!ret) {
> +    if (qemu_guest_getrandom(s->randr_value, sizeof(s->randr_value), &err)) {
> +        error_report_err(err);
> +    } else {
>          /* Notify that PRNG is ready */
>          s->reg_status |= EXYNOS4210_RNG_STATUS_PRNG_DONE;
> -    } else {
> -        error_report_err(err);
>      }
>  
>  out:
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH v3 19/23] hw/misc/exynos4210_rng: Use qemu_guest_getrandom

[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.
> 
> Cc: qemu-arm@nongnu.org
> Cc: Igor Mitsyanko <i.mitsyanko@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  hw/misc/exynos4210_rng.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/misc/exynos4210_rng.c b/hw/misc/exynos4210_rng.c
> index 4ecbebd2d7..0e70ffb404 100644
> --- a/hw/misc/exynos4210_rng.c
> +++ b/hw/misc/exynos4210_rng.c
> @@ -18,10 +18,10 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "crypto/random.h"
>  #include "hw/sysbus.h"
>  #include "qapi/error.h"
>  #include "qemu/log.h"
> +#include "qemu/guest-random.h"
>  
>  #define DEBUG_EXYNOS_RNG 0
>  
> @@ -109,7 +109,6 @@ static void exynos4210_rng_set_seed(Exynos4210RngState *s, unsigned int i,
>  static void exynos4210_rng_run_engine(Exynos4210RngState *s)
>  {
>      Error *err = NULL;
> -    int ret;
>  
>      /* Seed set? */
>      if ((s->reg_status & EXYNOS4210_RNG_STATUS_SEED_SETTING_DONE) == 0) {
> @@ -127,13 +126,11 @@ static void exynos4210_rng_run_engine(Exynos4210RngState *s)
>      }
>  
>      /* Get randoms */
> -    ret = qcrypto_random_bytes((uint8_t *)s->randr_value,
> -                               sizeof(s->randr_value), &err);
> -    if (!ret) {
> +    if (qemu_guest_getrandom(s->randr_value, sizeof(s->randr_value), &err)) {
> +        error_report_err(err);
> +    } else {
>          /* Notify that PRNG is ready */
>          s->reg_status |= EXYNOS4210_RNG_STATUS_PRNG_DONE;
> -    } else {
> -        error_report_err(err);
>      }
>  
>  out:
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[Qemu-arm] [PATCH v3 21/23] target/arm: Implement ARMv8.5-RNG

[Richard Henderson]

Cc: qemu-arm@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v3: Log errors with -d unimp, for lack of a better flag.
---
 target/arm/cpu.h    |  5 +++++
 target/arm/cpu64.c  |  1 +
 target/arm/helper.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index ae2381e222..6a078aa1a0 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3443,6 +3443,11 @@ static inline bool isar_feature_aa64_condm_5(const ARMISARegisters *id)
     return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TS) >= 2;
 }
 
+static inline bool isar_feature_aa64_rndr(const ARMISARegisters *id)
+{
+    return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, RNDR) != 0;
+}
+
 static inline bool isar_feature_aa64_jscvt(const ARMISARegisters *id)
 {
     return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, JSCVT) != 0;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 228906f267..835f73cceb 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -310,6 +310,7 @@ static void aarch64_max_initfn(Object *obj)
         t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
         t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 1);
         t = FIELD_DP64(t, ID_AA64ISAR0, TS, 2); /* v8.5-CondM */
+        t = FIELD_DP64(t, ID_AA64ISAR0, RNDR, 1);
         cpu->isar.id_aa64isar0 = t;
 
         t = cpu->isar.id_aa64isar1;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7b5d63f894..4a8d2d4481 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -20,6 +20,8 @@
 #include "fpu/softfloat.h"
 #include "qemu/range.h"
 #include "qapi/qapi-commands-target.h"
+#include "qapi/error.h"
+#include "qemu/guest-random.h"
 
 #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
 
@@ -5717,6 +5719,45 @@ static const ARMCPRegInfo pauth_reginfo[] = {
       .fieldoffset = offsetof(CPUARMState, keys.apib.hi) },
     REGINFO_SENTINEL
 };
+
+static uint64_t rndr_readfn(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    Error *err = NULL;
+    uint64_t ret;
+
+    /* Success sets NZCV = 0000.  */
+    env->NF = env->CF = env->VF = 0, env->ZF = 1;
+
+    if (qemu_guest_getrandom(&ret, sizeof(ret), &err) < 0) {
+        /*
+         * ??? Failed, for unknown reasons in the crypto subsystem.
+         * The best we can do is log the reason and return the
+         * timed-out indication to the guest.  There is no reason
+         * we know to expect this failure to be transitory, so the
+         * guest may well hang retrying the operation.
+         */
+        qemu_log_mask(LOG_UNIMP, "%s: Crypto failure: %s",
+                      ri->name, error_get_pretty(err));
+        error_free(err);
+
+        env->ZF = 0; /* NZCF = 0100 */
+        return 0;
+    }
+    return ret;
+}
+
+/* We do not support re-seeding, so the two registers operate the same.  */
+static const ARMCPRegInfo rndr_reginfo[] = {
+    { .name = "RNDR", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
+      .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 0,
+      .access = PL0_R, .readfn = rndr_readfn },
+    { .name = "RNDRRS", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END,
+      .opc0 = 3, .opc1 = 3, .crn = 2, .crm = 4, .opc2 = 1,
+      .access = PL0_R, .readfn = rndr_readfn },
+    REGINFO_SENTINEL
+};
 #endif
 
 static CPAccessResult access_predinv(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -6661,6 +6702,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
     if (cpu_isar_feature(aa64_pauth, cpu)) {
         define_arm_cp_regs(cpu, pauth_reginfo);
     }
+    if (cpu_isar_feature(aa64_rndr, cpu)) {
+        define_arm_cp_regs(cpu, rndr_reginfo);
+    }
 #endif
 
     /*
-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 04/23] crypto: Use O_CLOEXEC in qcrypto_random_init

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> Avoids leaking the /dev/urandom fd into any child processes.
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  crypto/random-platform.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/crypto/random-platform.c b/crypto/random-platform.c
> index 260b64564d..6df40744c7 100644
> --- a/crypto/random-platform.c
> +++ b/crypto/random-platform.c
> @@ -42,9 +42,9 @@ int qcrypto_random_init(Error **errp)
>  #else
>      /* TBD perhaps also add support for BSD getentropy / Linux
>       * getrandom syscalls directly */
> -    fd = open("/dev/urandom", O_RDONLY);
> +    fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC);
>      if (fd == -1 && errno == ENOENT) {
> -        fd = open("/dev/random", O_RDONLY);
> +        fd = open("/dev/random", O_RDONLY | O_CLOEXEC);
>      }
>  
>      if (fd < 0) {
> 

Re: [Qemu-devel] [PATCH v3 07/23] ui/vnc: Split out authentication_failure

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> There were 3 copies of this code, one of which used the wrong
> data size for the failure indicator.

Which, as the comment said, has never been called.

Good cleanup.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  ui/vnc.c | 37 +++++++++++++++----------------------
>  1 file changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 1871422e1d..785edf3af1 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2535,6 +2535,18 @@ void start_client_init(VncState *vs)
>      vnc_read_when(vs, protocol_client_init, 1);
>  }
>  
> +static void authentication_failed(VncState *vs)
> +{
> +    vnc_write_u32(vs, 1); /* Reject auth */
> +    if (vs->minor >= 8) {
> +        static const char err[] = "Authentication failed";
> +        vnc_write_u32(vs, sizeof(err));
> +        vnc_write(vs, err, sizeof(err));
> +    }
> +    vnc_flush(vs);
> +    vnc_client_error(vs);
> +}
> +
>  static void make_challenge(VncState *vs)
>  {
>      int i;
> @@ -2609,14 +2621,7 @@ static int protocol_client_auth_vnc(VncState *vs, uint8_t *data, size_t len)
>      return 0;
>  
>  reject:
> -    vnc_write_u32(vs, 1); /* Reject auth */
> -    if (vs->minor >= 8) {
> -        static const char err[] = "Authentication failed";
> -        vnc_write_u32(vs, sizeof(err));
> -        vnc_write(vs, err, sizeof(err));
> -    }
> -    vnc_flush(vs);
> -    vnc_client_error(vs);
> +    authentication_failed(vs);
>      qcrypto_cipher_free(cipher);
>      return 0;
>  }
> @@ -2638,13 +2643,7 @@ static int protocol_client_auth(VncState *vs, uint8_t *data, size_t len)
>       * must pick the one we sent. Verify this */
>      if (data[0] != vs->auth) { /* Reject auth */
>         trace_vnc_auth_reject(vs, vs->auth, (int)data[0]);
> -       vnc_write_u32(vs, 1);
> -       if (vs->minor >= 8) {
> -           static const char err[] = "Authentication failed";
> -           vnc_write_u32(vs, sizeof(err));
> -           vnc_write(vs, err, sizeof(err));
> -       }
> -       vnc_client_error(vs);
> +       authentication_failed(vs);
>      } else { /* Accept requested auth */
>         trace_vnc_auth_start(vs, vs->auth);
>         switch (vs->auth) {
> @@ -2673,13 +2672,7 @@ static int protocol_client_auth(VncState *vs, uint8_t *data, size_t len)
>  
>         default: /* Should not be possible, but just in case */
>             trace_vnc_auth_fail(vs, vs->auth, "Unhandled auth method", "");
> -           vnc_write_u8(vs, 1);
> -           if (vs->minor >= 8) {
> -               static const char err[] = "Authentication failed";
> -               vnc_write_u32(vs, sizeof(err));
> -               vnc_write(vs, err, sizeof(err));
> -           }
> -           vnc_client_error(vs);
> +           authentication_failed(vs);
>         }
>      }
>      return 0;
> 

Re: [Qemu-devel] [PATCH v3 08/23] ui/vnc: Use gcrypto_random_bytes for start_auth_vnc

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> Use a better interface for random numbers than rand().
> Fail gracefully if for some reason we cannot use the crypto system.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
> v2: Use qcrypto_random_bytes, not qemu_getrandom, as there is
>     no need for deterministic results for this interface.
> v3: Fail gracefully in the event qcrypto_random_bytes fails.
> ---
>  ui/vnc.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 785edf3af1..d83f4a6ff9 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -43,6 +43,7 @@
>  #include "crypto/hash.h"
>  #include "crypto/tlscredsanon.h"
>  #include "crypto/tlscredsx509.h"
> +#include "crypto/random.h"
>  #include "qom/object_interfaces.h"
>  #include "qemu/cutils.h"
>  #include "io/dns-resolver.h"
> @@ -2547,16 +2548,6 @@ static void authentication_failed(VncState *vs)
>      vnc_client_error(vs);
>  }
>  
> -static void make_challenge(VncState *vs)
> -{
> -    int i;
> -
> -    srand(time(NULL)+getpid()+getpid()*987654+rand());
> -
> -    for (i = 0 ; i < sizeof(vs->challenge) ; i++)
> -        vs->challenge[i] = (int) (256.0*rand()/(RAND_MAX+1.0));
> -}
> -
>  static int protocol_client_auth_vnc(VncState *vs, uint8_t *data, size_t len)
>  {
>      unsigned char response[VNC_AUTH_CHALLENGE_SIZE];
> @@ -2628,7 +2619,16 @@ reject:
>  
>  void start_auth_vnc(VncState *vs)
>  {
> -    make_challenge(vs);
> +    Error *err = NULL;
> +
> +    if (qcrypto_random_bytes(vs->challenge, sizeof(vs->challenge), &err)) {
> +        trace_vnc_auth_fail(vs, vs->auth, "cannot get random bytes",
> +                            error_get_pretty(err));
> +        error_free(err);
> +        authentication_failed(vs);
> +        return;
> +    }
> +
>      /* Send client a 'random' challenge */
>      vnc_write(vs, vs->challenge, sizeof(vs->challenge));
>      vnc_flush(vs);
> 

Re: [Qemu-devel] [PATCH v3 09/23] util: Add qemu_guest_getrandom and associated routines

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> This routine is intended to produce high-quality random numbers to the
> guest.  Normally, such numbers are crypto quality from the host, but a
> command-line option can force the use of a fully deterministic sequence
> for use while debugging.
> 
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/qemu/guest-random.h | 68 +++++++++++++++++++++++++++
>  util/guest-random.c         | 93 +++++++++++++++++++++++++++++++++++++
>  util/Makefile.objs          |  1 +
>  3 files changed, 162 insertions(+)
>  create mode 100644 include/qemu/guest-random.h
>  create mode 100644 util/guest-random.c
> 
> diff --git a/include/qemu/guest-random.h b/include/qemu/guest-random.h
> new file mode 100644
> index 0000000000..09ff9c2236
> --- /dev/null
> +++ b/include/qemu/guest-random.h
> @@ -0,0 +1,68 @@
> +/*
> + * QEMU guest-visible random functions
> + *
> + * Copyright 2019 Linaro, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#ifndef QEMU_GUEST_RANDOM_H
> +#define QEMU_GUEST_RANDOM_H
> +
> +/**
> + * qemu_guest_random_seed_main(const char *optarg, Error **errp)
> + * @optarg: a non-NULL pointer to a C string
> + * @errp: an error indicator
> + *
> + * The @optarg value is that which accompanies the -seed argument.
> + * This forces qemu_guest_getrandom into deterministic mode.
> + *
> + * Returns 0 on success, < 0 on failure while setting *errp.
> + */
> +int qemu_guest_random_seed_main(const char *optarg, Error **errp);
> +
> +/**
> + * qemu_guest_random_seed_thread_part1(void)
> + *
> + * If qemu_getrandom is in deterministic mode, returns an
> + * independent seed for the new thread.  Otherwise returns 0.
> + */
> +uint64_t qemu_guest_random_seed_thread_part1(void);
> +
> +/**
> + * qemu_guest_random_seed_thread_part2(uint64_t seed)
> + * @seed: a value for the new thread.
> + *
> + * If qemu_guest_getrandom is in deterministic mode, this stores an
> + * independent seed for the new thread.  Otherwise a no-op.
> + */
> +void qemu_guest_random_seed_thread_part2(uint64_t seed);
> +
> +/**
> + * qemu_guest_getrandom(void *buf, size_t len, Error **errp)
> + * @buf: a buffer of bytes to be written
> + * @len: the number of bytes in @buf
> + * @errp: an error indicator
> + *
> + * Fills len bytes in buf with random data.  This should only be used
> + * for data presented to the guest.  Host-side crypto services should
> + * use qcrypto_random_bytes.
> + *
> + * Returns 0 on success, < 0 on failure while setting *errp.
> + */
> +int qemu_guest_getrandom(void *buf, size_t len, Error **errp);
> +
> +/**
> + * qemu_guest_getrandom_nofail(void *buf, size_t len)
> + * @buf: a buffer of bytes to be written
> + * @len: the number of bytes in @buf

API documentation appreciated, thanks!

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> + *
> + * Like qemu_guest_getrandom, but will assert for failure.
> + * Use this when there is no reasonable recovery.
> + */
> +void qemu_guest_getrandom_nofail(void *buf, size_t len);
> +
> +#endif /* QEMU_GUEST_RANDOM_H */
> diff --git a/util/guest-random.c b/util/guest-random.c
> new file mode 100644
> index 0000000000..e8124a3cad
> --- /dev/null
> +++ b/util/guest-random.c
> @@ -0,0 +1,93 @@
> +/*
> + * QEMU guest-visible random functions
> + *
> + * Copyright 2019 Linaro, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/cutils.h"
> +#include "qapi/error.h"
> +#include "qemu/guest-random.h"
> +#include "crypto/random.h"
> +
> +
> +static __thread GRand *thread_rand;
> +static bool deterministic;
> +
> +
> +static int glib_random_bytes(void *buf, size_t len)
> +{
> +    GRand *rand = thread_rand;
> +    size_t i;
> +    uint32_t x;
> +
> +    if (unlikely(rand == NULL)) {
> +        /* Thread not initialized for a cpu, or main w/o -seed.  */
> +        thread_rand = rand = g_rand_new();
> +    }
> +
> +    for (i = 0; i + 4 <= len; i += 4) {
> +        x = g_rand_int(rand);
> +        __builtin_memcpy(buf + i, &x, 4);
> +    }
> +    if (i < len) {
> +        x = g_rand_int(rand);
> +        __builtin_memcpy(buf + i, &x, i - len);
> +    }
> +    return 0;
> +}
> +
> +int qemu_guest_getrandom(void *buf, size_t len, Error **errp)
> +{
> +    if (unlikely(deterministic)) {
> +        /* Deterministic implementation using Glib's Mersenne Twister.  */
> +        return glib_random_bytes(buf, len);
> +    } else {
> +        /* Non-deterministic implementation using crypto routines.  */
> +        return qcrypto_random_bytes(buf, len, errp);
> +    }
> +}
> +
> +void qemu_guest_getrandom_nofail(void *buf, size_t len)
> +{
> +    qemu_guest_getrandom(buf, len, &error_fatal);
> +}
> +
> +uint64_t qemu_guest_random_seed_thread_part1(void)
> +{
> +    if (deterministic) {
> +        uint64_t ret;
> +        glib_random_bytes(&ret, sizeof(ret));
> +        return ret;
> +    }
> +    return 0;
> +}
> +
> +void qemu_guest_random_seed_thread_part2(uint64_t seed)
> +{
> +    g_assert(thread_rand == NULL);
> +    if (deterministic) {
> +        thread_rand =
> +            g_rand_new_with_seed_array((const guint32 *)&seed,
> +                                       sizeof(seed) / sizeof(guint32));
> +    }
> +}
> +
> +int qemu_guest_random_seed_main(const char *optarg, Error **errp)
> +{
> +    unsigned long long seed;
> +    if (parse_uint_full(optarg, &seed, 0)) {
> +        error_setg(errp, "Invalid seed number: %s", optarg);
> +        return -1;
> +    } else {
> +        deterministic = true;
> +        qemu_guest_random_seed_thread_part2(seed);
> +        return 0;
> +    }
> +}
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 835fcd69e2..4d4db653cc 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -53,5 +53,6 @@ util-obj-y += iova-tree.o
>  util-obj-$(CONFIG_INOTIFY1) += filemonitor-inotify.o
>  util-obj-$(CONFIG_LINUX) += vfio-helpers.o
>  util-obj-$(CONFIG_OPENGL) += drm.o
> +util-obj-y += guest-random.o
>  
>  stub-obj-y += filemonitor-stub.o
> 

Re: [Qemu-devel] [PATCH v3 10/23] cpus: Initialize pseudo-random seeds for all guest cpus

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> When the -seed option is given, call qemu_guest_random_seed_main,
> putting the subsystem into deterministic mode.  Pass derived seeds
> to each cpu created; which is a no-op unless the subsystem is in
> deterministic mode.
> 
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/qom/cpu.h |  1 +
>  cpus.c            |  9 +++++++++
>  vl.c              |  4 ++++
>  qemu-options.hx   | 10 ++++++++++
>  4 files changed, 24 insertions(+)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 1d6099e5d4..343cc6d51e 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -372,6 +372,7 @@ struct CPUState {
>      int singlestep_enabled;
>      int64_t icount_budget;
>      int64_t icount_extra;
> +    uint64_t random_seed;
>      sigjmp_buf jmp_env;
>  
>      QemuMutex work_mutex;
> diff --git a/cpus.c b/cpus.c
> index e83f72b48b..6d88f6bfcd 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -49,6 +49,7 @@
>  #include "qemu/option.h"
>  #include "qemu/bitmap.h"
>  #include "qemu/seqlock.h"
> +#include "qemu/guest-random.h"
>  #include "tcg.h"
>  #include "hw/nmi.h"
>  #include "sysemu/replay.h"
> @@ -1275,6 +1276,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>      /* signal CPU creation */
>      cpu->created = true;
>      qemu_cond_signal(&qemu_cpu_cond);
> +    qemu_guest_random_seed_thread_part2(cpu->random_seed);
>  
>      do {
>          if (cpu_can_run(cpu)) {
> @@ -1318,6 +1320,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
>      /* signal CPU creation */
>      cpu->created = true;
>      qemu_cond_signal(&qemu_cpu_cond);
> +    qemu_guest_random_seed_thread_part2(cpu->random_seed);
>  
>      do {
>          qemu_mutex_unlock_iothread();
> @@ -1477,6 +1480,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
>      cpu->created = true;
>      cpu->can_do_io = 1;
>      qemu_cond_signal(&qemu_cpu_cond);
> +    qemu_guest_random_seed_thread_part2(cpu->random_seed);
>  
>      /* wait for initial kick-off after machine start */
>      while (first_cpu->stopped) {
> @@ -1591,6 +1595,7 @@ static void *qemu_hax_cpu_thread_fn(void *arg)
>  
>      hax_init_vcpu(cpu);
>      qemu_cond_signal(&qemu_cpu_cond);
> +    qemu_guest_random_seed_thread_part2(cpu->random_seed);
>  
>      do {
>          if (cpu_can_run(cpu)) {
> @@ -1630,6 +1635,7 @@ static void *qemu_hvf_cpu_thread_fn(void *arg)
>      /* signal CPU creation */
>      cpu->created = true;
>      qemu_cond_signal(&qemu_cpu_cond);
> +    qemu_guest_random_seed_thread_part2(cpu->random_seed);
>  
>      do {
>          if (cpu_can_run(cpu)) {
> @@ -1670,6 +1676,7 @@ static void *qemu_whpx_cpu_thread_fn(void *arg)
>      /* signal CPU creation */
>      cpu->created = true;
>      qemu_cond_signal(&qemu_cpu_cond);
> +    qemu_guest_random_seed_thread_part2(cpu->random_seed);
>  
>      do {
>          if (cpu_can_run(cpu)) {
> @@ -1723,6 +1730,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>      cpu->can_do_io = 1;
>      current_cpu = cpu;
>      qemu_cond_signal(&qemu_cpu_cond);
> +    qemu_guest_random_seed_thread_part2(cpu->random_seed);
>  
>      /* process any pending work */
>      cpu->exit_request = 1;
> @@ -2070,6 +2078,7 @@ void qemu_init_vcpu(CPUState *cpu)
>      cpu->nr_cores = smp_cores;
>      cpu->nr_threads = smp_threads;
>      cpu->stopped = true;
> +    cpu->random_seed = qemu_guest_random_seed_thread_part1();
>  
>      if (!cpu->as) {
>          /* If the target cpu hasn't set up any address spaces itself,
> diff --git a/vl.c b/vl.c
> index c1d5484e12..d84f883c2c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -128,6 +128,7 @@ int main(int argc, char **argv)
>  #include "qapi/qapi-commands-ui.h"
>  #include "qapi/qmp/qerror.h"
>  #include "sysemu/iothread.h"
> +#include "qemu/guest-random.h"
>  
>  #define MAX_VIRTIO_CONSOLES 1
>  
> @@ -3330,6 +3331,9 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_DFILTER:
>                  qemu_set_dfilter_ranges(optarg, &error_fatal);
>                  break;
> +            case QEMU_OPTION_seed:
> +                qemu_guest_random_seed_main(optarg, &error_fatal);
> +                break;
>              case QEMU_OPTION_s:
>                  add_device_config(DEV_GDB, "tcp::" DEFAULT_GDBSTUB_PORT);
>                  break;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 08749a3391..b8da998668 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3601,6 +3601,16 @@ the 0x200 sized block starting at 0xffffffc000080000 and another 0x1000 sized
>  block starting at 0xffffffc00005f000.
>  ETEXI
>  
> +DEF("seed", HAS_ARG, QEMU_OPTION_seed, \
> +    "-seed number       seed the pseudo-random number generator\n",
> +    QEMU_ARCH_ALL)
> +STEXI
> +@item -seed @var{number}
> +@findex -seed
> +Force the guest to use a deterministic pseudo-random number generator, seeded
> +with @var{number}.  This does not affect crypto routines within the host.
> +ETEXI

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +
>  DEF("L", HAS_ARG, QEMU_OPTION_L, \
>      "-L path         set the directory for the BIOS, VGA BIOS and keymaps\n",
>      QEMU_ARCH_ALL)
> 

Re: [Qemu-devel] [PATCH v3 11/23] linux-user: Initialize pseudo-random seeds for all guest cpus

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> When the -seed option is given, call qemu_guest_random_seed_main,
> putting the subsystem into deterministic mode.  Pass derived seeds
> to each cpu created during clone; which is a no-op unless the
> subsystem is in deterministic mode.
> 
> Cc: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/main.c    | 21 ++++++++++-----------
>  linux-user/syscall.c |  3 +++
>  2 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a0aba9cb1e..cf7095bdaf 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -33,6 +33,7 @@
>  #include "tcg.h"
>  #include "qemu/timer.h"
>  #include "qemu/envlist.h"
> +#include "qemu/guest-random.h"
>  #include "elf.h"
>  #include "trace/control.h"
>  #include "target_elf.h"
> @@ -47,6 +48,7 @@ static int gdbstub_port;
>  static envlist_t *envlist;
>  static const char *cpu_model;
>  static const char *cpu_type;
> +static const char *seed_optarg;
>  unsigned long mmap_min_addr;
>  unsigned long guest_base;
>  int have_guest_base;
> @@ -289,15 +291,9 @@ static void handle_arg_pagesize(const char *arg)
>      }
>  }
>  
> -static void handle_arg_randseed(const char *arg)
> +static void handle_arg_seed(const char *arg)
>  {
> -    unsigned long long seed;
> -
> -    if (parse_uint_full(arg, &seed, 0) != 0 || seed > UINT_MAX) {
> -        fprintf(stderr, "Invalid seed number: %s\n", arg);
> -        exit(EXIT_FAILURE);
> -    }
> -    srand(seed);
> +    seed_optarg = arg;
>  }
>  
>  static void handle_arg_gdb(const char *arg)
> @@ -432,7 +428,7 @@ static const struct qemu_argument arg_table[] = {
>       "",           "run in singlestep mode"},
>      {"strace",     "QEMU_STRACE",      false, handle_arg_strace,
>       "",           "log system calls"},
> -    {"seed",       "QEMU_RAND_SEED",   true,  handle_arg_randseed,
> +    {"seed",       "QEMU_RAND_SEED",   true,  handle_arg_seed,
>       "",           "Seed for pseudo-random number generator"},
>      {"trace",      "QEMU_TRACE",       true,  handle_arg_trace,
>       "",           "[[enable=]<pattern>][,events=<file>][,file=<file>]"},
> @@ -687,8 +683,11 @@ int main(int argc, char **argv, char **envp)
>          do_strace = 1;
>      }
>  
> -    if (getenv("QEMU_RAND_SEED")) {
> -        handle_arg_randseed(getenv("QEMU_RAND_SEED"));
> +    if (seed_optarg == NULL) {
> +        seed_optarg = getenv("QEMU_RAND_SEED");
> +    }
> +    if (seed_optarg != NULL) {
> +        qemu_guest_random_seed_main(seed_optarg, &error_fatal);
>      }
>  
>      target_environ = envlist_to_environ(envlist, NULL);
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 208fd1813d..8f7125cd67 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -110,6 +110,7 @@
>  #include "uname.h"
>  
>  #include "qemu.h"
> +#include "qemu/guest-random.h"
>  #include "fd-trans.h"
>  
>  #ifndef CLONE_IO
> @@ -5448,6 +5449,7 @@ static void *clone_func(void *arg)
>          put_user_u32(info->tid, info->child_tidptr);
>      if (info->parent_tidptr)
>          put_user_u32(info->tid, info->parent_tidptr);
> +    qemu_guest_random_seed_thread_part2(cpu->random_seed);
>      /* Enable signals.  */
>      sigprocmask(SIG_SETMASK, &info->sigmask, NULL);
>      /* Signal to the parent that we're ready.  */
> @@ -5534,6 +5536,7 @@ static int do_fork(CPUArchState *env, unsigned int flags, abi_ulong newsp,
>             initializing, so temporarily block all signals.  */
>          sigfillset(&sigmask);
>          sigprocmask(SIG_BLOCK, &sigmask, &info.sigmask);
> +        cpu->random_seed = qemu_guest_random_seed_thread_part1();
>  
>          /* If this is our first additional thread, we need to ensure we
>           * generate code for parallel execution and flush old translations.
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH v3 12/23] linux-user: Call qcrypto_init if not using -seed

[Philippe Mathieu-Daudé]

Hi Richard,

On 3/15/19 4:26 AM, Richard Henderson wrote:
> Cc: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/main.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index cf7095bdaf..8478306eef 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -38,6 +38,7 @@
>  #include "trace/control.h"
>  #include "target_elf.h"
>  #include "cpu_loop-common.h"
> +#include "crypto/init.h"
>  
>  char *exec_path;
>  
> @@ -686,8 +687,18 @@ int main(int argc, char **argv, char **envp)
>      if (seed_optarg == NULL) {
>          seed_optarg = getenv("QEMU_RAND_SEED");
>      }
> -    if (seed_optarg != NULL) {
> -        qemu_guest_random_seed_main(seed_optarg, &error_fatal);
> +    {

Since 7be41675f7c we use gnu99 C, so this extra block indentation can be
removed.

> +        Error *err = NULL;
> +        if (seed_optarg != NULL) {
> +            qemu_guest_random_seed_main(seed_optarg, &err);
> +        } else {
> +            /* ??? Assumes qcrypto is only used by qemu_guest_getrandom.  */
> +            qcrypto_init(&err);
> +        }
> +        if (err) {
> +            error_reportf_err(err, "cannot initialize crypto: ");
> +            exit(1);
> +        }
>      }
>  
>      target_environ = envlist_to_environ(envlist, NULL);
> 

Re: [Qemu-devel] [PATCH v3 12/23] linux-user: Call qcrypto_init if not using -seed

[Philippe Mathieu-Daudé]

Hi Richard,

On 3/15/19 4:26 AM, Richard Henderson wrote:
> Cc: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/main.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index cf7095bdaf..8478306eef 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -38,6 +38,7 @@
>  #include "trace/control.h"
>  #include "target_elf.h"
>  #include "cpu_loop-common.h"
> +#include "crypto/init.h"
>  
>  char *exec_path;
>  
> @@ -686,8 +687,18 @@ int main(int argc, char **argv, char **envp)
>      if (seed_optarg == NULL) {
>          seed_optarg = getenv("QEMU_RAND_SEED");
>      }
> -    if (seed_optarg != NULL) {
> -        qemu_guest_random_seed_main(seed_optarg, &error_fatal);
> +    {

Since 7be41675f7c we use gnu99 C, so this extra block indentation can be
removed.

> +        Error *err = NULL;
> +        if (seed_optarg != NULL) {
> +            qemu_guest_random_seed_main(seed_optarg, &err);
> +        } else {
> +            /* ??? Assumes qcrypto is only used by qemu_guest_getrandom.  */
> +            qcrypto_init(&err);
> +        }
> +        if (err) {
> +            error_reportf_err(err, "cannot initialize crypto: ");
> +            exit(1);
> +        }
>      }
>  
>      target_environ = envlist_to_environ(envlist, NULL);
> 

Re: [Qemu-devel] [PATCH v3 12/23] linux-user: Call qcrypto_init if not using -seed

[Richard Henderson]

On 4/10/19 11:49 PM, Philippe Mathieu-Daudé wrote:
>> -    if (seed_optarg != NULL) {
>> -        qemu_guest_random_seed_main(seed_optarg, &error_fatal);
>> +    {
> Since 7be41675f7c we use gnu99 C, so this extra block indentation can be
> removed.
> 
>> +        Error *err = NULL;
>> +        if (seed_optarg != NULL) {
>> +            qemu_guest_random_seed_main(seed_optarg, &err);
>> +        } else {
>> +            /* ??? Assumes qcrypto is only used by qemu_guest_getrandom.  */
>> +            qcrypto_init(&err);
>> +        }
>> +        if (err) {
>> +            error_reportf_err(err, "cannot initialize crypto: ");
>> +            exit(1);
>> +        }
>>      }

I could, but it also limits the scope, which is of more importance to variables
who have their address taken.  It means that their storage could (in theory) be
shared with objects not overlapping in scope.


r~

Re: [Qemu-devel] [PATCH v3 12/23] linux-user: Call qcrypto_init if not using -seed

[Richard Henderson]

On 4/10/19 11:49 PM, Philippe Mathieu-Daudé wrote:
>> -    if (seed_optarg != NULL) {
>> -        qemu_guest_random_seed_main(seed_optarg, &error_fatal);
>> +    {
> Since 7be41675f7c we use gnu99 C, so this extra block indentation can be
> removed.
> 
>> +        Error *err = NULL;
>> +        if (seed_optarg != NULL) {
>> +            qemu_guest_random_seed_main(seed_optarg, &err);
>> +        } else {
>> +            /* ??? Assumes qcrypto is only used by qemu_guest_getrandom.  */
>> +            qcrypto_init(&err);
>> +        }
>> +        if (err) {
>> +            error_reportf_err(err, "cannot initialize crypto: ");
>> +            exit(1);
>> +        }
>>      }

I could, but it also limits the scope, which is of more importance to variables
who have their address taken.  It means that their storage could (in theory) be
shared with objects not overlapping in scope.


r~

Re: [Qemu-devel] [PATCH v3 12/23] linux-user: Call qcrypto_init if not using -seed

[Philippe Mathieu-Daudé]

On 4/13/19 8:44 AM, Richard Henderson wrote:
> On 4/10/19 11:49 PM, Philippe Mathieu-Daudé wrote:
>>> -    if (seed_optarg != NULL) {
>>> -        qemu_guest_random_seed_main(seed_optarg, &error_fatal);
>>> +    {
>> Since 7be41675f7c we use gnu99 C, so this extra block indentation can be
>> removed.
>>
>>> +        Error *err = NULL;
>>> +        if (seed_optarg != NULL) {
>>> +            qemu_guest_random_seed_main(seed_optarg, &err);
>>> +        } else {
>>> +            /* ??? Assumes qcrypto is only used by qemu_guest_getrandom.  */
>>> +            qcrypto_init(&err);
>>> +        }
>>> +        if (err) {
>>> +            error_reportf_err(err, "cannot initialize crypto: ");
>>> +            exit(1);
>>> +        }
>>>      }
> 
> I could, but it also limits the scope, which is of more importance to variables
> who have their address taken.  It means that their storage could (in theory) be
> shared with objects not overlapping in scope.

Fine then.

I think your '???' comment is appropriate but I'd rather let Daniel
opinate. Except that comment, for the rest:

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH v3 12/23] linux-user: Call qcrypto_init if not using -seed

[Philippe Mathieu-Daudé]

On 4/13/19 8:44 AM, Richard Henderson wrote:
> On 4/10/19 11:49 PM, Philippe Mathieu-Daudé wrote:
>>> -    if (seed_optarg != NULL) {
>>> -        qemu_guest_random_seed_main(seed_optarg, &error_fatal);
>>> +    {
>> Since 7be41675f7c we use gnu99 C, so this extra block indentation can be
>> removed.
>>
>>> +        Error *err = NULL;
>>> +        if (seed_optarg != NULL) {
>>> +            qemu_guest_random_seed_main(seed_optarg, &err);
>>> +        } else {
>>> +            /* ??? Assumes qcrypto is only used by qemu_guest_getrandom.  */
>>> +            qcrypto_init(&err);
>>> +        }
>>> +        if (err) {
>>> +            error_reportf_err(err, "cannot initialize crypto: ");
>>> +            exit(1);
>>> +        }
>>>      }
> 
> I could, but it also limits the scope, which is of more importance to variables
> who have their address taken.  It means that their storage could (in theory) be
> shared with objects not overlapping in scope.

Fine then.

I think your '???' comment is appropriate but I'd rather let Daniel
opinate. Except that comment, for the rest:

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH v3 12/23] linux-user: Call qcrypto_init if not using -seed

[Daniel P. Berrangé]

On Mon, Apr 15, 2019 at 12:23:18PM +0200, Philippe Mathieu-Daudé wrote:
> On 4/13/19 8:44 AM, Richard Henderson wrote:
> > On 4/10/19 11:49 PM, Philippe Mathieu-Daudé wrote:
> >>> -    if (seed_optarg != NULL) {
> >>> -        qemu_guest_random_seed_main(seed_optarg, &error_fatal);
> >>> +    {
> >> Since 7be41675f7c we use gnu99 C, so this extra block indentation can be
> >> removed.
> >>
> >>> +        Error *err = NULL;
> >>> +        if (seed_optarg != NULL) {
> >>> +            qemu_guest_random_seed_main(seed_optarg, &err);
> >>> +        } else {
> >>> +            /* ??? Assumes qcrypto is only used by qemu_guest_getrandom.  */
> >>> +            qcrypto_init(&err);
> >>> +        }
> >>> +        if (err) {
> >>> +            error_reportf_err(err, "cannot initialize crypto: ");
> >>> +            exit(1);
> >>> +        }
> >>>      }
> > 
> > I could, but it also limits the scope, which is of more importance to variables
> > who have their address taken.  It means that their storage could (in theory) be
> > shared with objects not overlapping in scope.
> 
> Fine then.
> 
> I think your '???' comment is appropriate but I'd rather let Daniel
> opinate. Except that comment, for the rest:

In linux-user context, afaik, the random APIs are the only stuff that
will be used, none of the hash or cipher stuff is needed.

> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v3 12/23] linux-user: Call qcrypto_init if not using -seed

[Daniel P. Berrangé]

On Mon, Apr 15, 2019 at 12:23:18PM +0200, Philippe Mathieu-Daudé wrote:
> On 4/13/19 8:44 AM, Richard Henderson wrote:
> > On 4/10/19 11:49 PM, Philippe Mathieu-Daudé wrote:
> >>> -    if (seed_optarg != NULL) {
> >>> -        qemu_guest_random_seed_main(seed_optarg, &error_fatal);
> >>> +    {
> >> Since 7be41675f7c we use gnu99 C, so this extra block indentation can be
> >> removed.
> >>
> >>> +        Error *err = NULL;
> >>> +        if (seed_optarg != NULL) {
> >>> +            qemu_guest_random_seed_main(seed_optarg, &err);
> >>> +        } else {
> >>> +            /* ??? Assumes qcrypto is only used by qemu_guest_getrandom.  */
> >>> +            qcrypto_init(&err);
> >>> +        }
> >>> +        if (err) {
> >>> +            error_reportf_err(err, "cannot initialize crypto: ");
> >>> +            exit(1);
> >>> +        }
> >>>      }
> > 
> > I could, but it also limits the scope, which is of more importance to variables
> > who have their address taken.  It means that their storage could (in theory) be
> > shared with objects not overlapping in scope.
> 
> Fine then.
> 
> I think your '???' comment is appropriate but I'd rather let Daniel
> opinate. Except that comment, for the rest:

In linux-user context, afaik, the random APIs are the only stuff that
will be used, none of the hash or cipher stuff is needed.

> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v3 14/23] linux-user/aarch64: Use qemu_guest_getrandom for PAUTH keys

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> Use a better interface for random numbers than rand() * 3.
> 
> Cc: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/aarch64/target_syscall.h |  2 --
>  linux-user/aarch64/cpu_loop.c       | 29 ++++++---------------------
>  linux-user/syscall.c                | 31 ++++++++++++++++++++++++-----
>  3 files changed, 32 insertions(+), 30 deletions(-)
> 
> diff --git a/linux-user/aarch64/target_syscall.h b/linux-user/aarch64/target_syscall.h
> index b595e5da82..995e475c73 100644
> --- a/linux-user/aarch64/target_syscall.h
> +++ b/linux-user/aarch64/target_syscall.h
> @@ -29,6 +29,4 @@ struct target_pt_regs {
>  # define TARGET_PR_PAC_APDBKEY   (1 << 3)
>  # define TARGET_PR_PAC_APGAKEY   (1 << 4)
>  
> -void arm_init_pauth_key(ARMPACKey *key);
> -
>  #endif /* AARCH64_TARGET_SYSCALL_H */
> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
> index d75fd9d3e2..cedad39ca0 100644
> --- a/linux-user/aarch64/cpu_loop.c
> +++ b/linux-user/aarch64/cpu_loop.c
> @@ -20,6 +20,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu.h"
>  #include "cpu_loop-common.h"
> +#include "qemu/guest-random.h"
>  
>  #define get_user_code_u32(x, gaddr, env)                \
>      ({ abi_long __r = get_user_u32((x), (gaddr));       \
> @@ -147,24 +148,6 @@ void cpu_loop(CPUARMState *env)
>      }
>  }
>  
> -static uint64_t arm_rand64(void)
> -{
> -    int shift = 64 - clz64(RAND_MAX);
> -    int i, n = 64 / shift + (64 % shift != 0);
> -    uint64_t ret = 0;
> -
> -    for (i = 0; i < n; i++) {
> -        ret = (ret << shift) | rand();
> -    }
> -    return ret;
> -}
> -
> -void arm_init_pauth_key(ARMPACKey *key)
> -{
> -    key->lo = arm_rand64();
> -    key->hi = arm_rand64();
> -}
> -
>  void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
>  {
>      ARMCPU *cpu = arm_env_get_cpu(env);
> @@ -192,11 +175,11 @@ void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
>  #endif
>  
>      if (cpu_isar_feature(aa64_pauth, cpu)) {
> -        arm_init_pauth_key(&env->apia_key);
> -        arm_init_pauth_key(&env->apib_key);
> -        arm_init_pauth_key(&env->apda_key);
> -        arm_init_pauth_key(&env->apdb_key);
> -        arm_init_pauth_key(&env->apga_key);
> +        qemu_guest_getrandom_nofail(&env->apia_key, sizeof(ARMPACKey));
> +        qemu_guest_getrandom_nofail(&env->apib_key, sizeof(ARMPACKey));
> +        qemu_guest_getrandom_nofail(&env->apda_key, sizeof(ARMPACKey));
> +        qemu_guest_getrandom_nofail(&env->apdb_key, sizeof(ARMPACKey));
> +        qemu_guest_getrandom_nofail(&env->apga_key, sizeof(ARMPACKey));
>      }
>  
>      ts->stack_base = info->start_stack;
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 8f7125cd67..c2168db1c8 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -111,6 +111,7 @@
>  
>  #include "qemu.h"
>  #include "qemu/guest-random.h"
> +#include "qapi/error.h"
>  #include "fd-trans.h"
>  
>  #ifndef CLONE_IO
> @@ -9731,25 +9732,45 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>                      int all = (TARGET_PR_PAC_APIAKEY | TARGET_PR_PAC_APIBKEY |
>                                 TARGET_PR_PAC_APDAKEY | TARGET_PR_PAC_APDBKEY |
>                                 TARGET_PR_PAC_APGAKEY);
> +                    int ret = 0;
> +                    Error *err = NULL;
> +
>                      if (arg2 == 0) {
>                          arg2 = all;
>                      } else if (arg2 & ~all) {
>                          return -TARGET_EINVAL;
>                      }
>                      if (arg2 & TARGET_PR_PAC_APIAKEY) {
> -                        arm_init_pauth_key(&env->apia_key);
> +                        ret |= qemu_guest_getrandom(&env->apia_key,
> +                                                    sizeof(ARMPACKey), &err);
>                      }
>                      if (arg2 & TARGET_PR_PAC_APIBKEY) {
> -                        arm_init_pauth_key(&env->apib_key);
> +                        ret |= qemu_guest_getrandom(&env->apib_key,
> +                                                    sizeof(ARMPACKey), &err);
>                      }
>                      if (arg2 & TARGET_PR_PAC_APDAKEY) {
> -                        arm_init_pauth_key(&env->apda_key);
> +                        ret |= qemu_guest_getrandom(&env->apda_key,
> +                                                    sizeof(ARMPACKey), &err);
>                      }
>                      if (arg2 & TARGET_PR_PAC_APDBKEY) {
> -                        arm_init_pauth_key(&env->apdb_key);
> +                        ret |= qemu_guest_getrandom(&env->apdb_key,
> +                                                    sizeof(ARMPACKey), &err);
>                      }
>                      if (arg2 & TARGET_PR_PAC_APGAKEY) {
> -                        arm_init_pauth_key(&env->apga_key);
> +                        ret |= qemu_guest_getrandom(&env->apga_key,
> +                                                    sizeof(ARMPACKey), &err);
> +                    }
> +                    if (ret != 0) {
> +                        /*
> +                         * Some unknown failure in the crypto.  The best
> +                         * we can do is log it and fail the syscall.
> +                         * The real syscall cannot fail this way.
> +                         */
> +                        qemu_log_mask(LOG_UNIMP,
> +                                      "PR_PAC_RESET_KEYS: Crypto failure: %s",
> +                                      error_get_pretty(err));
> +                        error_free(err);
> +                        return -TARGET_EIO;

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>                      }
>                      return 0;
>                  }
> 

Re: [Qemu-devel] [PATCH v3 15/23] linux-user: Remove srand call

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> We no longer use rand() within linux-user.
> 
> Cc: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/main.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 8478306eef..80cb9fdfbe 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -621,8 +621,6 @@ int main(int argc, char **argv, char **envp)
>  
>      cpu_model = NULL;
>  
> -    srand(time(NULL));
> -
>      qemu_add_opts(&qemu_trace_opts);
>  
>      optind = parse_args(argc, argv);
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH v3 20/23] target/arm: Put all PAC keys into a structure

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> This allows us to use a single syscall to initialize them all.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/cpu.h              | 12 +++++++-----
>  linux-user/aarch64/cpu_loop.c |  6 +-----
>  linux-user/syscall.c          | 10 +++++-----
>  target/arm/helper.c           | 20 ++++++++++----------
>  target/arm/pauth_helper.c     | 18 +++++++++---------
>  5 files changed, 32 insertions(+), 34 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 5f23c62132..ae2381e222 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -628,11 +628,13 @@ typedef struct CPUARMState {
>      } iwmmxt;
>  
>  #ifdef TARGET_AARCH64
> -    ARMPACKey apia_key;
> -    ARMPACKey apib_key;
> -    ARMPACKey apda_key;
> -    ARMPACKey apdb_key;
> -    ARMPACKey apga_key;
> +    struct {
> +        ARMPACKey apia;
> +        ARMPACKey apib;
> +        ARMPACKey apda;
> +        ARMPACKey apdb;
> +        ARMPACKey apga;
> +    } keys;

Yay!

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>  #endif
>  
>  #if defined(CONFIG_USER_ONLY)
> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
> index cedad39ca0..2f2f63e3e8 100644
> --- a/linux-user/aarch64/cpu_loop.c
> +++ b/linux-user/aarch64/cpu_loop.c
> @@ -175,11 +175,7 @@ void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
>  #endif
>  
>      if (cpu_isar_feature(aa64_pauth, cpu)) {
> -        qemu_guest_getrandom_nofail(&env->apia_key, sizeof(ARMPACKey));
> -        qemu_guest_getrandom_nofail(&env->apib_key, sizeof(ARMPACKey));
> -        qemu_guest_getrandom_nofail(&env->apda_key, sizeof(ARMPACKey));
> -        qemu_guest_getrandom_nofail(&env->apdb_key, sizeof(ARMPACKey));
> -        qemu_guest_getrandom_nofail(&env->apga_key, sizeof(ARMPACKey));
> +        qemu_guest_getrandom_nofail(&env->keys, sizeof(env->keys));
>      }
>  
>      ts->stack_base = info->start_stack;
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index c2168db1c8..ebbda9e470 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9741,23 +9741,23 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>                          return -TARGET_EINVAL;
>                      }
>                      if (arg2 & TARGET_PR_PAC_APIAKEY) {
> -                        ret |= qemu_guest_getrandom(&env->apia_key,
> +                        ret |= qemu_guest_getrandom(&env->keys.apia,
>                                                      sizeof(ARMPACKey), &err);
>                      }
>                      if (arg2 & TARGET_PR_PAC_APIBKEY) {
> -                        ret |= qemu_guest_getrandom(&env->apib_key,
> +                        ret |= qemu_guest_getrandom(&env->keys.apib,
>                                                      sizeof(ARMPACKey), &err);
>                      }
>                      if (arg2 & TARGET_PR_PAC_APDAKEY) {
> -                        ret |= qemu_guest_getrandom(&env->apda_key,
> +                        ret |= qemu_guest_getrandom(&env->keys.apda,
>                                                      sizeof(ARMPACKey), &err);
>                      }
>                      if (arg2 & TARGET_PR_PAC_APDBKEY) {
> -                        ret |= qemu_guest_getrandom(&env->apdb_key,
> +                        ret |= qemu_guest_getrandom(&env->keys.apdb,
>                                                      sizeof(ARMPACKey), &err);
>                      }
>                      if (arg2 & TARGET_PR_PAC_APGAKEY) {
> -                        ret |= qemu_guest_getrandom(&env->apga_key,
> +                        ret |= qemu_guest_getrandom(&env->keys.apga,
>                                                      sizeof(ARMPACKey), &err);
>                      }
>                      if (ret != 0) {
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 2607d39ad1..7b5d63f894 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -5678,43 +5678,43 @@ static const ARMCPRegInfo pauth_reginfo[] = {
>      { .name = "APDAKEYLO_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 2, .opc2 = 0,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apda_key.lo) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apda.lo) },
>      { .name = "APDAKEYHI_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 2, .opc2 = 1,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apda_key.hi) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apda.hi) },
>      { .name = "APDBKEYLO_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 2, .opc2 = 2,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apdb_key.lo) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apdb.lo) },
>      { .name = "APDBKEYHI_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 2, .opc2 = 3,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apdb_key.hi) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apdb.hi) },
>      { .name = "APGAKEYLO_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 3, .opc2 = 0,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apga_key.lo) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apga.lo) },
>      { .name = "APGAKEYHI_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 3, .opc2 = 1,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apga_key.hi) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apga.hi) },
>      { .name = "APIAKEYLO_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 1, .opc2 = 0,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apia_key.lo) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apia.lo) },
>      { .name = "APIAKEYHI_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 1, .opc2 = 1,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apia_key.hi) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apia.hi) },
>      { .name = "APIBKEYLO_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 1, .opc2 = 2,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apib_key.lo) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apib.lo) },
>      { .name = "APIBKEYHI_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 1, .opc2 = 3,
>        .access = PL1_RW, .accessfn = access_pauth,
> -      .fieldoffset = offsetof(CPUARMState, apib_key.hi) },
> +      .fieldoffset = offsetof(CPUARMState, keys.apib.hi) },
>      REGINFO_SENTINEL
>  };
>  #endif
> diff --git a/target/arm/pauth_helper.c b/target/arm/pauth_helper.c
> index d750f96edf..7f30ae7395 100644
> --- a/target/arm/pauth_helper.c
> +++ b/target/arm/pauth_helper.c
> @@ -403,7 +403,7 @@ uint64_t HELPER(pacia)(CPUARMState *env, uint64_t x, uint64_t y)
>          return x;
>      }
>      pauth_check_trap(env, el, GETPC());
> -    return pauth_addpac(env, x, y, &env->apia_key, false);
> +    return pauth_addpac(env, x, y, &env->keys.apia, false);
>  }
>  
>  uint64_t HELPER(pacib)(CPUARMState *env, uint64_t x, uint64_t y)
> @@ -413,7 +413,7 @@ uint64_t HELPER(pacib)(CPUARMState *env, uint64_t x, uint64_t y)
>          return x;
>      }
>      pauth_check_trap(env, el, GETPC());
> -    return pauth_addpac(env, x, y, &env->apib_key, false);
> +    return pauth_addpac(env, x, y, &env->keys.apib, false);
>  }
>  
>  uint64_t HELPER(pacda)(CPUARMState *env, uint64_t x, uint64_t y)
> @@ -423,7 +423,7 @@ uint64_t HELPER(pacda)(CPUARMState *env, uint64_t x, uint64_t y)
>          return x;
>      }
>      pauth_check_trap(env, el, GETPC());
> -    return pauth_addpac(env, x, y, &env->apda_key, true);
> +    return pauth_addpac(env, x, y, &env->keys.apda, true);
>  }
>  
>  uint64_t HELPER(pacdb)(CPUARMState *env, uint64_t x, uint64_t y)
> @@ -433,7 +433,7 @@ uint64_t HELPER(pacdb)(CPUARMState *env, uint64_t x, uint64_t y)
>          return x;
>      }
>      pauth_check_trap(env, el, GETPC());
> -    return pauth_addpac(env, x, y, &env->apdb_key, true);
> +    return pauth_addpac(env, x, y, &env->keys.apdb, true);
>  }
>  
>  uint64_t HELPER(pacga)(CPUARMState *env, uint64_t x, uint64_t y)
> @@ -441,7 +441,7 @@ uint64_t HELPER(pacga)(CPUARMState *env, uint64_t x, uint64_t y)
>      uint64_t pac;
>  
>      pauth_check_trap(env, arm_current_el(env), GETPC());
> -    pac = pauth_computepac(x, y, env->apga_key);
> +    pac = pauth_computepac(x, y, env->keys.apga);
>  
>      return pac & 0xffffffff00000000ull;
>  }
> @@ -453,7 +453,7 @@ uint64_t HELPER(autia)(CPUARMState *env, uint64_t x, uint64_t y)
>          return x;
>      }
>      pauth_check_trap(env, el, GETPC());
> -    return pauth_auth(env, x, y, &env->apia_key, false, 0);
> +    return pauth_auth(env, x, y, &env->keys.apia, false, 0);
>  }
>  
>  uint64_t HELPER(autib)(CPUARMState *env, uint64_t x, uint64_t y)
> @@ -463,7 +463,7 @@ uint64_t HELPER(autib)(CPUARMState *env, uint64_t x, uint64_t y)
>          return x;
>      }
>      pauth_check_trap(env, el, GETPC());
> -    return pauth_auth(env, x, y, &env->apib_key, false, 1);
> +    return pauth_auth(env, x, y, &env->keys.apib, false, 1);
>  }
>  
>  uint64_t HELPER(autda)(CPUARMState *env, uint64_t x, uint64_t y)
> @@ -473,7 +473,7 @@ uint64_t HELPER(autda)(CPUARMState *env, uint64_t x, uint64_t y)
>          return x;
>      }
>      pauth_check_trap(env, el, GETPC());
> -    return pauth_auth(env, x, y, &env->apda_key, true, 0);
> +    return pauth_auth(env, x, y, &env->keys.apda, true, 0);
>  }
>  
>  uint64_t HELPER(autdb)(CPUARMState *env, uint64_t x, uint64_t y)
> @@ -483,7 +483,7 @@ uint64_t HELPER(autdb)(CPUARMState *env, uint64_t x, uint64_t y)
>          return x;
>      }
>      pauth_check_trap(env, el, GETPC());
> -    return pauth_auth(env, x, y, &env->apdb_key, true, 1);
> +    return pauth_auth(env, x, y, &env->keys.apdb, true, 1);
>  }
>  
>  uint64_t HELPER(xpaci)(CPUARMState *env, uint64_t a)
> 

Re: [Qemu-devel] [PATCH v3 01/23] crypto: Merge crypto-obj-y into libqemuutil.a

[Peter Maydell]

On Fri, 15 Mar 2019 at 03:49, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We will shortly need this in the user-only binaries, so drop the split
> into system and tools binaries.  This also means that crypto-aes-obj-y
> can be merged back into crypto-obj-y.
>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  Makefile             | 12 +++++-------
>  Makefile.objs        |  8 ++------
>  Makefile.target      |  4 ----
>  configure            |  9 +++------
>  crypto/Makefile.objs |  5 +----
>  5 files changed, 11 insertions(+), 27 deletions(-)

This patch breaks a --static configure for me:

e104462:bionic:qemu$ (cd build/all-linux-static/ && '../../configure'
'--cc=ccache gcc' '--enable-debug' '--static' '--disable-system'
'--disable-tools')

ERROR: sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T.
       You probably need to set PKG_CONFIG_LIBDIR
       to point to the right pkg-config files for your
       build target

The error message is bogus, but the problem is that we
tried to build one of the test programs like this:

ccache gcc -pthread -I/usr/include/glib-2.0
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -m64 -mcx16 -D_GNU_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes
-fno-strict-aliasing -fno-common -fwrapv -std=gnu99
-Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value
-Wno-missing-include-dirs -Wempty-body -Wnested-externs
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wold-style-declaration -Wold-style-definition -Wtype-limits
-fstack-protector-strong -I/usr/include/p11-kit-1
-I/usr/include/libpng16 -g -o config-temp/qemu-conf.exe
config-temp/qemu-conf.c -m64 -static -g -lgthread-2.0 -pthread
-lglib-2.0 -pthread -lpcre -pthread -lnettle -lgnutls -lgmp
/usr/lib/x86_64-linux-gnu/libunistring.so -lidn2 -lhogweed -lgmp
-lnettle -ltasn1 -lp11-kit -lz
gcc: error: /usr/lib/x86_64-linux-gnu/libunistring.so: No such file or directory

which is almost certainly because we're now including
a bunch of extra libraries in the link for the static
linux-user binaries which don't work statically linked.
(In particular one of them looks to have a bogus
pkg-config which puts a .so file in the link line for
a static link...)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 01/23] crypto: Merge crypto-obj-y into libqemuutil.a

[Richard Henderson]

On 4/29/19 5:57 AM, Peter Maydell wrote:
> gcc: error: /usr/lib/x86_64-linux-gnu/libunistring.so: No such file or directory
> 
> which is almost certainly because we're now including
> a bunch of extra libraries in the link for the static
> linux-user binaries which don't work statically linked.
> (In particular one of them looks to have a bogus
> pkg-config which puts a .so file in the link line for
> a static link...)

This comes from gnutls.  Thankfully, we can do without it,
so a quick configure-time link test is all that is needed
to disable it.


r~