Validate time when setting RTC

[asomers]

Summary

Previously a debug kernel would panic, and a non-debug kernel would silently
allow integer overflows when setting the RTC. Now, settimeofday will
still set the in-memory clock, but it won't change the RTC.

Also,

• Tolerate leap seconds in vrtc.
• Don't do a BCD conversion in ddf_meta_create unless we really need to.
• Print RTC failures even if debug.bootverbose = 0

PR: 267157
Reported by: Jan Schaumann <[e-mail]>

Test Plan

manual testing

[corvink]

sys/amd64/vmm/io/vrtc.c

212–225

The values are already validated by clock_ts_to_ct. In my opinion, we can drop those checks to avoid checking them twice.

sys/kern/subr_clock.c

270

[asomers]

sys/amd64/vmm/io/vrtc.c

212–225

That was my first impulse. OTOH, there's no guarantee that clock_ts_to_ct will continue to validate them the same way that this function does (and you'll notice that it doesn't validate year quite the same way). And since these are only assertions, they don't even get included in production builds, so I left them in.

sys/kern/subr_clock.c

270

Good catch.

[asomers]

• Fix an assertion spotted by Corvin Köhne

[corvink]

sys/amd64/vmm/io/vrtc.c

212–225

clock_ts_to_ct should always return a valid clocktime if it succeeds. If the vrtc isn't able to handle a valid clocktime, it shouldn't ask for it.

The asserts are already included. So, it's fine to keep them. We could just think of removing these unneccessary checks while we're already here.

[asomers]

sys/amd64/vmm/io/vrtc.c

212–225

What do you mean by "keep the asserts", but "remove the unneccessary checks"? What's the difference between checks and asserts?

[imp]

Really don't like the panics that I highlighted.
Leap seconds aren't a thing that time_t can encode, so we can't ever extract a leap second from a timespec.

sys/amd64/vmm/io/vrtc.c

209

could this be reached from setting the date via the command line? If so, then it shouldn't panic.
Also, if this is in the boot path, it will keep us from booting at all.

212

This is wrong. It can never be 60 here. time_t has no ability to represent a leap second since POSIX does not define an encoding for it, let alone a unique one (There's a formula in the standard, but it's specifically not for leap seconds).

sys/dev/smartpqi/smartpqi_misc.c

42

So how does this fail? Especially since we truncate the year a few lines below to 25500 years.
Since this is vendor code (I know it's in the wrong place in the tree for that, but it is), and since there's other bugs, I'd suggest leaving it alone and reporting it upstream.

sys/geom/raid/md_ddf.c

715

Panic here means we can't boot.

867

panic here means we can't boot.

sys/kern/subr_clock.c

290

No. It does not. In fact, it cannot because time_t doesn't encode leap seconds. There's no defined time_t encoding for a leap second.

291

59. Though this can never fire after the rsec = rsec % 60

[asomers]

sys/amd64/vmm/io/vrtc.c

209

As far as I can tell, no. I think only BHyve uses this. And this panic isn't new; there was always a panic in this path, at least in INVARIANTS builds.

212

Allright, I'll adjust all of the assertions and checks to exclude the possibility of leap seconds.

sys/dev/smartpqi/smartpqi_misc.c

42

Even though the RTC can't be set to a year greater than 10,000 AD, the system clock can be. So getnanouptime could return 1.4 billion AD or something like that. In that case, the clocktime structure won't be initialized. And even though there's no precedent for this function returning an error, there is for its caller. So I think it's ok to return an error here. The only alternatives would be to panic, or to zero the structure, or something like that.
Do you know where the upstream source is?

sys/geom/raid/md_ddf.c

715

Yeah, but it paniced before this change too, at least when INVARIANTS were on. It's easy enough to fix though, so I'll do it.

867

Same as above, this line paniced before too. And I'll fix it too.

sys/kern/subr_clock.c

291

As a matter of fact, it should be impossible for any of the checks to fail except for the year one. I'll remove them all, but restore the KASSERTs.

[asomers]

• Respond to @imp's comments

[corvink]

sys/amd64/vmm/io/vrtc.c

212–225

Sry, for being unclear. We can think of removing the asserts because we're touching this code but if we keep them, it's fine.