linux-kernel - [PATCH] timekeeping: Change type of nsec variable to unsigned in its calculation.

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-next>] [day] [month] [year] [list]

Message-Id: <1479531216-25361-1-git-send-email-john.stultz@linaro.org>
Date:   Fri, 18 Nov 2016 20:53:36 -0800
From:   John Stultz <john.stultz@...aro.org>
To:     lkml <linux-kernel@...r.kernel.org>
Cc:     Liav Rehana <liavr@...lanox.com>,
        Chris Metcalf <cmetcalf@...lanox.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Richard Cochran <richardcochran@...il.com>,
        Ingo Molnar <mingo@...nel.org>,
        Prarit Bhargava <prarit@...hat.com>,
        Laurent Vivier <lvivier@...hat.com>,
        David Gibson <david@...son.dropbear.id.au>,
        "Christopher S . Hall" <christopher.s.hall@...el.com>,
        stable@...r.kernel.org (4.6+),
        John Stultz <john.stultz@...aro.org>
Subject: [PATCH] timekeeping: Change type of nsec variable to unsigned in its calculation.

From: Liav Rehana <liavr@...lanox.com>

During the calculation of the nsec variable in the inline function
timekeeping_delta_to_ns, it may undergo a sign extension if its msb
is set just before the shift. The sign extension may, in some cases,
gain it a value near the maximum value of the 64-bit range. This is
bad when it is later used in a division function, such as
__iter_div_u64_rem, where the amount of loops it will go through to
calculate the division will be too large. One can encounter such a
problem, for example, when trying to connect through ftp from an
outside host to the operation system. When the OS is too overloaded,
delta will get a high enough value for the msb of the sum
delta * tkr->mult + tkr->xtime_nsec to be set, and so after the
shift the nsec variable will gain a value similar to
0xffffffffff000000. Using a variable with such a value in the
inline function __iter_div_u64_rem will take too long, making the
ftp connection attempt seem to get stuck.
The following commit fixes that chance of sign extension, while
maintaining the type of the nsec variable as signed for other
functions that use this variable, for possible legit negative
time intervals.

Cc: Chris Metcalf <cmetcalf@...lanox.com>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Richard Cochran <richardcochran@...il.com>
Cc: Ingo Molnar <mingo@...nel.org>
Cc: Prarit Bhargava <prarit@...hat.com>
Cc: Laurent Vivier <lvivier@...hat.com>
Cc: David Gibson <david@...son.dropbear.id.au>
Cc: "Christopher S . Hall" <christopher.s.hall@...el.com>
Cc: stable@...r.kernel.org  (4.6+)
Fixes: 6bd58f09e1d8 ("time: Add cycles to nanoseconds translation")
Also-Reported-by: Chris Metcalf <cmetcalf@...lanox.com>
Signed-off-by: Liav Rehana <liavr@...lanox.com>
Signed-off-by: John Stultz <john.stultz@...aro.org>
---
Thomas/Ingo: This is for tip:timers/urgent.

 kernel/time/timekeeping.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 37dec7e..46e312e 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -299,10 +299,10 @@ u32 (*arch_gettimeoffset)(void) = default_arch_gettimeoffset;
 static inline u32 arch_gettimeoffset(void) { return 0; }
 #endif

-static inline s64 timekeeping_delta_to_ns(struct tk_read_base *tkr,
+static inline u64 timekeeping_delta_to_ns(struct tk_read_base *tkr,
 					  cycle_t delta)
 {
-	s64 nsec;
+	u64 nsec;

 	nsec = delta * tkr->mult + tkr->xtime_nsec;
 	nsec >>= tkr->shift;
-- 
2.7.4