-
Notifications
You must be signed in to change notification settings - Fork 339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hwclock: fix the calls to settimeofday of glibc #479
base: master
Are you sure you want to change the base?
Conversation
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.
Info: |
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? |
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.] |
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". |
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? |
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. |
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. |
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"... |
the "seconds" in |
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 The "kernel timezone" thing done by |
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. |
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 BTW, per the man page:
|
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 |
Did commit da1474b 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. |
AFAICT it will not compile on riscv32. If you insist on doing this you should at least condition it on |
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.