diff mbox

linux-user: Let user specify random seed

Message ID 1412881938-3757-2-git-send-email-reftel@spotify.com
State New
Headers show

Commit Message

Magnus Reftel Oct. 9, 2014, 7:12 p.m. UTC
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(-)

Comments

Eric Blake Oct. 9, 2014, 9:30 p.m. UTC | #1
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>
Magnus Reftel Oct. 10, 2014, 8:16 a.m. UTC | #2
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
Eric Blake Oct. 10, 2014, 4:20 p.m. UTC | #3
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.
Magnus Reftel Oct. 14, 2014, 9:46 a.m. UTC | #4
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
diff mbox

Patch

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);