qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-08 12:13 [Qemu-devel] [PATCH] Let user specify random seed for linux-user Magnus Reftel
@ 2014-10-08 12:13 ` Magnus Reftel
  2014-10-08 14:45   ` Eric Blake
  0 siblings, 1 reply; 16+ messages in thread
From: Magnus Reftel @ 2014-10-08 12:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Magnus Reftel

This patch introduces the -seed command line option and the
QEMU_RAND_SEED environment variable for setting the random seed, which
is used for the AT_RANDOM ELF aux entry.

Signed-off-by: Magnus Reftel <reftel@spotify.com>
---
 linux-user/elfload.c |  1 -
 linux-user/main.c    | 21 +++++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 1c04fcf..f2e2197 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1539,7 +1539,6 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
      * Generate 16 random bytes for userspace PRNG seeding (not
      * cryptically secure but it's not the aim of QEMU).
      */
-    srand((unsigned int) time(NULL));
     for (i = 0; i < 16; i++) {
         k_rand_bytes[i] = rand();
     }
diff --git a/linux-user/main.c b/linux-user/main.c
index 483eb3f..57cd721 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -46,6 +46,8 @@ unsigned long mmap_min_addr;
 #if defined(CONFIG_USE_GUEST_BASE)
 unsigned long guest_base;
 int have_guest_base;
+static bool have_rand_seed = false;
+static int rand_seed;
 #if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
 /*
  * When running 32-on-64 we should make sure we can fit all of the possible
@@ -3546,6 +3548,12 @@ static void handle_arg_pagesize(const char *arg)
     }
 }
 
+static void handle_arg_randseed(const char *arg)
+{
+    have_rand_seed = true;
+    rand_seed = atoi(arg);
+}
+
 static void handle_arg_gdb(const char *arg)
 {
     gdbstub_port = atoi(arg);
@@ -3674,6 +3682,8 @@ 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 for pseudo-random number generator"},
     {"version",    "QEMU_VERSION",     false, handle_arg_version,
      "",           "display version information and exit"},
     {NULL, NULL, false, NULL, NULL, NULL}
@@ -3926,6 +3936,17 @@ int main(int argc, char **argv, char **envp)
         do_strace = 1;
     }
 
+    if (getenv("QEMU_RAND_SEED")) {
+        have_rand_seed = true;
+        rand_seed = atoi(getenv("QEMU_RAND_SEED"));
+    }
+
+    if (have_rand_seed) {
+        srand(rand_seed);
+    } else {
+        srand((int)time(NULL));
+    }
+
     target_environ = envlist_to_environ(envlist, NULL);
     envlist_free(envlist);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-08 12:13 ` [Qemu-devel] [PATCH] linux-user: Let user specify random seed Magnus Reftel
@ 2014-10-08 14:45   ` Eric Blake
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2014-10-08 14:45 UTC (permalink / raw)
  To: Magnus Reftel, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]

On 10/08/2014 06:13 AM, Magnus Reftel wrote:
> This patch introduces the -seed command line option and the
> QEMU_RAND_SEED environment variable for setting the random seed, which
> is used for the AT_RANDOM ELF aux entry.
> 
> Signed-off-by: Magnus Reftel <reftel@spotify.com>
> ---
>  linux-user/elfload.c |  1 -
>  linux-user/main.c    | 21 +++++++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 

> +++ b/linux-user/main.c
> @@ -46,6 +46,8 @@ unsigned long mmap_min_addr;
>  #if defined(CONFIG_USE_GUEST_BASE)
>  unsigned long guest_base;
>  int have_guest_base;
> +static bool have_rand_seed = false;

static variables are automatically 0-initialized without needing an
explicit initializer.

> +static int rand_seed;
>  #if (TARGET_LONG_BITS == 32) && (HOST_LONG_BITS == 64)
>  /*
>   * When running 32-on-64 we should make sure we can fit all of the possible
> @@ -3546,6 +3548,12 @@ static void handle_arg_pagesize(const char *arg)
>      }
>  }
>  
> +static void handle_arg_randseed(const char *arg)
> +{
> +    have_rand_seed = true;
> +    rand_seed = atoi(arg);
> +}

atoi() is trash when compared to strtol() - it doesn't diagnose
overflow, trailing garbage, or empty input.


> @@ -3926,6 +3936,17 @@ int main(int argc, char **argv, char **envp)
>          do_strace = 1;
>      }
>  
> +    if (getenv("QEMU_RAND_SEED")) {
> +        have_rand_seed = true;
> +        rand_seed = atoi(getenv("QEMU_RAND_SEED"));
> +    }

why not call handle_arg_randseed(getenv("QEMU_RAND_SEED")) here?

> +
> +    if (have_rand_seed) {
> +        srand(rand_seed);
> +    } else {
> +        srand((int)time(NULL));

The cast is pointless.  This is C.

> +    }

Do you even need have_rand_seed?  Why not just pre-initialize
rand_seed=time(NULL) and then overwrite rand_seed if the environment
variable is present?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2] linux-user: Let user specify random seed
@ 2014-10-09  8:36 Magnus Reftel
  2014-10-09  8:36 ` [Qemu-devel] [PATCH] " Magnus Reftel
  2014-10-09 19:43 ` [Qemu-devel] [PATCH v2] " Tom Musta
  0 siblings, 2 replies; 16+ messages in thread
From: Magnus Reftel @ 2014-10-09  8:36 UTC (permalink / raw)
  To: qemu-devel

This patch introduces the -seed command line option and the
QEMU_RAND_SEED environment variable for setting the random seed, which
is used for the AT_RANDOM ELF aux entry.

This is an updated version of the patch, addressing review comments
from Eric Blake.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-09  8:36 [Qemu-devel] [PATCH v2] linux-user: Let user specify random seed Magnus Reftel
@ 2014-10-09  8:36 ` Magnus Reftel
  2014-10-09 15:27   ` Eric Blake
  2014-10-09 19:43 ` [Qemu-devel] [PATCH v2] " Tom Musta
  1 sibling, 1 reply; 16+ messages in thread
From: Magnus Reftel @ 2014-10-09  8:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Magnus Reftel

This patch introduces the -seed command line option and the
QEMU_RAND_SEED environment variable for setting the random seed, which
is used for the AT_RANDOM ELF aux entry.

Signed-off-by: Magnus Reftel <reftel@spotify.com>
---
 linux-user/elfload.c |  1 -
 linux-user/main.c    | 20 ++++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 1c04fcf..f2e2197 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1539,7 +1539,6 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
      * Generate 16 random bytes for userspace PRNG seeding (not
      * cryptically secure but it's not the aim of QEMU).
      */
-    srand((unsigned int) time(NULL));
     for (i = 0; i < 16; i++) {
         k_rand_bytes[i] = rand();
     }
diff --git a/linux-user/main.c b/linux-user/main.c
index 483eb3f..e80255c 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3546,6 +3546,18 @@ static void handle_arg_pagesize(const char *arg)
     }
 }
 
+static void handle_arg_randseed(const char *arg)
+{
+    unsigned long seed;
+    char* end;
+    seed = strtoul(arg, &end, 0);
+    if (end==arg || *end!='\0' || seed > UINT_MAX) {
+        fprintf(stderr, "Invalid seed number: %s\n", arg);
+        exit(1);
+    }
+    srand(seed);
+}
+
 static void handle_arg_gdb(const char *arg)
 {
     gdbstub_port = atoi(arg);
@@ -3674,6 +3686,8 @@ 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 for pseudo-random number generator"},
     {"version",    "QEMU_VERSION",     false, handle_arg_version,
      "",           "display version information and exit"},
     {NULL, NULL, false, NULL, NULL, NULL}
@@ -3856,6 +3870,8 @@ int main(int argc, char **argv, char **envp)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
+    srand(time(NULL));
+
     optind = parse_args(argc, argv);
 
     /* Zero out regs */
@@ -3926,6 +3942,10 @@ int main(int argc, char **argv, char **envp)
         do_strace = 1;
     }
 
+    if (getenv("QEMU_RAND_SEED")) {
+        handle_arg_randseed(getenv("QEMU_RAND_SEED"));
+    }
+
     target_environ = envlist_to_environ(envlist, NULL);
     envlist_free(envlist);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-09  8:36 ` [Qemu-devel] [PATCH] " Magnus Reftel
@ 2014-10-09 15:27   ` Eric Blake
  2014-10-09 19:10     ` Magnus Reftel
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2014-10-09 15:27 UTC (permalink / raw)
  To: Magnus Reftel, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]

On 10/09/2014 02:36 AM, Magnus Reftel wrote:
> This patch introduces the -seed command line option and the
> QEMU_RAND_SEED environment variable for setting the random seed, which
> is used for the AT_RANDOM ELF aux entry.
> 
> Signed-off-by: Magnus Reftel <reftel@spotify.com>
> ---

>  
> +static void handle_arg_randseed(const char *arg)
> +{
> +    unsigned long seed;
> +    char* end;

Style: we prefer:

char *end;

> +    seed = strtoul(arg, &end, 0);
> +    if (end==arg || *end!='\0' || seed > UINT_MAX) {

Style: spaces around operators:

if (end == arg || *end || seed > UINT_MAX) {

Bug: strtoul() sometimes reports error via errno; the only safe way to
use it is to first prime errno = 0, then do strtoul, then check if errno
was changed.

Reimplementation: util/cutils.c already provides parse_uint() that takes
care of calling strtoul safely (hmm, that version only parses 64-bit
numbers; maybe we should expand it to also parse 32-bit numbers?)

Surprising behavior: your code behaves differently on 32-bit hosts than
it does on 64-bit hosts.  Seriously.  strotoul() has the annoying
specification of requiring twos-complement wraparound according to the
size of long, which means "-1" on a 32-bit platform parses as 0xffffffff
(accepted), while on a 64-bit platform parses it as 0xffffffffffffffff
(which you reject as > UINT_MAX); conversely "-18446744073709551615"
fails to parse due to overflow on a 32-bit platform, while successfully
being parsed as 1 on 64-bit.

> +        fprintf(stderr, "Invalid seed number: %s\n", arg);
> +        exit(1);
> +    }
> +    srand(seed);
> +}
> +
>  static void handle_arg_gdb(const char *arg)
>  {
>      gdbstub_port = atoi(arg);
> @@ -3674,6 +3686,8 @@ 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 for pseudo-random number generator"},
>      {"version",    "QEMU_VERSION",     false, handle_arg_version,
>       "",           "display version information and exit"},
>      {NULL, NULL, false, NULL, NULL, NULL}
> @@ -3856,6 +3870,8 @@ int main(int argc, char **argv, char **envp)
>      cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
>  #endif
>  
> +    srand(time(NULL));
> +
>      optind = parse_args(argc, argv);
>  
>      /* Zero out regs */
> @@ -3926,6 +3942,10 @@ int main(int argc, char **argv, char **envp)
>          do_strace = 1;
>      }
>  
> +    if (getenv("QEMU_RAND_SEED")) {
> +        handle_arg_randseed(getenv("QEMU_RAND_SEED"));
> +    }

Now that you have exactly one caller of the static function, it might
make sense to just inline the body of that function here.

> +
>      target_environ = envlist_to_environ(envlist, NULL);
>      envlist_free(envlist);
>  
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-09 15:27   ` Eric Blake
@ 2014-10-09 19:10     ` Magnus Reftel
  0 siblings, 0 replies; 16+ messages in thread
From: Magnus Reftel @ 2014-10-09 19:10 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-devel

Hi,

Thank you for your patience! I will send a third version.

On Thu, Oct 9, 2014 at 5:27 PM, Eric Blake <eblake@redhat.com> wrote:
> On 10/09/2014 02:36 AM, Magnus Reftel wrote:
>> +    char* end;
> Style: we prefer:
> char *end;

Done.

>> +    if (end==arg || *end!='\0' || seed > UINT_MAX) {
> Style: spaces around operators:

Done.

> Bug: strtoul() sometimes reports error via errno; the only safe way to
> use it is to first prime errno = 0, then do strtoul, then check if errno
> was changed.
>
> Reimplementation: util/cutils.c already provides parse_uint() that takes
> care of calling strtoul safely (hmm, that version only parses 64-bit
> numbers; maybe we should expand it to also parse 32-bit numbers?)

Solved both by switching to parse_uint.

>> +    {"seed",       "QEMU_RAND_SEED",   true,  handle_arg_randseed,
>> +     "",           "Seed for pseudo-random number generator"},
...
>> +    if (getenv("QEMU_RAND_SEED")) {
>> +        handle_arg_randseed(getenv("QEMU_RAND_SEED"));
>> +    }
>
> Now that you have exactly one caller of the static function, it might
> make sense to just inline the body of that function here.

No, it may be called using the function pointer in the argument table, above.

Best Regards
Magnus reftel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-09 19:12 [Qemu-devel] [PATCH v3] " Magnus Reftel
@ 2014-10-09 19:12 ` Magnus Reftel
  2014-10-09 21:30   ` Eric Blake
  0 siblings, 1 reply; 16+ messages in thread
From: Magnus Reftel @ 2014-10-09 19:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Magnus Reftel

This patch introduces the -seed command line option and the
QEMU_RAND_SEED environment variable for setting the random seed, which
is used for the AT_RANDOM ELF aux entry.

Signed-off-by: Magnus Reftel <reftel@spotify.com>
---
 linux-user/elfload.c |  1 -
 linux-user/main.c    | 20 ++++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 1c04fcf..f2e2197 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1539,7 +1539,6 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
      * Generate 16 random bytes for userspace PRNG seeding (not
      * cryptically secure but it's not the aim of QEMU).
      */
-    srand((unsigned int) time(NULL));
     for (i = 0; i < 16; i++) {
         k_rand_bytes[i] = rand();
     }
diff --git a/linux-user/main.c b/linux-user/main.c
index 483eb3f..56e0e28 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3546,6 +3546,18 @@ static void handle_arg_pagesize(const char *arg)
     }
 }
 
+static void handle_arg_randseed(const char *arg)
+{
+    unsigned long long seed;
+    char *end;
+
+    if (parse_uint(arg, &seed, &end, 0) != 0 || *end != 0 || seed > UINT_MAX) {
+        fprintf(stderr, "Invalid seed number: %s\n", arg);
+        exit(1);
+    }
+    srand(seed);
+}
+
 static void handle_arg_gdb(const char *arg)
 {
     gdbstub_port = atoi(arg);
@@ -3674,6 +3686,8 @@ 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 for pseudo-random number generator"},
     {"version",    "QEMU_VERSION",     false, handle_arg_version,
      "",           "display version information and exit"},
     {NULL, NULL, false, NULL, NULL, NULL}
@@ -3856,6 +3870,8 @@ int main(int argc, char **argv, char **envp)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
+    srand(time(NULL));
+
     optind = parse_args(argc, argv);
 
     /* Zero out regs */
@@ -3926,6 +3942,10 @@ int main(int argc, char **argv, char **envp)
         do_strace = 1;
     }
 
+    if (getenv("QEMU_RAND_SEED")) {
+        handle_arg_randseed(getenv("QEMU_RAND_SEED"));
+    }
+
     target_environ = envlist_to_environ(envlist, NULL);
     envlist_free(envlist);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2] linux-user: Let user specify random seed
  2014-10-09  8:36 [Qemu-devel] [PATCH v2] linux-user: Let user specify random seed Magnus Reftel
  2014-10-09  8:36 ` [Qemu-devel] [PATCH] " Magnus Reftel
@ 2014-10-09 19:43 ` Tom Musta
  2014-10-10  8:00   ` Magnus Reftel
  1 sibling, 1 reply; 16+ messages in thread
From: Tom Musta @ 2014-10-09 19:43 UTC (permalink / raw)
  To: Magnus Reftel, qemu-devel

On 10/9/2014 3:36 AM, Magnus Reftel wrote:
> This patch introduces the -seed command line option and the
> QEMU_RAND_SEED environment variable for setting the random seed, which
> is used for the AT_RANDOM ELF aux entry.
> 
> This is an updated version of the patch, addressing review comments
> from Eric Blake.
> 

Magnus:

Possibly a dumb question:  In a regular environment, is there a way for a user to control the 16 bytes of random data pointed to by AT_RANDOM?  (I cannot find one).

If not, why is this capability needed in Linux user mode?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-09 19:12 ` [Qemu-devel] [PATCH] " Magnus Reftel
@ 2014-10-09 21:30   ` Eric Blake
  2014-10-10  8:16     ` Magnus Reftel
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2014-10-09 21:30 UTC (permalink / raw)
  To: Magnus Reftel, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 927 bytes --]

On 10/09/2014 01:12 PM, Magnus Reftel wrote:
> This patch introduces the -seed command line option and the
> QEMU_RAND_SEED environment variable for setting the random seed, which
> is used for the AT_RANDOM ELF aux entry.
> 
> Signed-off-by: Magnus Reftel <reftel@spotify.com>
> ---
>  linux-user/elfload.c |  1 -
>  linux-user/main.c    | 20 ++++++++++++++++++++
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 

>  
> +static void handle_arg_randseed(const char *arg)
> +{
> +    unsigned long long seed;
> +    char *end;
> +
> +    if (parse_uint(arg, &seed, &end, 0) != 0 || *end != 0 || seed > UINT_MAX) {

Slightly shorter as:

if (parse_uint_full(arg, &seed, 0) < 0 || seed > UINT_MAX) {

but that's not a functional difference.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2] linux-user: Let user specify random seed
  2014-10-09 19:43 ` [Qemu-devel] [PATCH v2] " Tom Musta
@ 2014-10-10  8:00   ` Magnus Reftel
  0 siblings, 0 replies; 16+ messages in thread
From: Magnus Reftel @ 2014-10-10  8:00 UTC (permalink / raw)
  To: Tom Musta; +Cc: qemu-devel

On Thu, Oct 9, 2014 at 9:43 PM, Tom Musta <tommusta@gmail.com> wrote:
> On 10/9/2014 3:36 AM, Magnus Reftel wrote:
>> This patch introduces the -seed command line option and the
>> QEMU_RAND_SEED environment variable for setting the random seed, which
>> is used for the AT_RANDOM ELF aux entry.
>>
>> This is an updated version of the patch, addressing review comments
>> from Eric Blake.
> Possibly a dumb question:  In a regular environment, is there a way for a user to control the 16 bytes of random data
> pointed to by AT_RANDOM?  (I cannot find one).
>
> If not, why is this capability needed in Linux user mode?

The reason for this feature is to enable fully reproducible automatic
testing. You are correct that this is currently lacking in the Linux
kernel (the random bytes are always obtained using get_random_bytes).
For my purposes, having reproducibility under qemu is enough, but if
I'd be running on bare metal, I'd submit a patch for the kernel as
well.

BR
Magnus Reftel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-09 21:30   ` Eric Blake
@ 2014-10-10  8:16     ` Magnus Reftel
  2014-10-10 16:20       ` Eric Blake
  0 siblings, 1 reply; 16+ messages in thread
From: Magnus Reftel @ 2014-10-10  8:16 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-devel

On Thu, Oct 9, 2014 at 11:30 PM, Eric Blake <eblake@redhat.com> wrote:
> On 10/09/2014 01:12 PM, Magnus Reftel wrote:
>> +    if (parse_uint(arg, &seed, &end, 0) != 0 || *end != 0 || seed > UINT_MAX) {
>
> Slightly shorter as:
>
> if (parse_uint_full(arg, &seed, 0) < 0 || seed > UINT_MAX) {
>
> but that's not a functional difference.

That would silently truncate and accept strings containing illegal
characters at the end, e.g. 123a would be treated at 123 (decimal)
while it was likely intended to be 0x123a. Therefore, I suggest to
keep the code as proposed in version 3 of the patch.

BR
Magnus

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-10  8:16     ` Magnus Reftel
@ 2014-10-10 16:20       ` Eric Blake
  2014-10-14  9:46         ` Magnus Reftel
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2014-10-10 16:20 UTC (permalink / raw)
  To: Magnus Reftel; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

On 10/10/2014 02:16 AM, Magnus Reftel wrote:
> On Thu, Oct 9, 2014 at 11:30 PM, Eric Blake <eblake@redhat.com> wrote:
>> On 10/09/2014 01:12 PM, Magnus Reftel wrote:
>>> +    if (parse_uint(arg, &seed, &end, 0) != 0 || *end != 0 || seed > UINT_MAX) {
>>
>> Slightly shorter as:
>>
>> if (parse_uint_full(arg, &seed, 0) < 0 || seed > UINT_MAX) {
>>
>> but that's not a functional difference.
> 
> That would silently truncate and accept strings containing illegal
> characters at the end, e.g. 123a would be treated at 123 (decimal)

No, the whole point of using parse_uint_full() instead of parse_uint()
is that parse_uint_full() has one less parameter and enforces no
trailing garbage on your behalf.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-10 16:20       ` Eric Blake
@ 2014-10-14  9:46         ` Magnus Reftel
  0 siblings, 0 replies; 16+ messages in thread
From: Magnus Reftel @ 2014-10-14  9:46 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-devel

On Fri, Oct 10, 2014 at 6:20 PM, Eric Blake <eblake@redhat.com> wrote:
> On 10/10/2014 02:16 AM, Magnus Reftel wrote:
>> On Thu, Oct 9, 2014 at 11:30 PM, Eric Blake <eblake@redhat.com> wrote:
>>> On 10/09/2014 01:12 PM, Magnus Reftel wrote:
>>>> +    if (parse_uint(arg, &seed, &end, 0) != 0 || *end != 0 || seed > UINT_MAX) {
>>>
>>> Slightly shorter as:
>>>
>>> if (parse_uint_full(arg, &seed, 0) < 0 || seed > UINT_MAX) {
>>>
>>> but that's not a functional difference.
>>
>> That would silently truncate and accept strings containing illegal
>> characters at the end, e.g. 123a would be treated at 123 (decimal)
>
> No, the whole point of using parse_uint_full() instead of parse_uint()
> is that parse_uint_full() has one less parameter and enforces no
> trailing garbage on your behalf.

Right, missed the "_full". Sending an updated patch.

BR
Magnus Reftel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-14  9:50 [Qemu-devel] [PATCH v4] " Magnus Reftel
@ 2014-10-14  9:50 ` Magnus Reftel
  2014-10-14 15:07   ` Eric Blake
  0 siblings, 1 reply; 16+ messages in thread
From: Magnus Reftel @ 2014-10-14  9:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Magnus Reftel

This patch introduces the -seed command line option and the
QEMU_RAND_SEED environment variable for setting the random seed, which
is used for the AT_RANDOM ELF aux entry.

Signed-off-by: Magnus Reftel <reftel@spotify.com>
---
 linux-user/elfload.c |  1 -
 linux-user/main.c    | 19 +++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 1c04fcf..f2e2197 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1539,7 +1539,6 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
      * Generate 16 random bytes for userspace PRNG seeding (not
      * cryptically secure but it's not the aim of QEMU).
      */
-    srand((unsigned int) time(NULL));
     for (i = 0; i < 16; i++) {
         k_rand_bytes[i] = rand();
     }
diff --git a/linux-user/main.c b/linux-user/main.c
index 483eb3f..5887022 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3546,6 +3546,17 @@ static void handle_arg_pagesize(const char *arg)
     }
 }
 
+static void handle_arg_randseed(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(1);
+    }
+    srand(seed);
+}
+
 static void handle_arg_gdb(const char *arg)
 {
     gdbstub_port = atoi(arg);
@@ -3674,6 +3685,8 @@ 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 for pseudo-random number generator"},
     {"version",    "QEMU_VERSION",     false, handle_arg_version,
      "",           "display version information and exit"},
     {NULL, NULL, false, NULL, NULL, NULL}
@@ -3856,6 +3869,8 @@ int main(int argc, char **argv, char **envp)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
+    srand(time(NULL));
+
     optind = parse_args(argc, argv);
 
     /* Zero out regs */
@@ -3926,6 +3941,10 @@ int main(int argc, char **argv, char **envp)
         do_strace = 1;
     }
 
+    if (getenv("QEMU_RAND_SEED")) {
+        handle_arg_randseed(getenv("QEMU_RAND_SEED"));
+    }
+
     target_environ = envlist_to_environ(envlist, NULL);
     envlist_free(envlist);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-14  9:50 ` [Qemu-devel] [PATCH] " Magnus Reftel
@ 2014-10-14 15:07   ` Eric Blake
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2014-10-14 15:07 UTC (permalink / raw)
  To: Magnus Reftel, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 584 bytes --]

On 10/14/2014 03:50 AM, Magnus Reftel wrote:
> This patch introduces the -seed command line option and the
> QEMU_RAND_SEED environment variable for setting the random seed, which
> is used for the AT_RANDOM ELF aux entry.
> 
> Signed-off-by: Magnus Reftel <reftel@spotify.com>
> ---
>  linux-user/elfload.c |  1 -
>  linux-user/main.c    | 19 +++++++++++++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH] linux-user: Let user specify random seed
  2014-10-14 15:18 [Qemu-devel] [PATCH v5] " Magnus Reftel
@ 2014-10-14 15:18 ` Magnus Reftel
  0 siblings, 0 replies; 16+ messages in thread
From: Magnus Reftel @ 2014-10-14 15:18 UTC (permalink / raw)
  To: qemu-devel, riku.voipio; +Cc: Magnus Reftel

This patch introduces the -seed command line option and the
QEMU_RAND_SEED environment variable for setting the random seed, which
is used for the AT_RANDOM ELF aux entry.

Signed-off-by: Magnus Reftel <reftel@spotify.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 linux-user/elfload.c |  1 -
 linux-user/main.c    | 19 +++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 1c04fcf..f2e2197 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1539,7 +1539,6 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
      * Generate 16 random bytes for userspace PRNG seeding (not
      * cryptically secure but it's not the aim of QEMU).
      */
-    srand((unsigned int) time(NULL));
     for (i = 0; i < 16; i++) {
         k_rand_bytes[i] = rand();
     }
diff --git a/linux-user/main.c b/linux-user/main.c
index 483eb3f..5887022 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3546,6 +3546,17 @@ static void handle_arg_pagesize(const char *arg)
     }
 }
 
+static void handle_arg_randseed(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(1);
+    }
+    srand(seed);
+}
+
 static void handle_arg_gdb(const char *arg)
 {
     gdbstub_port = atoi(arg);
@@ -3674,6 +3685,8 @@ 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 for pseudo-random number generator"},
     {"version",    "QEMU_VERSION",     false, handle_arg_version,
      "",           "display version information and exit"},
     {NULL, NULL, false, NULL, NULL, NULL}
@@ -3856,6 +3869,8 @@ int main(int argc, char **argv, char **envp)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
+    srand(time(NULL));
+
     optind = parse_args(argc, argv);
 
     /* Zero out regs */
@@ -3926,6 +3941,10 @@ int main(int argc, char **argv, char **envp)
         do_strace = 1;
     }
 
+    if (getenv("QEMU_RAND_SEED")) {
+        handle_arg_randseed(getenv("QEMU_RAND_SEED"));
+    }
+
     target_environ = envlist_to_environ(envlist, NULL);
     envlist_free(envlist);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-10-14 15:20 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-09  8:36 [Qemu-devel] [PATCH v2] linux-user: Let user specify random seed Magnus Reftel
2014-10-09  8:36 ` [Qemu-devel] [PATCH] " Magnus Reftel
2014-10-09 15:27   ` Eric Blake
2014-10-09 19:10     ` Magnus Reftel
2014-10-09 19:43 ` [Qemu-devel] [PATCH v2] " Tom Musta
2014-10-10  8:00   ` Magnus Reftel
  -- strict thread matches above, loose matches on Subject: below --
2014-10-14 15:18 [Qemu-devel] [PATCH v5] " Magnus Reftel
2014-10-14 15:18 ` [Qemu-devel] [PATCH] " Magnus Reftel
2014-10-14  9:50 [Qemu-devel] [PATCH v4] " Magnus Reftel
2014-10-14  9:50 ` [Qemu-devel] [PATCH] " Magnus Reftel
2014-10-14 15:07   ` Eric Blake
2014-10-09 19:12 [Qemu-devel] [PATCH v3] " Magnus Reftel
2014-10-09 19:12 ` [Qemu-devel] [PATCH] " Magnus Reftel
2014-10-09 21:30   ` Eric Blake
2014-10-10  8:16     ` Magnus Reftel
2014-10-10 16:20       ` Eric Blake
2014-10-14  9:46         ` Magnus Reftel
2014-10-08 12:13 [Qemu-devel] [PATCH] Let user specify random seed for linux-user Magnus Reftel
2014-10-08 12:13 ` [Qemu-devel] [PATCH] linux-user: Let user specify random seed Magnus Reftel
2014-10-08 14:45   ` Eric Blake

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).