Page MenuHomeFreeBSD

Allow hostuuid to be preloaded for early-boot use
ClosedPublic

Authored by kevans on Apr 4 2020, 2:12 AM.

Details

Summary

prison0's hostuuid will get set by the hostid rc script, either after generating it and saving it to /etc/hostid or by simply reading /etc/hostid.

Some things (e.g. arbitrary MAC address generation) may use the hostuuid as a factor in early boot, so providing a way to read /etc/hostid (if it's available) and using it before userland starts up is desirable. The code is written such that the preload doesn't *have* to be /etc/hostid, thus not assuming that there will be newline at the end of the buffer or even the exact shape of the newline. White trailing whitespace/non-printables trimmed, the result will be validated as a valid uuid before it's used for early boot purposes.

It doesn't seem necessary at this time to be concerned with kern.hostid.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sys/kern/kern_jail.c
241 ↗(On Diff #70180)

The code and comment don't quite agree; spaces are printable. I'm also not fond of hex constants. But maybe it would make sense to check for hex digits and hyphen, and skip anything else? Otherwise 0x20 could be ' ' (a space character)? Maybe the comment should be "non-printable characters and spaces".

sys/kern/kern_jail.c
241 ↗(On Diff #70180)

I guess the comment could read "and spaces" instead of "including spaces" as it currently does...I'm a bit unsure of enforcing anything else, as we don't seem to enforce actual uuid formatting elsewhere

sys/kern/kern_jail.c
241 ↗(On Diff #70180)

The kernel doesn't enforce anything on the hostuuid content, but rc.d/hostid does. Should we make those same checks here?

sys/kern/kern_jail.c
241 ↗(On Diff #70180)

To be clear, I wasn't proposing syntax checks, but just to trim trailing characters that are not hex digits or hyphens, although even the latter shouldn't be last either.

kevans edited the summary of this revision. (Show Details)
  • Wordsmith the comment a little bit to note we're intentionally trimming off whitespace and non-printables
  • Add a new validate_uuid that's much like parse_uuid but strictly 'just the facts'. Use it to validate the uuid we pulled from the preload.

Looks good, I didn't realize that the kernel already had validation code. Might as well use it!

This revision is now accepted and ready to land.Apr 14 2020, 5:45 AM

Looks good to me overall, but I think we don't really need to scan the first 36 characters because validate_uuid would have checked it.

sys/kern/kern_jail.c
242 ↗(On Diff #70471)

Shouldn't this be size > 36? (validate_uuid already checks for the contents, so it doesn't seem that we need to check the first 36 characters.).

sys/kern/kern_jail.c
242 ↗(On Diff #70471)

Size check is just to make sure that we weren't passed an entirely string once the whitespace near the end is passed; the expectation is that we'll stop at character 36 because data[size - 1] > 0x20 and we're walking it backwards.

I'm contemplating defaulting the preload to off and turning it on for ARM images in release(7). It's unclear if we run into the class of problem this solves with any VM images (i.e. needing to generate a MAC and we use hostuuid in early boot).

Arm seems most likely to want pre-loading, but I don't think it hurts anything. I think it could be enabled for everything. How about mips, powerpc?

Arm seems most likely to want pre-loading, but I don't think it hurts anything. I think it could be enabled for everything. How about mips, powerpc?

mips could benefit, but they don't use loader so it's a bit dead in the water there. powerpc* NICs generally get real MAC addresses from ofw data, IIRC, so they don't use hostuuid in early boot either. [edit to add: also, a lot (most?) of ppc can't use loader]

This revision was automatically updated to reflect the committed changes.