linux-kernel - RE: [tip: timers/urgent] time: Handle negative seconds correctly in timespec64_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a4bbf640306c42429afda8a4fc396f98@AcuMS.aculab.com>
Date:   Wed, 8 Sep 2021 16:01:43 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-tip-commits@...r.kernel.org" 
        <linux-tip-commits@...r.kernel.org>
CC:     Lukas Hannen <lukas.hannen@...nsource.tttech-industrial.com>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
Subject: RE: [tip: timers/urgent] time: Handle negative seconds correctly in
 timespec64_to_ns()

> Committer:     Thomas Gleixner <tglx@...utronix.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@...nsource.tttech-industrial.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Cc: stable@...r.kernel.org
> Link:
> https://lore.kernel.org/r/AM6PR01MB541637BD6F336B8FFB72AF80EEC69@AM6PR01MB5416.eurprd01.prod.exchangel
> 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)