[PATCH v2] seedrng: import SeedRNG utility for kernel RNG seed files

[Jason A. Donenfeld]

Hi Bernhard,

On Sun, Apr 10, 2022 at 3:40 PM Bernhard Reutner-Fischer
<rep.dot.nop at gmail.com> wrote:
> > > mkdir: bb_make_directory
> >
> > This one wasn't so useful: I don't actually want _recursive_ directory
> > creation, I don't think.
>
> Probably. But it doesn't harm either. And if it saves size it's
> preferable.

Recursive directory creation has a tendency to make bugs worse --
instead of failing with some error, you get some monster path created
someplace you didn't want, which is a problem for a root utility.
Using that function also doesn't change binary size.

> Also seed_dir et al are only used in main so i'd move them there and
> not have them static. Doesn't make much difference though.

seed_dir isn't only used from main. It doesn't impact binary size.

> Furthermore you don't need separate realtime and boottime storage.

Will adjust for a v4.

>
> fprintf(stderr,...,strerror(errno)) -> bb_simple_perror_msg_and_die
> or bb_perror_msg_and_die

Yep, did this for v3.

By the way, I'm going to go back to vanilla strotul for v4. I noticed
bb_strotul was bloating the binary a bit more than I wanted, expanding
to 4 calls because of additional checks. My min/max check is an okay
replacement for those checks.

>
> >
> > > Why do you open(), flock() instead of using O_EXCL?
> >
> > So that if the system crashes, the next boot can still move forward,
>
> The next boot will not find any previous data i assume? I thought they
> live in /run which usually is not in battery backed up RAM.

Sometimes it's tmpfs, sometimes it isn't. And sometimes programs get
oom'd mid-execution and can't cleanup files. O_EXCL is not the right
way of doing this.

>
> I'd first create the directories and try to obtain the exclusive lock
> and only then start fiddling with the hash, no?

It really doesn't matter, but sure, if you prefer, I'll reorder that.
My rationale was so that two processes executing very close to each
other have a shorter critical section and therefore need to perform
less work once taking the lock. But it seems like that's not to your
tastes, so I'll change the order for v4.

> > or if the process crashes, the next run can still move forward.
> > flock() is a runtime thing, where as O_EXCL is a "must be in
> > filesystem" thing, which is much weaker.
>
> I admit i did not really look. Don't you ever only want to run one
> instance at once? I had thought so, no?

Yes, which is why the lock is there. Running more than one at once can
be catastrophic. That invariant must be enforced with a lock. And
given that people might wind up running this on timers -- timers that
can race with other events like shutdown -- that lock must be there.
I'm not going to remove the lock, sorry.

> Maybe you can write a small introductory note at the top of the file
> that describes the theory of operation, please?

Sure, will do for v4.

>
> I'd put the out: cleanup dance in an
> if (ENABLE_FEATURE_CLEAN_UP) guard

For an even smaller binary, nice. Added to v4.

Will send out v4 momentarily.

Thanks,
Jason