Bug 6761
| Summary: | adjtimex broken when delta is NULL | ||
|---|---|---|---|
| Product: | Timers | Reporter: | Michael Kerrisk (michael.kerrisk) |
| Component: | Other | Assignee: | john stultz (john.stultz) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | akpm, drepper, ismail, mingo, protasnb, stsp2, tglx, zippel |
| Priority: | P2 | ||
| Hardware: | i386 | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.17 | Subsystem: | |
| Regression: | --- | Bisected commit-id: | |
| Attachments: | Add ADJ_OFFSET_SS_READ | ||
Looking over the context, it does seem like the fix is a bit more complicated. It looks like this would require both a kernel and glibc change (this issue appears to stem from sys_adjtimex's multiplexing of current adjtimex interface along w/ the old adjtime interface). I'm worried that the patch below would fix the old adjtime interface but break the adjtimex interface. Yes, for all I know the patch I gave may well be insufficient. I don't understand the code there well enough. Nevertheless, things do seem to be broken as they stand, and some type of fix is required... What is the current state of this issue, will fix be needed eventually? it seems like for portablity adjtime() should be handled properly. Are there any plans to find a solution? Thanks. guys, that pain in your ribs is my thumb ;) :( The issue is the adjtimex interface multiplexes both the old adjtime and new adjtimex interfaces.
The problem being, if nothing is passed in on the modes, you get the get new adjtimex()'s offset value, not the adjtime offset.
In order to get the current adjtime offset value, you have to use the ADJ_OFFSET_SINGLESHOT mode, which overwrites the current adjtime offset (and returns the old).
The only fix I can imagine for this would be to change the adjtimex interface and introduce a new modes flag, something like:
#define ADJ_OFFSET_SINGLESHOT_READ 0x2000
And we can add a simple:
if (txc->modes == ADJ_OFFSET_SINGLESHOT_READ)
txc->offset = saved_adjust;
Then glibc can pass it in if adjtime() is called w/ no new adjustment value.
Outside of that I'm not sure I see how to solve this.
Created attachment 12184 [details]
Add ADJ_OFFSET_SS_READ
Untested patch to add ADJ_OFFSET_SS_READ modes flag. How does this sound:
When passed in the modes field to adjtimex() we will return the current singleshot (adjtime style) adjustment in the timex.offset field without modifying the kernel's time_adjust value.
Needs glibc to use this flag in its adjtime() implementation in the case where the new offset is null and the old offset is non-null.
It should be noted that the existing behavior is documented in the man page for adjtime():
"BUGS
Currently, if delta is specified as NULL, no valid information about
the outstanding clock adjustment is returned in olddelta. (In this
circumstance, adjtime() should return the outstanding clock adjustment,
without changing it.) This is the result of a kernel limitation."
(In reply to comment #7) > It should be noted that the existing behavior is documented in the man page > for > adjtime(): > "BUGS > Currently, if delta is specified as NULL, no valid information > about > the outstanding clock adjustment is returned in olddelta. (In > this > circumstance, adjtime() should return the outstanding clock > adjustment, > without changing it.) This is the result of a kernel limitation." Yes, it's documented in the man page because as well as creating this bug report, I also wrote the man page text. *** Bug 7491 has been marked as a duplicate of this bug. *** Sorry, disregard #9, it was operator error. Ulrich, what do you think about this new proposed ADJ_OFFSET_SINGLESHOT_READ modes flag? glibc would use this flag in its adjtime() implementation in the case where the new offset is null and the old offset is non-null. old kernels just ignore unknown mode flags and return TIME_OK. If this is OK to you then we can add this fix to the upstream kernel. We've added this to the x86 tree's "still cooking" queue of patches. This means it will show up in -mm for testing, but it will need Ulrich's ACK before it can go upstream. Kernel part [0] and Glibc part [1] is done, time to close this bug as fixed. [0] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=52bfb36050c8529d9031d2c2513b281a360922ec [1] http://sourceware.org/cgi-bin/cvsweb.cgi/libc/sysdeps/unix/sysv/linux/adjtime.c.diff?cvsroot=glibc&r1=1.13&r2=1.14 great - i'm closing it. Thanks to everyone involved. |
Most recent kernel where this bug did not occur: occurs in all kernels (also in 2.6.17) Distribution: n/a (most recently tested on SUSE 10.0) Hardware Environment: x86 (and probably all others) Software Environment: Problem Description: A long standing bug in the adjtimex() system call causes glibc's adjtime(3) function to deliver the wrong results if 'delta' is NULL. The adjtime(3) manual page says If olddelta is not NULL, then the buffer that it points to is used to return the amount of time remaining from any previous adjustment that has not yet been completed. The FreeBSD manual pages says similar: If olddelta is not a null pointer, the structure pointed to will contain, upon return, the number of microseconds still to be corrected from the earlier call. This information should be returned in 'olddelta' regardless of whether the 'delta' argument is or is not NULL. However, Linux/glibc only returns valid information in 'olddelta' if 'delta' is non-NULL; in other words it is only possible to enquire about 'olddelta' if we at the same time change 'delta'. FreeBSD does not have this limitation. The problem lies in the implementation of adjtimex(). I earlier suggested a fix for this problem: --- time.c.orig 2006-03-12 11:03:10.000000000 +1300 +++ time.c 2006-03-12 11:04:26.000000000 +1300 @@ -375,7 +375,9 @@ result = TIME_ERROR; - if ((txc->modes & ADJ_OFFSET_SINGLESHOT) == ADJ_OFFSET_SINGLESHOT) + if(txc->modes == 0) + txc->offset = time_adjust; + else if ((txc->modes & ADJ_OFFSET_SINGLESHOT) == ADJ_OFFSET_SINGLESHOT) txc->offset = save_adjust; else { txc->offset = shift_right(time_offset, SHIFT_UPDATE); (the patch line numbers are wrong for 2.6.17, but the fix is the same.) I made that suggestion in 2002: http://marc.theaimsgroup.com/?l=linux-kernel&m=102404614411225&w=2 Some follow up acknowledged the problem but implied that the fix is more complex (and put off until later): http://marc.theaimsgroup.com/?l=linux-kernel&m=103674954619484&w=2 http://marc.theaimsgroup.com/?l=linux-kernel&m=103689681229548&w=2 But later has never come, and the problem still exists. It should be fixed. There is also a relevant glibc report that I made to get further background on this point: http://sourceware.org/bugzilla/show_bug.cgi?id=2449 . Steps to reproduce: This problem can be demonstrated with the following program: /* t_adjtime.c */ #define _GNU_SOURCE #include <sys/time.h> #include <sys/types.h> #include <stdio.h> #include <stdlib.h> #include <unistd.h> int main(int argc, char *argv[]) { struct timeval delta, oldDelta, *deltap; oldDelta.tv_sec = oldDelta.tv_usec = 0; delta.tv_sec = (argc > 1) ? atoi(argv[1]) : 0; delta.tv_usec = (argc > 2) ? atoi(argv[2]) : 0; if (argc > 1) deltap = δ else deltap = NULL; if (adjtime(deltap, &oldDelta) == -1) { perror("adjtime"); exit(EXIT_FAILURE); } printf("old delta %ld.%06ld\n", oldDelta.tv_sec, oldDelta.tv_usec); exit(EXIT_SUCCESS); } /* main */ If given command-line arguments, this program uses them to initialise 'delta'. If no arguments are supplied then delta is specified as NULL. In either case, the program prints the information returned in 'olddelta'. On Linux we see something like the following: # ./t_adjtime 2 old delta 0.000000 # ./t_adjtime old delta 0.000000 The last line of output shows the problem. When we do similar on FreeBSD, we see something like the following # ./t_adjtime 2 old delta 0.000000 # ./t_adjtime old delta 1.990000 == END ==