linux-kernel - [PATCH 3.16 115/178] kernel.h: make abs() work with 64-bit types

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]

Date:   Sun, 16 Jul 2017 14:56:46 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, "Michal Nazarewicz" <mina86@...a86.com>,
        "Linus Torvalds" <torvalds@...ux-foundation.org>,
        "Peter Zijlstra" <peterz@...radead.org>,
        "Steven Rostedt" <rostedt@...dmis.org>,
        "John Stultz" <john.stultz@...aro.org>,
        "Masami Hiramatsu" <masami.hiramatsu.pt@...achi.com>,
        "Peter Zijlstra" <a.p.zijlstra@...llo.nl>,
        "Ingo Molnar" <mingo@...nel.org>
Subject: [PATCH 3.16 115/178] kernel.h: make abs() work with 64-bit types

3.16.46-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Michal Nazarewicz <mina86@...a86.com>

commit c8299cb605b27dd5a49f7a69e48fd23e5a206298 upstream.

For 64-bit arguments, the abs macro casts it to an int which leads to
lost precision and may cause incorrect results.  To deal with 64-bit
types abs64 macro has been introduced but still there are places where
abs macro is used incorrectly.

To deal with the problem, expand abs macro such that it operates on s64
type when dealing with 64-bit types while still returning long when
dealing with smaller types.

This fixes one known bug (per John):

The internal clocksteering done for fine-grained error correction uses a
: logarithmic approximation, so any time adjtimex() adjusts the clock
: steering, timekeeping_freqadjust() quickly approximates the correct clock
: frequency over a series of ticks.
:
: Unfortunately, the logic in timekeeping_freqadjust(), introduced in commit
: dc491596f639438 (Rework frequency adjustments to work better w/ nohz),
: used the abs() function with a s64 error value to calculate the size of
: the approximated adjustment to be made.
:
: Per include/linux/kernel.h: "abs() should not be used for 64-bit types
: (s64, u64, long long) - use abs64()".
:
: Thus on 32-bit platforms, this resulted in the clocksteering to take a
: quite dampended random walk trying to converge on the proper frequency,
: which caused the adjustments to be made much slower then intended (most
: easily observed when large adjustments are made).

Signed-off-by: Michal Nazarewicz <mina86@...a86.com>
Reported-by: John Stultz <john.stultz@...aro.org>
Tested-by: John Stultz <john.stultz@...aro.org>
Cc: Ingo Molnar <mingo@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 include/linux/kernel.h | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -183,28 +183,31 @@ extern int _cond_resched(void);

 #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)

-/*
- * abs() handles unsigned and signed longs, ints, shorts and chars.  For all
- * input types abs() returns a signed long.
- * abs() should not be used for 64-bit types (s64, u64, long long) - use abs64()
- * for those.
+/**
+ * abs - return absolute value of an argument
+ * @x: the value.  If it is unsigned type, it is converted to signed type first
+ *   (s64, long or int depending on its size).
+ *
+ * Return: an absolute value of x.  If x is 64-bit, macro's return type is s64,
+ *   otherwise it is signed long.
  */
-#define abs(x) ({						\
-		long ret;					\
-		if (sizeof(x) == sizeof(long)) {		\
-			long __x = (x);				\
-			ret = (__x < 0) ? -__x : __x;		\
-		} else {					\
-			int __x = (x);				\
-			ret = (__x < 0) ? -__x : __x;		\
-		}						\
-		ret;						\
-	})
+#define abs(x) __builtin_choose_expr(sizeof(x) == sizeof(s64), ({	\
+		s64 __x = (x);						\
+		(__x < 0) ? -__x : __x;					\
+	}), ({								\
+		long ret;						\
+		if (sizeof(x) == sizeof(long)) {			\
+			long __x = (x);					\
+			ret = (__x < 0) ? -__x : __x;			\
+		} else {						\
+			int __x = (x);					\
+			ret = (__x < 0) ? -__x : __x;			\
+		}							\
+		ret;							\
+	}))

-#define abs64(x) ({				\
-		s64 __x = (x);			\
-		(__x < 0) ? -__x : __x;		\
-	})
+/* Deprecated, use abs instead. */
+#define abs64(x) abs((s64)(x))

 /**
  * reciprocal_scale - "scale" a value into range [0, ep_ro)