svn commit: r337334 - head/lib/libc/sys

[Bruce Evans]

On Sun, 5 Aug 2018, Ian Lepore wrote:

> On Sat, 2018-08-04 at 16:39 -0600, Warner Losh wrote:
>>> [Context lost to corruption of spaces}
>>
>> Specifically, for compatibility, we store minutes west of UTC on
>> settimeofday and we retrieve it for settimeofday. Otherwise it's 100%
>> unused by anything else at all in the system. Well, technically, we use it
>> for utc_offset, we don't really use that elsewhere (one can find references
>> in utc_offset, but usually we set this via adjkerntz, which is actively
>> used in the system). tz_minuteswest likely should just be removed, and the
>> argument to settimzeofday should just be completely ignored.

I already described how adjkerntz sets both tz_minuteswest and other more
cryptic variables used in utc_offset().

> utc_offset is used by all RTC drivers, and by the convenience routines
> that convert times to/from FAT filesystem format (which I assume are
> used by FAT filesystem code, but I haven't checked that). So if the tz
> info passed to settimeofday() ends up in utc_offset, how can we be sure
> nobody is relying on that?

I forgot about FAT times, and only remembered that utc_offset() was used
for RTC drivers.  I thought that utc_offset() is 0 unless something sets
the timezone to nonzero or the RTC is on local time (wall_cmos_clock case).
However, since the kernel needs the timezone for FAT times, it must be
known even if the RTC is on UTC time.  Now I don't see how FAT times can
even work unless the wall_cmos_clock.  They are just the UTC minus
utc_offset(), where utc_offset() is

 	(tz_minuteswest * 60 + (wall_cmos_clock ? adjkerntz : 0))

Here wall_cmos_clock is only for managing the RTC offset.  It must be 0
unless the RTC is on local time.  But then either FAT times or RTC times
must be broken except in the Greenwich meridian.  Otherwise, tz_minuteswest
must be nonzero to ajust FAT times right, but this makes utc_offset()
unusable for the RTC offset.

The timezone and setting it using settimeofday() is the correct API for
implementing all of this (it only has minutes resolution, but no more is
needed).  The correct offsets are then:

 	rtc_offset() = wall_cmos_clock ? tz_minuteswest * 60 : 0;
 	fat_offset() = tz_minuteswest * 60;

where tz_minuteswest must include the dst offset so it needs a daemon
like adjkerntz(8) to manage it even if wall_cmos_clock == 0.  I think
adjkerntz already does this management, and it works in the
wall_cmos_clock != 0 case (except for missing locking of all 3 of the
variables, and fundamental races updating them all and also adjusting
the RTC on every dst change).  FAT times work in this case.  I think
FAT times just don't work if wall_cmos_clock == 0.

I think the extra variables are to try to manage this complexity, but
the just get in the way.  wall_cmos_clock is obviously needed.  But
the extra offset can't do anything good.  It might have helped on
non-preemptible UP systems where locking was not needed.  It allowed
adjusting the offset without going near settimeofday().  But now non-
broken locking would need to use the same lock for settimeofday() and
the machdep.adjkerntz sysctl.  It is easier to just lock settimeofday().

>> So while hyper technically, one could use this, nobody does, nor has since
>> between 4.3 and 4.4 when it was realized that storing the timezone in the
>> kernel was a really stupid idea. That's what the language used in the man
>> page that you removed was trying to say.

adjkerntz(8) uses this.  Storing the timezone in the kernel is an essential
idea.  Otherwise, every FAT time calculation would have to call userland to
ask for the timezone.  Similarly for RTC offsets, except those don't need to
be calculated very often.

adjkerntz(8) can be simplified by only using 1 offset, but that offset should
be the timezone set by settimeofday() and not a sysctl with even more broken
locking.

My program for demonstrating overflow bugs and panics in RTC code uses this.
It could use either the timezone or machdep.adjkerntz to set a bad offset,
but the timezone is easier to use.  Bad offsets can be as little as 1 second,
to convert the Epoch time (time_t)0 to the invalid time (time_t)-1.  The
program uses a timezone of +-24 hours to clobber any more reasonable offset
in machdep.adjkerntz.

Bruce