Hi guys, On Wed, 23 Dec 2020 at 03:24, Torsten Duwe <d...@lst.de> wrote: > > On Sun, 20 Dec 2020 11:17:50 -0700 > Simon Glass <s...@chromium.org> wrote: > > > Hi Torsten, > > > > On Sun, 20 Dec 2020 at 10:00, Torsten Duwe <d...@lst.de> wrote: > > > > > > On Fri, 18 Dec 2020 19:29:12 -0700 > > > Simon Glass <s...@chromium.org> wrote: > > > > > > > > - int i; > > > > > - > > > > > - srand(get_ticks() + rand()); > > > > > + int i, ret; > > > > > + struct udevice *devp; > > > > > + u8 randv = 0; > > > > > + > > > > > +#if defined(CONFIG_DM_RNG) > > > > > > > > This seems a little backwards to me. The caller should request a > > > > RNG device, getting either a hardware one or a software one, and > > > > then call the uclass method to get the uuid. > > > > > > Strictly speaking, there's no such thing as a "software RNG". The > > > term "DRBG" was coined for accurateness, "deterministic random bit > > > generator". The oxymoron "deterministic random" pretty much nails > > > it. Alternatively, it can be called "pseudo" RNG. > > > rand() and srand() exactly implement such a mechanism already, with > > > low coding overhead. U-Boot runs fine with them most of the time, > > > but there are rare cases where real entropy would be needed. This > > > is what these two patches are about. In case there's more, I > > > already speculated about a centralised entity in my response to the > > > v1 cover letter, but for now these two changes should do. > > > > I am used to the term pseudo-random, but it doesn't much matter what > > kind of random number is used. It is still covered by the RNG class. > > Well, in these 2 cases, it _does_ matter. And besides, as I wrote > above, pseudo randomness is produced by the rand() function, and RNG > devices provide _real_ entropy to a system. > > So, while every other entity in U-Boot is happy with a DRBG, a UUID and > a BOOTP delay need this real entropy, hence the different code, for a > start. > > > You are currently burying device access in a utility function. That > > really isn't the right way to do it. See my comment above. There is no > > way to control which RNG device is used and no visibility that this is > > happening at all, outside this function. > > The code looks a bit odd to me, too, as I mentioned. I imagined > something not so full blown as in the Linux kernel, but still some > central mechanism to get entropy from, for those who really need it (in > the current situation, just these 2 cases). This alternative would > result in a real /dev/random in U-Boot, which would yield a cleaner > structure, but would require more code to be produced and more code > needing change. That given, I'd agree to these 2 hacks, especially > because there are security implications. > > What's your opinion, how would you like to create really unique UUIDs? > How should BOOTP clients wait randomly (esp. in a large group)? > Shall we create some U-Boot version of /dev/random and haveged? > > I'm really open to suggestions. >
I really don't mind too much about that side of it. But I do worry when I see code that buries hard-coded DM access in a lib/ function. If boot wants a random device it should get one, at the top level, not in the bowels of U-Boot. It can pass that device down to other functions as needed. To my reading, the current definition of the RNG uclass looks similar to a /dev/random Let me know if I am missing something. Regards, Simon
