| Message ID | 1392847670-7053-1-git-send-email-john.stultz@linaro.org |
|---|---|
| State | New |
| Headers | show |
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 0aa4ce8..0b5ef8c 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -539,7 +539,11 @@ int timekeeping_inject_offset(struct timespec *ts) struct timespec tmp; int ret = 0; - if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC) + /* Disallow any {1,-500} style timespecs */ + if ((ts->tv_sec > 0) && ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC)) + return -EINVAL; + /* While interval may be negative, should still be sane */ + if (abs(ts->tv_nsec) >= NSEC_PER_SEC) return -EINVAL; raw_spin_lock_irqsave(&timekeeper_lock, flags);
Kay, Richard, Scratched out a fix for this below and it seems to pass my trivial test, which you can find here: https://raw2.github.com/johnstultz-work/timetests/master/adj-setoffset.c I need to validate that it doesn't cause other problems in the time accounting code, but wanted to send it out for your initial thoughts and any further testing you might have before I send it to a wider audience. thanks -john The adjtimex() ADJ_SETOFFSET feature allows a offset in timespec format to be added to the current time. This value can be positive or negative. However, representing a negative value in a timespec can be confusing, as there may be numerous ways to represent the same amount of time. Positive values are most obviously represented: 1) { 0, 500000000} 2) { 3, 0} 3) { 3, 500000000} While negative values are more complex and confusing: 4) {-5, 0} 5) { 0, -500000000} 6) {-5, -500000000} 7) {-6, 500000000} Note that the last two values (#6 and #7) actually represent the same amount of time. In timekeeping_inject_offset, a naieve validation check on the timespec value resulted in -EINVAL being returned if the nsec portion of the timespec was negative. This resulted in the ADJ_SETOFFSET interface to consider examples #5 and #6 above as invalid, forcing users to use the more awkward representations of {sec - 1, NSEC_PER_SEC + nsec} for negative values (like example #7 above. Initially I suspected the extra logic for underflow handling was the reason this was done, but in looking at it, set_normalized_timespec() handles both overflows and underflows properly. Thus this patch allows for all of the representations above to be considered valid. There is of course, one missing example from the list above: 8) { 4, -500000000} But I consider this is too ackward and misformed to be reasonably supported, so it will still trigger EINVAL errors. Cc: Richard Cochran <richardcochran@gmail.com> Cc: Kay Sievers <kay@vrfy.org> Reported-by: Kay Sievers <kay@vrfy.org> Signed-off-by: John Stultz <john.stultz@linaro.org> --- kernel/time/timekeeping.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)