[U-Boot] [PATCH] lib: uuid: Improve randomness of uuid values on RANDOM_UUID=y

The random uuid values (enabled via CONFIG_RANDOM_UUID=y) on our platform are always the same. Below is consistent on each cold boot:
=> ### reach U-Boot prompt => setenv uuid_gpt_misc => gpt verify mmc 1 $partitions Verify GPT: success! => print uuid_gpt_misc uuid_gpt_misc=d117f98e-6f2c-d04b-a5b2-331a19f91cb2 => setenv uuid_gpt_misc => gpt verify mmc 1 $partitions Verify GPT: success! => print uuid_gpt_misc uuid_gpt_misc=ad5ec4b6-2d9f-8544-9417-fe3bd1c9b1b3
While the uuids do change on every 'gpt {write,verify}' command, the values appear to be taken from the same pool, in the same order.
As a user, I expect a trully random uuid value in the above example. Otherwise, system/RFS designers and OS people might assume they have a reliable/consistent uuid passed by the bootloader, while the truth is U-Boot simply lacks entropy to generate a random string.
Let's use get_timer() to update the seed, same as done in commit da384a9d7628 ("net: rename and refactor eth_rand_ethaddr() function").
Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com --- lib/uuid.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lib/uuid.c b/lib/uuid.c index fa20ee39fc32..8a82cb234b88 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -238,6 +238,8 @@ void gen_rand_uuid(unsigned char *uuid_bin) unsigned int *ptr = (unsigned int *)&uuid; int i;
+ srand(get_timer(0)); + /* Set all fields randomly */ for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++) *(ptr + i) = cpu_to_be32(rand());

On Fri, 26 Apr 2019 23:54:58 +0200 Eugeniu Rosca roscaeugeniu@gmail.com wrote:
The random uuid values (enabled via CONFIG_RANDOM_UUID=y) on our platform are always the same. Below is consistent on each cold boot:
=> ### reach U-Boot prompt => setenv uuid_gpt_misc => gpt verify mmc 1 $partitions Verify GPT: success! => print uuid_gpt_misc uuid_gpt_misc=d117f98e-6f2c-d04b-a5b2-331a19f91cb2 => setenv uuid_gpt_misc => gpt verify mmc 1 $partitions Verify GPT: success! => print uuid_gpt_misc uuid_gpt_misc=ad5ec4b6-2d9f-8544-9417-fe3bd1c9b1b3
While the uuids do change on every 'gpt {write,verify}' command, the values appear to be taken from the same pool, in the same order.
As a user, I expect a trully random uuid value in the above example. Otherwise, system/RFS designers and OS people might assume they have a reliable/consistent uuid passed by the bootloader, while the truth is U-Boot simply lacks entropy to generate a random string.
Let's use get_timer() to update the seed, same as done in commit da384a9d7628 ("net: rename and refactor eth_rand_ethaddr() function").
Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com
lib/uuid.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lib/uuid.c b/lib/uuid.c index fa20ee39fc32..8a82cb234b88 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -238,6 +238,8 @@ void gen_rand_uuid(unsigned char *uuid_bin) unsigned int *ptr = (unsigned int *)&uuid; int i;
- srand(get_timer(0));
- /* Set all fields randomly */ for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++) *(ptr + i) = cpu_to_be32(rand());
Reviewed-by: Lukasz Majewski lukma@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Hi Lukasz,
On Sat, Apr 27, 2019 at 05:05:28PM +0200, Lukasz Majewski wrote: [..]
Reviewed-by: Lukasz Majewski lukma@denx.de
Thank you for the prompt review.

Hi Lukasz,
Thanks to Roman, we found a potential regression in this solution.
Particularly when gen_rand_uuid() is called in a loop, get_timer(0) might return the same value, leading to the same seed value being passed to srand(), leading to the same uuid value being generated for several partitions.
Please, hold off from applying the patch until I push a v2.
To be more clear, NAK.

Superseded by https://patchwork.ozlabs.org/patch/1092945/ ("[U-Boot,4/4] lib: uuid: Improve randomness of uuid values on RANDOM_UUID=y")
participants (3)
-
Eugeniu Rosca
-
Eugeniu Rosca
-
Lukasz Majewski