The following commit has been merged into the timers/urgent branch of tip:
Commit-ID: 39ff83f2f6cc5cc1458dfcea9697f96338210beb Gitweb: https://git.kernel.org/tip/39ff83f2f6cc5cc1458dfcea9697f96338210beb Author: Lukas Hannen lukas.hannen@opensource.tttech-industrial.com AuthorDate: Wed, 25 Aug 2021 10:12:43 Committer: Thomas Gleixner tglx@linutronix.de CommitterDate: Wed, 08 Sep 2021 17:44:26 +02:00
time: Handle negative seconds correctly in timespec64_to_ns()
timespec64_ns() prevents multiplication overflows by comparing the seconds value of the timespec to KTIME_SEC_MAX. If the value is greater or equal it returns KTIME_MAX.
But that check casts the signed seconds value to unsigned which makes the comparision true for all negative values and therefore return wrongly KTIME_MAX.
Negative second values are perfectly valid and required in some places, e.g. ptp_clock_adjtime().
Remove the cast and add a check for the negative boundary which is required to prevent undefined behaviour due to multiplication underflow.
Fixes: cb47755725da ("time: Prevent undefined behaviour in timespec64_to_ns()")' Signed-off-by: Lukas Hannen lukas.hannen@opensource.tttech-industrial.com Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/AM6PR01MB541637BD6F336B8FFB72AF80EEC69@AM6PR01MB54... --- include/linux/time64.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/linux/time64.h b/include/linux/time64.h index 5117cb5..81b9686 100644 --- a/include/linux/time64.h +++ b/include/linux/time64.h @@ -25,7 +25,9 @@ struct itimerspec64 { #define TIME64_MIN (-TIME64_MAX - 1)
#define KTIME_MAX ((s64)~((u64)1 << 63)) +#define KTIME_MIN (-KTIME_MAX - 1) #define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) +#define KTIME_SEC_MIN (KTIME_MIN / NSEC_PER_SEC)
/* * Limits for settimeofday(): @@ -124,10 +126,13 @@ static inline bool timespec64_valid_settod(const struct timespec64 *ts) */ static inline s64 timespec64_to_ns(const struct timespec64 *ts) { - /* Prevent multiplication overflow */ - if ((unsigned long long)ts->tv_sec >= KTIME_SEC_MAX) + /* Prevent multiplication overflow / underflow */ + if (ts->tv_sec >= KTIME_SEC_MAX) return KTIME_MAX;
+ if (ts->tv_sec <= KTIME_SEC_MIN) + return KTIME_MIN; + return ((s64) ts->tv_sec * NSEC_PER_SEC) + ts->tv_nsec; }
Committer: Thomas Gleixner tglx@linutronix.de CommitterDate: Wed, 08 Sep 2021 17:44:26 +02:00
time: Handle negative seconds correctly in timespec64_to_ns()
timespec64_ns() prevents multiplication overflows by comparing the seconds value of the timespec to KTIME_SEC_MAX. If the value is greater or equal it returns KTIME_MAX.
But that check casts the signed seconds value to unsigned which makes the comparision true for all negative values and therefore return wrongly KTIME_MAX.
Negative second values are perfectly valid and required in some places, e.g. ptp_clock_adjtime().
Remove the cast and add a check for the negative boundary which is required to prevent undefined behaviour due to multiplication underflow.
Fixes: cb47755725da ("time: Prevent undefined behaviour in timespec64_to_ns()")' Signed-off-by: Lukas Hannen lukas.hannen@opensource.tttech-industrial.com Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/AM6PR01MB541637BD6F336B8FFB72AF80EEC69@AM6PR01MB54... abs.com
include/linux/time64.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/linux/time64.h b/include/linux/time64.h index 5117cb5..81b9686 100644 --- a/include/linux/time64.h +++ b/include/linux/time64.h @@ -25,7 +25,9 @@ struct itimerspec64 { #define TIME64_MIN (-TIME64_MAX - 1)
#define KTIME_MAX ((s64)~((u64)1 << 63)) +#define KTIME_MIN (-KTIME_MAX - 1) #define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) +#define KTIME_SEC_MIN (KTIME_MIN / NSEC_PER_SEC)
/*
- Limits for settimeofday():
@@ -124,10 +126,13 @@ static inline bool timespec64_valid_settod(const struct timespec64 *ts) */ static inline s64 timespec64_to_ns(const struct timespec64 *ts) {
- /* Prevent multiplication overflow */
- if ((unsigned long long)ts->tv_sec >= KTIME_SEC_MAX)
/* Prevent multiplication overflow / underflow */
if (ts->tv_sec >= KTIME_SEC_MAX) return KTIME_MAX;
if (ts->tv_sec <= KTIME_SEC_MIN)
return KTIME_MIN;
return ((s64) ts->tv_sec * NSEC_PER_SEC) + ts->tv_nsec;
}
Adding tv_nsec can still overflow - even if tv_nsec is bounded to +/- 1 second. This is no more 'garbage in' => 'garbage out' than the code without the multiply under/overflow check.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
David,
On Wed, Sep 08 2021 at 16:01, David Laight wrote:
- if (ts->tv_sec <= KTIME_SEC_MIN)
return KTIME_MIN;
- return ((s64) ts->tv_sec * NSEC_PER_SEC) + ts->tv_nsec;
}
Adding tv_nsec can still overflow - even if tv_nsec is bounded to +/- 1 second. This is no more 'garbage in' => 'garbage out' than the code without the multiply under/overflow check.
In kernel timespecs are always normalized: 0 < tv_nsec < 1e9 - 1
Let's do the math:
KTIME_SEC_MAX = KTIME_MAX / NSEC_PER_SEC
The overflow prevention does:
if PSVAL >= KTIME_SEC_MAX: return KTIME_MAX
so the largest positive seconds value which passes the above is:
PSMAX = KTIME_SEC_MAX - 1
ergo:
PSMAX * NSEC_PER_SEC + (NSEC_PER_SEC - 1) < KTIME_SEC_MAX < KTIME_MAX
I leave the proof for negative values as an excercise for the reader.
Thanks,
tglx --- "Math is hard, let's go shopping!" - John Stultz
linux-stable-mirror@lists.linaro.org