Reintroduce loadable modules for random(4)

[markm]

Summary

• Add DEV_RANDOM pseudo-option and use it to "include out" random(4) if desired.
• Retire randomdev_none.c and introduce random_infra.c for resident infrastructure. Completely stub out random(4) calls in the "without DEV_RANDOM" case.
• Add RANDOM_LOADABLE option to allow loadable Yarrow/Fortuna/LocallyWritten algorithm.
• Build modules for Yarrow and Fortuna.
• Use atomics for the live entropy rate-tracking.

[markm]

In D3354#68411, @jmg wrote:

Are you opening this up to more reviewers? Or still only for SO (aka delphij)? The reason I ask is that secteam only consists of delphij, so adding secteam doesn't add any new reviewers.

I wasn't aware that secteam was only delphij.

I'm still tweeking code.

M

[markm]

In D3354#68413, @delphij wrote:

I have only glanced over the code and it seems good overall.

Thanks!

One thought: can we make DEV_RANDOM something like NO_DEV_RANDOM? That way it would be an opt-out feature, as most systems still need it.

Nope, sorry. The DEV_RANDOM pseudo-option comes for free from 'device random' in the kernel config file. There is no negative variant, but it is safe enough given that the kernel configs that need this device have it.

[jmg]

In D3354#68427, @markm wrote:

In D3354#68413, @delphij wrote:

I have only glanced over the code and it seems good overall.

Thanks!

One thought: can we make DEV_RANDOM something like NO_DEV_RANDOM? That way it would be an opt-out feature, as most systems still need it.

Nope, sorry. The DEV_RANDOM pseudo-option comes for free from 'device random' in the kernel config file. There is no negative variant, but it is safe enough given that the kernel configs that need this device have it.

Please, do not add negative options.. Now that we have things like "nodevice random" that works in our config file, we don't need to add a new option like NO_DEV_RANDOM...

[markm]

Build tweeks to make "make universe" work.

[markm]

• Add DEV_RANDOM pseudo-option and use it to "include out" random(4)
• Update documentation for random(4) modules.

[markm]

I am now seeking formal review for this please.

[markm]

I removed 'manages' as an auto-reviewer - it gave a thoroughly broken scripted review.

[wblock]

UPDATING

36 ↗

(On Diff #7961)

them, the kernel must have

41 ↗

(On Diff #7961)

kldload(8) can then be used to load random_fortuna.ko
or random_yarrow.ko.  Please note that due to the indirect

share/man/man4/random.4

138 ↗

(On Diff #7961)

Not yours, but missing a "to"

139 ↗

(On Diff #7961)

Also not yours, but we are here now:
s/your/the/

149 ↗

(On Diff #7961)

"selected" is kind of ambiguous, could mean something other than "included in the config file". "used" is less fuzzy.
"random device" is also kind of unclear.

Suggestion:

When
.Cd "options RANDOM_LOADABLE" is used,
the
.Pa /dev/random
device is not created until an "algorithm module"
is loaded.

153 ↗

(On Diff #7961)

"such modules" might be difficult for translators. Simpler:

Two of these modules are built by default,

158 ↗

(On Diff #7961)

"The latter" requires the reader to look back and locate which was the last of an unspecified previous list. Simpler:

The
.Em random_yarrow
module is deprecated and will be removed in

161 ↗

(On Diff #7961)

This sentence is kind of back-to-front. It implies but does not outright state that Fortuna is used unless overridden with options RANDOM_YARROW, which is not recommended anyway (which should also be stated up front).

The Fortuna algorithm is used by default.
Use of the Yarrow algorithm is not encouraged, but
when still present in the kernel, it can be selected with the
.Cd "options RANDOM_YARROW"
​kernel option.

171 ↗

(On Diff #7961)

s/need to/must/

173 ↗

(On Diff #7961)

s/need to/must/

[markm]

Incorporated with edits, thank you!

[markm]

• Add DEV_RANDOM pseudo-option and use it to "include out" random(4)
• Update documentation for random(4) modules.

[markm]

What the hell is 'manpages' doing, blocking immediately on diff upload? Rogue script?

[markm]

Thanks Warren! (wblock)

[jmg]

Minor comments.

sys/conf/options

714 ↗

(On Diff #7963)

is this really needed in global?

sys/dev/random/random_harvestq.c

63 ↗

(On Diff #7963)

isn't RANDOM_DUMMY retired? Why include it here?

sys/dev/random/random_infra.c

47 ↗

(On Diff #7963)

RANDOM_DUMMY here too..

67 ↗

(On Diff #7963)

Maybe use the following so that the assignment of this variable is grouped closely, so it can easily be seen it gets defined in both cases, and also removed the #else that is far removed from the #if block.

Maybe:
#ifdef RANDOM_LOADABLE
struct random_algorithm *p_random_alg_context = NULL;
#else
struct random_algorithm *p_random_alg_context = & random_alg_context;
#endif

#ifdef RANDOM_LOADABLE
rest of code.
#endif

sys/dev/random/randomdev.c

64 ↗

(On Diff #7963)

again RANDOM_DUMMY removed.

sys/dev/random/randomdev.h

116 ↗

(On Diff #7963)

Maybe opt_random.h should be included in this header file so we don't have to depend upon .c files having it?

[markm]

Thanks JMG.

sys/conf/options

714 ↗

(On Diff #7963)

Yes. It is used when stubbing out calls that are all over the place.

sys/dev/random/random_harvestq.c

63 ↗

(On Diff #7963)

Oops. Removed!

sys/dev/random/random_infra.c

67 ↗

(On Diff #7963)

OK - neater, I agree.

sys/dev/random/randomdev.h

116 ↗

(On Diff #7963)

There is no precedent for that, that I could find. This is the sort of thing that attracts BDE bikesheds.

[markm]

Address jmg review comments.

[jmg]

sys/dev/random/randomdev.h

116–126 ↗

(On Diff #7967)

there is precedence in sys/terminal.h, cam/cam.h, contrib/dev/acpica/include/platform/acfreebsd.h, and dev/usb/usb.h to name a few... Looks like 67 total cases:
find /usr/src/sys -type f -name "*.h" -exec grep opt_ {} + | grep '#include' | wc

[markm]

Address review; tidy up the opt_*.h includes.

[markm]

Thanks J-M!

sys/dev/random/randomdev.h

116–126 ↗

(On Diff #7967)

Shows how well I checked that! OK. Moved to sys/random.h.

[markm]

sys/conf/options

714 ↗

(On Diff #7977)

Turns out I was wrong. This has been fixed - see random.h.

[markm]

Add example module.
Move common _write functions to mase of module.
Remove unused functions.

[bjk]

A couple man page nits inline

share/man/man4/random.4

26 ↗

(On Diff #7980)

Please update .Dd when the changes are finalized.

137 ↗

(On Diff #7980)

Also not yours, but this probably should be the plural "sysctls".

148 ↗

(On Diff #7980)

"is used" must be on a new line.

[markm]

Thanks, bjk!

share/man/man4/random.4

26 ↗

(On Diff #7980)

Will do, with the UPDATING date.

[markm]

Review nits from bjk.

[wblock]

Man page changes look okay to me. Please remember to update .Dd. Thanks!

[markm]

Thanks reviewers!