From 4daedc31f7453bdd8844519948cc1b4197096295 Mon Sep 17 00:00:00 2001 From: Jonas 'Sortie' Termansen Date: Sun, 20 Jun 2021 22:44:19 +0200 Subject: [PATCH] Fix handling of overflow and non-canonical values in timespec APIs. Support zero relative and absolute times in the timer API. --- kernel/alarm.cpp | 10 +++- kernel/descriptor.cpp | 4 +- kernel/disk/ahci/port.cpp | 7 +-- kernel/include/sortix/kernel/timer.h | 3 +- kernel/poll.cpp | 4 +- kernel/timer.cpp | 11 +++- kernel/user-timer.cpp | 16 +++++- libc/include/timespec.h | 26 +++++----- libc/timespec/timespec.c | 76 ++++++++++++++++++++++++---- libc/unistd/usleep.c | 4 +- 10 files changed, 125 insertions(+), 36 deletions(-) diff --git a/kernel/alarm.cpp b/kernel/alarm.cpp index 74eb4134..3d331fd4 100644 --- a/kernel/alarm.cpp +++ b/kernel/alarm.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2012, 2013 Jonas 'Sortie' Termansen. + * Copyright (c) 2011, 2012, 2013, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -47,9 +47,15 @@ int sys_alarmns(const struct timespec* user_delay, struct timespec* user_odelay) struct itimerspec delay, odelay; if ( !CopyFromUser(&delay.it_value, user_delay, sizeof(*user_delay)) ) return -1; + if ( !timespec_is_canonical(delay.it_value) ) + return errno = EINVAL, -1; delay.it_interval = timespec_nul(); - process->alarm_timer.Set(&delay, &odelay, 0, alarm_handler, (void*) process); + int flags = 0; + if ( !delay.it_value.tv_sec && !delay.it_value.tv_nsec ) + flags |= TIMER_DISARM; + + process->alarm_timer.Set(&delay, &odelay, flags, alarm_handler, process); if ( !CopyToUser(user_odelay, &odelay.it_value, sizeof(*user_odelay)) ) return -1; diff --git a/kernel/descriptor.cpp b/kernel/descriptor.cpp index 2242e0d3..f7f8791a 100644 --- a/kernel/descriptor.cpp +++ b/kernel/descriptor.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2013, 2014, 2015, 2016, 2017 Jonas 'Sortie' Termansen. + * Copyright (c) 2012-2017, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -579,7 +579,7 @@ ssize_t Descriptor::pwritev(ioctx_t* ctx, const struct iovec* iov_ptr, static inline bool valid_utimens_timespec(struct timespec ts) { - return ts.tv_nsec < 1000000000 || + return (0 <= ts.tv_nsec && ts.tv_nsec < 1000000000) || ts.tv_nsec == UTIME_NOW || ts.tv_nsec == UTIME_OMIT; } diff --git a/kernel/disk/ahci/port.cpp b/kernel/disk/ahci/port.cpp index 35d35657..5592e2a8 100644 --- a/kernel/disk/ahci/port.cpp +++ b/kernel/disk/ahci/port.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2014, 2015, 2016 Jonas 'Sortie' Termansen. + * Copyright (c) 2013, 2014, 2015, 2016, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -52,9 +52,10 @@ static inline void ahci_port_flush(volatile struct port_regs* port_regs) (void) port_regs->pxcmd; } -static inline void delay(int usecs) +static inline void delay(unsigned int usecs) { - struct timespec delay = timespec_make(usecs / 1000000, usecs * 1000); + struct timespec delay = + timespec_make(usecs / 1000000, (usecs % 1000000) * 1000); Clock* clock = Time::GetClock(CLOCK_BOOT); clock->SleepDelay(delay); } diff --git a/kernel/include/sortix/kernel/timer.h b/kernel/include/sortix/kernel/timer.h index 20734c48..ac85a633 100644 --- a/kernel/include/sortix/kernel/timer.h +++ b/kernel/include/sortix/kernel/timer.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2016, 2017 Jonas 'Sortie' Termansen. + * Copyright (c) 2013, 2016, 2017, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -57,6 +57,7 @@ static const int TIMER_FUNC_ADVANCE_THREAD = 1 << 4; // otherwise delay the destruction until the timer handler, which also grabs the // mutex and checks whether object destruction is supposed to happen. static const int TIMER_FUNC_MAY_DEALLOCATE_TIMER = 1 << 5; +static const int TIMER_DISARM = 1 << 6; class Timer { diff --git a/kernel/poll.cpp b/kernel/poll.cpp index 7c787085..fc3f431d 100644 --- a/kernel/poll.cpp +++ b/kernel/poll.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2014, 2015, 2018 Jonas 'Sortie' Termansen. + * Copyright (c) 2012, 2014, 2015, 2018, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -177,6 +177,8 @@ static bool FetchTimespec(struct timespec* dest, const struct timespec* user) dest->tv_nsec = 0; else if ( !CopyFromUser(dest, user, sizeof(*dest)) ) return false; + else if ( !timespec_is_canonical(*dest) ) + return errno = EINVAL, -1; return true; } diff --git a/kernel/timer.cpp b/kernel/timer.cpp index 09d9d22e..0fe48705 100644 --- a/kernel/timer.cpp +++ b/kernel/timer.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2016, 2017 Jonas 'Sortie' Termansen. + * Copyright (c) 2013, 2016, 2017, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -129,14 +129,21 @@ void Timer::Set(struct itimerspec* new_value, struct itimerspec* old_value, GetInternal(old_value); // Arm the timer if a value was specified. - if ( timespec_lt(timespec_nul(), new_value->it_value) ) + if ( !(new_flags & TIMER_DISARM) ) { value = *new_value; flags = new_flags; callback = new_callback; user = new_user; + if ( !(flags & TIMER_ABSOLUTE) && value.it_value.tv_sec < 0 ) + value.it_value = timespec_nul(); clock->Register(this); } + else + { + value.it_value = timespec_nul(); + value.it_interval = timespec_nul(); + } clock->UnlockClock(); } diff --git a/kernel/user-timer.cpp b/kernel/user-timer.cpp index 6f61ff6f..e0a653b2 100644 --- a/kernel/user-timer.cpp +++ b/kernel/user-timer.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2014 Jonas 'Sortie' Termansen. + * Copyright (c) 2013, 2014, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -194,6 +194,10 @@ int sys_timer_settime(timer_t timerid, if ( !CopyFromUser(&value, user_value, sizeof(value)) ) return -1; + if ( !timespec_is_canonical(value.it_value) || + !timespec_is_canonical(value.it_interval) ) + return errno = EINVAL, -1; + if ( timespec_lt(value.it_value, timespec_nul()) || timespec_lt(value.it_interval, timespec_nul()) || (flags & ~(TIMER_ABSTIME)) != 0 ) @@ -202,6 +206,8 @@ int sys_timer_settime(timer_t timerid, int timer_flags = 0; if ( flags & TIMER_ABSTIME ) timer_flags |= TIMER_ABSOLUTE; + if ( !value.it_value.tv_sec && !value.it_value.tv_nsec ) + timer_flags |= TIMER_DISARM; timer->Set(&value, &ovalue, timer_flags, timer_callback, user_timer); @@ -240,6 +246,11 @@ int sys_clock_settimeres(clockid_t clockid, (res && !CopyFromUser(&kres, res, sizeof(kres))) ) return -1; + if ( time && !timespec_is_canonical(ktime) ) + return errno = EINVAL, -1; + if ( res && !timespec_is_canonical(kres) ) + return errno = EINVAL, -1; + clock->Set(time ? &ktime : NULL, res ? &kres : NULL); return 0; @@ -259,6 +270,9 @@ int sys_clock_nanosleep(clockid_t clockid, if ( !CopyFromUser(&time, user_duration, sizeof(time)) ) return -1; + if ( !timespec_is_canonical(time) ) + return errno = EINVAL, -1; + time = flags & TIMER_ABSTIME ? clock->SleepUntil(time) : clock->SleepDelay(time); diff --git a/libc/include/timespec.h b/libc/include/timespec.h index d4a7dc7e..ddbbc946 100644 --- a/libc/include/timespec.h +++ b/libc/include/timespec.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013 Jonas 'Sortie' Termansen. + * Copyright (c) 2013, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -28,25 +28,22 @@ #include #endif -#ifdef __cplusplus -extern "C" { -#endif - #ifndef __time_t_defined #define __time_t_defined typedef __time_t time_t; #endif -#ifdef __cplusplus -} /* extern "C" */ -#endif - #include #ifdef __cplusplus extern "C" { #endif +static __inline bool timespec_is_canonical(struct timespec t) +{ + return 0 <= t.tv_nsec && t.tv_nsec <= 999999999; +} + static __inline bool timespec_eq(struct timespec a, struct timespec b) { return a.tv_sec == b.tv_sec && a.tv_nsec == b.tv_nsec; @@ -91,6 +88,9 @@ static __inline struct timespec timespec_make(time_t sec, long nsec) static __inline struct timespec timespec_neg(struct timespec t) { + if ( t.tv_sec == __TIME_MIN ) + return timespec_make(__TIME_MAX, + !t.tv_nsec ? 999999999 : 1000000000 - t.tv_nsec); if ( t.tv_nsec ) return timespec_make(-t.tv_sec - 1, 1000000000 - t.tv_nsec); return timespec_make(-t.tv_sec, 0); @@ -101,9 +101,11 @@ static __inline struct timespec timespec_nul(void) return timespec_make(0, 0); } -struct timespec timespec_canonalize(struct timespec t); -struct timespec timespec_add(struct timespec a, struct timespec b); -struct timespec timespec_sub(struct timespec a, struct timespec b); +struct timespec timespec_canonalize(struct timespec); +struct timespec timespec_add(struct timespec, struct timespec); +struct timespec timespec_sub(struct timespec, struct timespec); +bool timespec_add_overflow(struct timespec, struct timespec, struct timespec*); +bool timespec_sub_overflow(struct timespec, struct timespec, struct timespec*); #ifdef __cplusplus } /* extern "C" */ diff --git a/libc/timespec/timespec.c b/libc/timespec/timespec.c index f40248b2..e6c30209 100644 --- a/libc/timespec/timespec.c +++ b/libc/timespec/timespec.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013 Jonas 'Sortie' Termansen. + * Copyright (c) 2013, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -17,11 +17,14 @@ * Utility functions for manipulation of struct timespec. */ +#include + +#include #include static const long NANOSECONDS_PER_SECOND = 1000000000L; -// TODO: The C modulo operator doesn't do exactly what we desire. +// The C modulo operator doesn't do exactly what we desire. static long proper_modulo(long a, long b) { if ( 0 <= a ) @@ -37,19 +40,72 @@ struct timespec timespec_canonalize(struct timespec t) return t; } +bool timespec_add_overflow(struct timespec a, + struct timespec b, + struct timespec* res) +{ + assert(timespec_is_canonical(a) && timespec_is_canonical(b)); + struct timespec ret; + ret.tv_nsec = a.tv_nsec + b.tv_nsec; + if ( NANOSECONDS_PER_SECOND <= ret.tv_nsec ) + { + ret.tv_nsec -= NANOSECONDS_PER_SECOND; + if ( a.tv_sec != TIME_MAX ) + a.tv_sec++; + else if ( b.tv_sec != TIME_MAX ) + b.tv_sec++; + else + goto overflow; + } + if ( __builtin_add_overflow(a.tv_sec, b.tv_sec, &ret.tv_sec) ) + goto overflow; + *res = ret; + return false; +overflow: + *res = b.tv_sec < 0 ? + timespec_make(TIME_MIN, 0) : + timespec_make(TIME_MAX, NANOSECONDS_PER_SECOND - 1); + return true; +} + struct timespec timespec_add(struct timespec a, struct timespec b) { struct timespec ret; - a = timespec_canonalize(a); - b = timespec_canonalize(b); - ret.tv_sec = a.tv_sec + b.tv_sec; - ret.tv_nsec = a.tv_nsec + b.tv_nsec; - return timespec_canonalize(ret); + timespec_add_overflow(a, b, &ret); + return ret; +} + +bool timespec_sub_overflow(struct timespec a, + struct timespec b, + struct timespec* res) +{ + assert(timespec_is_canonical(a) && timespec_is_canonical(b)); + struct timespec ret; + ret.tv_nsec = a.tv_nsec - b.tv_nsec; + if ( ret.tv_nsec < 0 ) + { + ret.tv_nsec += NANOSECONDS_PER_SECOND; + if ( a.tv_sec != TIME_MIN ) + a.tv_sec--; + else if ( b.tv_sec != TIME_MAX ) + b.tv_sec++; + else + goto overflow; + } + if ( __builtin_sub_overflow(a.tv_sec, b.tv_sec, &ret.tv_sec) ) + goto overflow; + *res = ret; + return false; +overflow: + *res = b.tv_sec >= 0 ? + timespec_make(TIME_MIN, 0) : + timespec_make(TIME_MAX, NANOSECONDS_PER_SECOND - 1); + return true; } struct timespec timespec_sub(struct timespec a, struct timespec b) { - a = timespec_canonalize(a); - b = timespec_canonalize(b); - return timespec_add(a, timespec_neg(b)); + struct timespec ret; + timespec_sub_overflow(a, b, &ret); + return ret; } diff --git a/libc/unistd/usleep.c b/libc/unistd/usleep.c index 453f3030..e753bb4c 100644 --- a/libc/unistd/usleep.c +++ b/libc/unistd/usleep.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013 Jonas 'Sortie' Termansen. + * Copyright (c) 2013, 2021 Jonas 'Sortie' Termansen. * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -23,6 +23,6 @@ int usleep(useconds_t usecs) { - struct timespec delay = timespec_canonalize(timespec_make(0, usecs * 1000)); + struct timespec delay = timespec_make(usecs / 1000, (usecs % 1000) * 1000); return nanosleep(&delay, NULL); }