hwclock: fix the calls to settimeofday of glibc

[ryanzmi]

After glibc 2.31, the settimeofday function no longer accept setting the time and the offset together. If both of its two arguments are not null, the call will fail and return EINVAL. Therefore hwclock -s / -t are always failing with newer glibc.

Fix by separating into two calls, which should remain compatibility.

[ryanzmi]

Info: The util-linux has made the similar tweaks to adapt glibc >= 2.31. It also does more explaining. https://github.com/util-linux/util-linux/commit/cd781c405be82540484da3bfe3d3f17a39b8eb5c#diff-0ba721e0b4d88d4296552c06b999ad5b

[landley]

So if I travel from NYC to LA and something updates the clock to display properly for the location, the two would now get updated separately opening a race window where something could check the time and get a value that's 3 hours off, and gnu/libc considers that an improvement?

[enh-google]

yeah, i wasn't aware of the claim that the kernel is considering changing this. i did recently send a patch to clarify these differences in "VERSIONS" in https://man7.org/linux/man-pages/man2/settimeofday.2.html (which has been updated on the web!!!).

bionic never tried to outsmart the kernel, so we've never had any of these oddities. iirc musl just ignores the tz and calls clock_settime() instead.

[EDIT: actually, it was @zijunzhaocck's patch, not mine, found when adding nullability annotations to all of bionic's headers.]

[enh-google]

that is: "setting the tz was already silently broken on musl, and this patch doesn't fix that --- you might want to just use syscall() instead".

[landley]

The problem with using the syscall directly is that the underlying data structure is "struct old_timeval32" which if you unwrap the typedef uses a signed 32 bit integer for seconds, ala the y2038 problem.

clock_settime() uses a 64 bit structure (which is presumably why musl is using it) but doesn't mess with timezone at all.

The kernel's sys_tz field seems to be local to gettimeofday/settimeofday. It's declared and exported in kernel/time/time.c which handles those two syscalls, the clock_gettime() stuff is over in kernel/time/posix-timers.c which does not have the letters "tz" adjacent in the entire file. (Everything modern keeps the clock in UTC and uses the environment variable.) The only users of gettimeofday()/settimeofday() in toybox outside of pending are hwclock (which has a todo comment about switching to the other API).

The reason I haven't switched it yet is that historically, systems that dual boot between windows and linux would fight over whether the hardware clock was set to UTC (almost all linux) or localtime (almost all windows, which was always fun when the power was off during the daylight savings time switch: on 64 bit windows systems you can apparently regedit HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\TimeZoneInformation and add a QWORD entry RealTimeIsUniversal=1 and then do the "your mouse has moved, you must reboot windows for this change to take effect" cycle and then it doesn't work consistently because windows is made entirely of bugs trying to cancel each other out).

I guess switch to the new 64 bit API, rely on the TZ variable, never set the legacy sys_tz at all, and if you want to dual boot with windows do the registry edit over there?

[landley]

Yeah, sys_tz can't be in use on modern kernels because it never got containerized for CLONE_NEWTIME. It looks like a system global property to me that's stored and returned as-is with no processing, and thus largely useful as a way of surreptitiously passing data between containers.

[landley]

Nope, if you grep -rwl linux/* for it, 36 files use it and it's exported in the vdso. Definitely still used when set. And CLONE_NEWTIME is still pretty new (commit 769071ac9f20 added it, but commit f5eded1f5f11 last year made it work with clone) so that missing stuff doesn't mean it's intentional. And current windows still expects the bios clock set to local time by default ala https://www.howtogeek.com/323390/how-to-fix-windows-and-linux-showing-different-times-when-dual-booting/ so the underlying problem is still there... assuming "dual boot with windows" remains a use case anyone cares about?

So some userspace code is using the TZ environment variable, and some is using the kernel sys_tz setting, the only assignment to which is in the settimeofday() code in kernel/time/time.c (everything else reads it) which uses a 32 bit time structure to set the clock.

This seems like a kernel question.

[landley]

Possibly a bigger problem is that RTC_RD_TIME and RTC_SET_TIME ioctls use struct tm with signed 32 bit seconts, and I don't know of another way to get/set the hardware clock from within Linux? Nothing obvious in drivers/rtc/dev.c in current git...

Nevermind: that's the BROKEN DOWN time struct from localtime or gmtime. Because obviously the logical thing to feed the kernel is year, month, day, hours, minutes, seconds, weekday, day of the year, and a tristate daylight savings time indicator (with "I don't know" option). That signed 32 bit seconds is "seconds within current minute"...

[enh-google]

Possibly a bigger problem is that RTC_RD_TIME and RTC_SET_TIME ioctls use struct tm with signed 32 bit seconts, and I don't know of another way to get/set the hardware clock from within Linux? Nothing obvious in drivers/rtc/dev.c in current git...

the "seconds" in struct tm is the number of seconds into a minute, not the number of seconds since 1970... (strictly it's struct rtc_time, but it looks like the moral equivalent of struct tm.)

[richfelker]

The right solution here is to always set hardware clock to UTC and tell Windows you did that if you use Windows. Storing local time in the hardware clock across boots is always, inherently, subject to corruption of the system time if you reboot near the start/end of daylight time. This is independent of whether you're using the "kernel timezone" thing where the kernel restores it, or explicitly restoring it via the hwclock utility.

The "kernel timezone" thing done by settimeofday is a historical GNU/Linux misfeature that musl intentionally did not copy, not only because of the above, but because of a much more insideous thing Linux does if it's been set. Aside from using kernel timezone for hardware clock purposes, it's also used to apply an offset to FAT-based filesystem timestamps; AFAIK there is no way to disable this. This means that file timestamps get corrupted on removable USB media, and breaks older-than relationships between files if you move the media between devices with different timezones.

[landley]

Once again you don't like code that the kernel has, intentionally break it, and demand users change their behavior. Got it.

I'll wrap the system call.

[richfelker]

musl is not a lite glibc clone. It does not attempt to mimic the GNU/Linux system. What it does attempt to do is provide a POSIX-conforming platform with a reasonable set of extensions that are common across other major implementations and that facilitate supporting application software - not that facilitate setting up your system exactly the same way you might be familiar with setting it up using GNU/Linux.

If you want your version of the hwclock utility to use this functionality via raw syscall, that's probably possible, but you should check what happens on natively time64 systems like rv32, where the settimeofday syscall probably doesn't even exist and you might experience build failures. I'm not sure if there's another equivalent method you need to use to get the same functionality.

BTW, per the man page:

No doubt it is a bad idea to use this feature.

[landley]

Rich removed the __NR_settimeofday constant from musl in commit 5a105f19b5aa because he believes people shouldn't have the choice to use kernel features he disapproves of.

[richfelker]

Rich removed the __NR_settimeofday constant from musl in commit 5a105f19b5aa because he believes people shouldn't have the choice to use kernel features he disapproves of.

No I did not. If you bothered to read the text of the commit, all of the syscalls that take time32 arguments were renamed to have a _time32 suffix so that you can continue using them, but so that existing code using them and passing struct timeval/timespec, which is no longer valid, breaks at compile time rather than silently corrupting things. It can then be fixed to pass an explicitly old-format time as long[2].

[landley]

Did commit da1474b1589a fix this for everybody? I haven't tested vfat and similar to make sure they're reacting to kernel time zone, but I still need to re-poke upstream about containerizing that field.

[richfelker]

AFAICT it will not compile on riscv32. If you insist on doing this you should at least condition it on #ifdef __NR_settimeofday. As I've told you before, too, musl is not trying to prevent you from using this syscall and you don't need to #include <asm/unistd.h> to get it. You just need to check for __NR_settimeofday_time32 and use that if present (and since you're passing a null timeval pointer, it doesn't matter that it will not accept a timeval). But using the kernel header like you're doing should work fine too in this specific case.