linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros
@ 2025-09-12 15:30 Eliav Farber
  2025-09-12 15:30 ` [PATCH v2 1/4 5.10.y] overflow: Correct check_shl_overflow() comment Eliav Farber
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Eliav Farber @ 2025-09-12 15:30 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, natechancellor, ndesaulniers,
	keescook, sashal, akpm, ojeda, elver, gregkh, kbusch, sj,
	bvanassche, leon, jgg, linux-kernel, linux-sparse,
	clang-built-linux, stable
  Cc: jonnyc, farbere

This series backports four commits to bring include/linux/overflow.h in
line with v5.15.193:
 - 2541be80b1a2 ("overflow: Correct check_shl_overflow() comment")
 - 564e84663d25 ("compiler.h: drop fallback overflow checkers")
 - 1d1ac8244c22 ("overflow: Allow mixed type arguments")
 - f96cfe3e05b0 ("tracing: Define the is_signed_type() macro once")

The motivation is to fix build failures such as:

drivers/net/ethernet/intel/e1000e/ethtool.c: In function ‘e1000_set_eeprom’:
./include/linux/overflow.h:71:15: error: comparison of distinct pointer types lacks a cast [-Werror]
   71 |  (void) (&__a == __d);   \
      |               ^~
drivers/net/ethernet/intel/e1000e/ethtool.c:582:6: note: in expansion of macro ‘check_add_overflow’
  582 |  if (check_add_overflow(eeprom->offset, eeprom->len, &total_len) ||
      |      ^~~~~~~~~~~~~~~~~~

This regression was triggered by commit ce8829d3d44b ("e1000e: fix heap
overflow in e1000_set_eeprom").

check_add_overflow() requires the first two operands and the result
pointer to be of identical type. On 64-bit builds, using size_t for the
result conflicted with the u32 fields eeprom->offset and eeprom->len,
resulting in type check failures.

BarteVan Assche (1):
  tracing: Define the is_signed_type() macro once

Kees Cook (1):
  overflow: Allow mixed type arguments

Keith Busch (1):
  overflow: Correct check_shl_overflow() comment

Nick Desaulniers (1):
  compiler.h: drop fallback overflow checkers

 include/linux/compiler-clang.h     |  13 --
 include/linux/compiler-gcc.h       |   4 -
 include/linux/compiler.h           |   6 +
 include/linux/overflow.h           | 209 ++++++-----------------------
 include/linux/trace_events.h       |   2 -
 tools/include/linux/compiler-gcc.h |   4 -
 tools/include/linux/overflow.h     | 140 +------------------
 7 files changed, 52 insertions(+), 326 deletions(-)

---
Changes in v2:
 - Added missing sign-off in all patches

-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/4 5.10.y] overflow: Correct check_shl_overflow() comment
  2025-09-12 15:30 [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Eliav Farber
@ 2025-09-12 15:30 ` Eliav Farber
  2025-09-12 15:30 ` [PATCH v2 2/4 5.10.y] compiler.h: drop fallback overflow checkers Eliav Farber
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Eliav Farber @ 2025-09-12 15:30 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, natechancellor, ndesaulniers,
	keescook, sashal, akpm, ojeda, elver, gregkh, kbusch, sj,
	bvanassche, leon, jgg, linux-kernel, linux-sparse,
	clang-built-linux, stable
  Cc: jonnyc, farbere

From: Keith Busch <kbusch@kernel.org>

[ Upstream commit 4578be130a6470d85ff05b13b75a00e6224eeeeb ]

A 'false' return means the value was safely set, so the comment should
say 'true' for when it is not considered safe.

Cc: Jason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
Fixes: 0c66847793d1 ("overflow.h: Add arithmetic shift helper")
Link: https://lore.kernel.org/r/20210401160629.1941787-1-kbusch@kernel.org
---
 include/linux/overflow.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index 35af574d006f..d1dd039fe1c3 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -235,7 +235,7 @@ static inline bool __must_check __must_check_overflow(bool overflow)
  * - 'a << s' sets the sign bit, if any, in '*d'.
  *
  * '*d' will hold the results of the attempted shift, but is not
- * considered "safe for use" if false is returned.
+ * considered "safe for use" if true is returned.
  */
 #define check_shl_overflow(a, s, d) __must_check_overflow(({		\
 	typeof(a) _a = a;						\
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/4 5.10.y] compiler.h: drop fallback overflow checkers
  2025-09-12 15:30 [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Eliav Farber
  2025-09-12 15:30 ` [PATCH v2 1/4 5.10.y] overflow: Correct check_shl_overflow() comment Eliav Farber
@ 2025-09-12 15:30 ` Eliav Farber
  2025-09-12 15:30 ` [PATCH v2 3/4 5.10.y] overflow: Allow mixed type arguments Eliav Farber
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Eliav Farber @ 2025-09-12 15:30 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, natechancellor, ndesaulniers,
	keescook, sashal, akpm, ojeda, elver, gregkh, kbusch, sj,
	bvanassche, leon, jgg, linux-kernel, linux-sparse,
	clang-built-linux, stable
  Cc: jonnyc, farbere, Rasmus Villemoes, Nathan Chancellor

From: Nick Desaulniers <ndesaulniers@google.com>

[ Upstream commit 4eb6bd55cfb22ffc20652732340c4962f3ac9a91 ]

Once upgrading the minimum supported version of GCC to 5.1, we can drop
the fallback code for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW.

This is effectively a revert of commit f0907827a8a9 ("compiler.h: enable
builtin overflow checkers and add fallback code")

Link: https://github.com/ClangBuiltLinux/linux/issues/1438#issuecomment-916745801
Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 include/linux/compiler-clang.h     |  13 ---
 include/linux/compiler-gcc.h       |   4 -
 include/linux/overflow.h           | 138 +---------------------------
 tools/include/linux/compiler-gcc.h |   4 -
 tools/include/linux/overflow.h     | 140 +----------------------------
 5 files changed, 6 insertions(+), 293 deletions(-)

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 9ba951e3a6c2..928ec5c7776d 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -72,19 +72,6 @@
 #define __no_sanitize_coverage
 #endif
 
-/*
- * Not all versions of clang implement the type-generic versions
- * of the builtin overflow checkers. Fortunately, clang implements
- * __has_builtin allowing us to avoid awkward version
- * checks. Unfortunately, we don't know which version of gcc clang
- * pretends to be, so the macro may or may not be defined.
- */
-#if __has_builtin(__builtin_mul_overflow) && \
-    __has_builtin(__builtin_add_overflow) && \
-    __has_builtin(__builtin_sub_overflow)
-#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
-#endif
-
 #if __has_feature(shadow_call_stack)
 # define __noscs	__attribute__((__no_sanitize__("shadow-call-stack")))
 #endif
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 5b481a22b5fe..ae9a8e17287c 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -140,10 +140,6 @@
 #define __no_sanitize_coverage
 #endif
 
-#if GCC_VERSION >= 50100
-#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
-#endif
-
 /*
  * Turn individual warnings and errors on and off locally, depending
  * on version.
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index d1dd039fe1c3..59d7228104d0 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -6,12 +6,9 @@
 #include <linux/limits.h>
 
 /*
- * In the fallback code below, we need to compute the minimum and
- * maximum values representable in a given type. These macros may also
- * be useful elsewhere, so we provide them outside the
- * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block.
- *
- * It would seem more obvious to do something like
+ * We need to compute the minimum and maximum values representable in a given
+ * type. These macros may also be useful elsewhere. It would seem more obvious
+ * to do something like:
  *
  * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
  * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)
@@ -54,7 +51,6 @@ static inline bool __must_check __must_check_overflow(bool overflow)
 	return unlikely(overflow);
 }
 
-#ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW
 /*
  * For simplicity and code hygiene, the fallback code below insists on
  * a, b and *d having the same type (similar to the min() and max()
@@ -90,134 +86,6 @@ static inline bool __must_check __must_check_overflow(bool overflow)
 	__builtin_mul_overflow(__a, __b, __d);	\
 }))
 
-#else
-
-
-/* Checking for unsigned overflow is relatively easy without causing UB. */
-#define __unsigned_add_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = __a + __b;			\
-	*__d < __a;				\
-})
-#define __unsigned_sub_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = __a - __b;			\
-	__a < __b;				\
-})
-/*
- * If one of a or b is a compile-time constant, this avoids a division.
- */
-#define __unsigned_mul_overflow(a, b, d) ({		\
-	typeof(a) __a = (a);				\
-	typeof(b) __b = (b);				\
-	typeof(d) __d = (d);				\
-	(void) (&__a == &__b);				\
-	(void) (&__a == __d);				\
-	*__d = __a * __b;				\
-	__builtin_constant_p(__b) ?			\
-	  __b > 0 && __a > type_max(typeof(__a)) / __b : \
-	  __a > 0 && __b > type_max(typeof(__b)) / __a;	 \
-})
-
-/*
- * For signed types, detecting overflow is much harder, especially if
- * we want to avoid UB. But the interface of these macros is such that
- * we must provide a result in *d, and in fact we must produce the
- * result promised by gcc's builtins, which is simply the possibly
- * wrapped-around value. Fortunately, we can just formally do the
- * operations in the widest relevant unsigned type (u64) and then
- * truncate the result - gcc is smart enough to generate the same code
- * with and without the (u64) casts.
- */
-
-/*
- * Adding two signed integers can overflow only if they have the same
- * sign, and overflow has happened iff the result has the opposite
- * sign.
- */
-#define __signed_add_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = (u64)__a + (u64)__b;		\
-	(((~(__a ^ __b)) & (*__d ^ __a))	\
-		& type_min(typeof(__a))) != 0;	\
-})
-
-/*
- * Subtraction is similar, except that overflow can now happen only
- * when the signs are opposite. In this case, overflow has happened if
- * the result has the opposite sign of a.
- */
-#define __signed_sub_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = (u64)__a - (u64)__b;		\
-	((((__a ^ __b)) & (*__d ^ __a))		\
-		& type_min(typeof(__a))) != 0;	\
-})
-
-/*
- * Signed multiplication is rather hard. gcc always follows C99, so
- * division is truncated towards 0. This means that we can write the
- * overflow check like this:
- *
- * (a > 0 && (b > MAX/a || b < MIN/a)) ||
- * (a < -1 && (b > MIN/a || b < MAX/a) ||
- * (a == -1 && b == MIN)
- *
- * The redundant casts of -1 are to silence an annoying -Wtype-limits
- * (included in -Wextra) warning: When the type is u8 or u16, the
- * __b_c_e in check_mul_overflow obviously selects
- * __unsigned_mul_overflow, but unfortunately gcc still parses this
- * code and warns about the limited range of __b.
- */
-
-#define __signed_mul_overflow(a, b, d) ({				\
-	typeof(a) __a = (a);						\
-	typeof(b) __b = (b);						\
-	typeof(d) __d = (d);						\
-	typeof(a) __tmax = type_max(typeof(a));				\
-	typeof(a) __tmin = type_min(typeof(a));				\
-	(void) (&__a == &__b);						\
-	(void) (&__a == __d);						\
-	*__d = (u64)__a * (u64)__b;					\
-	(__b > 0   && (__a > __tmax/__b || __a < __tmin/__b)) ||	\
-	(__b < (typeof(__b))-1  && (__a > __tmin/__b || __a < __tmax/__b)) || \
-	(__b == (typeof(__b))-1 && __a == __tmin);			\
-})
-
-
-#define check_add_overflow(a, b, d)	__must_check_overflow(		\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_add_overflow(a, b, d),			\
-			__unsigned_add_overflow(a, b, d)))
-
-#define check_sub_overflow(a, b, d)	__must_check_overflow(		\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_sub_overflow(a, b, d),			\
-			__unsigned_sub_overflow(a, b, d)))
-
-#define check_mul_overflow(a, b, d)	__must_check_overflow(		\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_mul_overflow(a, b, d),			\
-			__unsigned_mul_overflow(a, b, d)))
-
-#endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
-
 /** check_shl_overflow() - Calculate a left-shifted value and check overflow
  *
  * @a: Value to be shifted
diff --git a/tools/include/linux/compiler-gcc.h b/tools/include/linux/compiler-gcc.h
index 95c072b70d0e..a590a1dfafd9 100644
--- a/tools/include/linux/compiler-gcc.h
+++ b/tools/include/linux/compiler-gcc.h
@@ -38,7 +38,3 @@
 #endif
 #define __printf(a, b)	__attribute__((format(printf, a, b)))
 #define __scanf(a, b)	__attribute__((format(scanf, a, b)))
-
-#if GCC_VERSION >= 50100
-#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
-#endif
diff --git a/tools/include/linux/overflow.h b/tools/include/linux/overflow.h
index 8712ff70995f..dcb0c1bf6866 100644
--- a/tools/include/linux/overflow.h
+++ b/tools/include/linux/overflow.h
@@ -5,12 +5,9 @@
 #include <linux/compiler.h>
 
 /*
- * In the fallback code below, we need to compute the minimum and
- * maximum values representable in a given type. These macros may also
- * be useful elsewhere, so we provide them outside the
- * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block.
- *
- * It would seem more obvious to do something like
+ * We need to compute the minimum and maximum values representable in a given
+ * type. These macros may also be useful elsewhere. It would seem more obvious
+ * to do something like:
  *
  * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
  * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)
@@ -36,8 +33,6 @@
 #define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
 #define type_min(T) ((T)((T)-type_max(T)-(T)1))
 
-
-#ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW
 /*
  * For simplicity and code hygiene, the fallback code below insists on
  * a, b and *d having the same type (similar to the min() and max()
@@ -73,135 +68,6 @@
 	__builtin_mul_overflow(__a, __b, __d);	\
 })
 
-#else
-
-
-/* Checking for unsigned overflow is relatively easy without causing UB. */
-#define __unsigned_add_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = __a + __b;			\
-	*__d < __a;				\
-})
-#define __unsigned_sub_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = __a - __b;			\
-	__a < __b;				\
-})
-/*
- * If one of a or b is a compile-time constant, this avoids a division.
- */
-#define __unsigned_mul_overflow(a, b, d) ({		\
-	typeof(a) __a = (a);				\
-	typeof(b) __b = (b);				\
-	typeof(d) __d = (d);				\
-	(void) (&__a == &__b);				\
-	(void) (&__a == __d);				\
-	*__d = __a * __b;				\
-	__builtin_constant_p(__b) ?			\
-	  __b > 0 && __a > type_max(typeof(__a)) / __b : \
-	  __a > 0 && __b > type_max(typeof(__b)) / __a;	 \
-})
-
-/*
- * For signed types, detecting overflow is much harder, especially if
- * we want to avoid UB. But the interface of these macros is such that
- * we must provide a result in *d, and in fact we must produce the
- * result promised by gcc's builtins, which is simply the possibly
- * wrapped-around value. Fortunately, we can just formally do the
- * operations in the widest relevant unsigned type (u64) and then
- * truncate the result - gcc is smart enough to generate the same code
- * with and without the (u64) casts.
- */
-
-/*
- * Adding two signed integers can overflow only if they have the same
- * sign, and overflow has happened iff the result has the opposite
- * sign.
- */
-#define __signed_add_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = (u64)__a + (u64)__b;		\
-	(((~(__a ^ __b)) & (*__d ^ __a))	\
-		& type_min(typeof(__a))) != 0;	\
-})
-
-/*
- * Subtraction is similar, except that overflow can now happen only
- * when the signs are opposite. In this case, overflow has happened if
- * the result has the opposite sign of a.
- */
-#define __signed_sub_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = (u64)__a - (u64)__b;		\
-	((((__a ^ __b)) & (*__d ^ __a))		\
-		& type_min(typeof(__a))) != 0;	\
-})
-
-/*
- * Signed multiplication is rather hard. gcc always follows C99, so
- * division is truncated towards 0. This means that we can write the
- * overflow check like this:
- *
- * (a > 0 && (b > MAX/a || b < MIN/a)) ||
- * (a < -1 && (b > MIN/a || b < MAX/a) ||
- * (a == -1 && b == MIN)
- *
- * The redundant casts of -1 are to silence an annoying -Wtype-limits
- * (included in -Wextra) warning: When the type is u8 or u16, the
- * __b_c_e in check_mul_overflow obviously selects
- * __unsigned_mul_overflow, but unfortunately gcc still parses this
- * code and warns about the limited range of __b.
- */
-
-#define __signed_mul_overflow(a, b, d) ({				\
-	typeof(a) __a = (a);						\
-	typeof(b) __b = (b);						\
-	typeof(d) __d = (d);						\
-	typeof(a) __tmax = type_max(typeof(a));				\
-	typeof(a) __tmin = type_min(typeof(a));				\
-	(void) (&__a == &__b);						\
-	(void) (&__a == __d);						\
-	*__d = (u64)__a * (u64)__b;					\
-	(__b > 0   && (__a > __tmax/__b || __a < __tmin/__b)) ||	\
-	(__b < (typeof(__b))-1  && (__a > __tmin/__b || __a < __tmax/__b)) || \
-	(__b == (typeof(__b))-1 && __a == __tmin);			\
-})
-
-
-#define check_add_overflow(a, b, d)					\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_add_overflow(a, b, d),			\
-			__unsigned_add_overflow(a, b, d))
-
-#define check_sub_overflow(a, b, d)					\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_sub_overflow(a, b, d),			\
-			__unsigned_sub_overflow(a, b, d))
-
-#define check_mul_overflow(a, b, d)					\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_mul_overflow(a, b, d),			\
-			__unsigned_mul_overflow(a, b, d))
-
-
-#endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
-
 /**
  * array_size() - Calculate size of 2-dimensional array.
  *
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/4 5.10.y] overflow: Allow mixed type arguments
  2025-09-12 15:30 [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Eliav Farber
  2025-09-12 15:30 ` [PATCH v2 1/4 5.10.y] overflow: Correct check_shl_overflow() comment Eliav Farber
  2025-09-12 15:30 ` [PATCH v2 2/4 5.10.y] compiler.h: drop fallback overflow checkers Eliav Farber
@ 2025-09-12 15:30 ` Eliav Farber
  2025-09-12 15:30 ` [PATCH v2 4/4 5.10.y] tracing: Define the is_signed_type() macro once Eliav Farber
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Eliav Farber @ 2025-09-12 15:30 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, natechancellor, ndesaulniers,
	keescook, sashal, akpm, ojeda, elver, gregkh, kbusch, sj,
	bvanassche, leon, jgg, linux-kernel, linux-sparse,
	clang-built-linux, stable
  Cc: jonnyc, farbere, Rasmus Villemoes, Gwan-gyeong Mun,
	Gustavo A. R. Silva, linux-hardening, Andrzej Hajda

From: Kees Cook <keescook@chromium.org>

commit d219d2a9a92e39aa92799efe8f2aa21259b6dd82 upstream.

When the check_[op]_overflow() helpers were introduced, all arguments
were required to be the same type to make the fallback macros simpler.
However, now that the fallback macros have been removed[1], it is fine
to allow mixed types, which makes using the helpers much more useful,
as they can be used to test for type-based overflows (e.g. adding two
large ints but storing into a u8), as would be handy in the drm core[2].

Remove the restriction, and add additional self-tests that exercise
some of the mixed-type overflow cases, and double-check for accidental
macro side-effects.

[1] https://git.kernel.org/linus/4eb6bd55cfb22ffc20652732340c4962f3ac9a91
[2] https://lore.kernel.org/lkml/20220824084514.2261614-2-gwan-gyeong.mun@intel.com

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: linux-hardening@vger.kernel.org
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Tested-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
[ dropped the test portion of the commit as that doesn't apply to
  5.15.y - gregkh]
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 include/linux/overflow.h | 72 +++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 31 deletions(-)

diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index 59d7228104d0..73bc67ec2136 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -51,40 +51,50 @@ static inline bool __must_check __must_check_overflow(bool overflow)
 	return unlikely(overflow);
 }
 
-/*
- * For simplicity and code hygiene, the fallback code below insists on
- * a, b and *d having the same type (similar to the min() and max()
- * macros), whereas gcc's type-generic overflow checkers accept
- * different types. Hence we don't just make check_add_overflow an
- * alias for __builtin_add_overflow, but add type checks similar to
- * below.
+/** check_add_overflow() - Calculate addition with overflow checking
+ *
+ * @a: first addend
+ * @b: second addend
+ * @d: pointer to store sum
+ *
+ * Returns 0 on success.
+ *
+ * *@d holds the results of the attempted addition, but is not considered
+ * "safe for use" on a non-zero return value, which indicates that the
+ * sum has overflowed or been truncated.
  */
-#define check_add_overflow(a, b, d) __must_check_overflow(({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	__builtin_add_overflow(__a, __b, __d);	\
-}))
+#define check_add_overflow(a, b, d)	\
+	__must_check_overflow(__builtin_add_overflow(a, b, d))
 
-#define check_sub_overflow(a, b, d) __must_check_overflow(({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	__builtin_sub_overflow(__a, __b, __d);	\
-}))
+/** check_sub_overflow() - Calculate subtraction with overflow checking
+ *
+ * @a: minuend; value to subtract from
+ * @b: subtrahend; value to subtract from @a
+ * @d: pointer to store difference
+ *
+ * Returns 0 on success.
+ *
+ * *@d holds the results of the attempted subtraction, but is not considered
+ * "safe for use" on a non-zero return value, which indicates that the
+ * difference has underflowed or been truncated.
+ */
+#define check_sub_overflow(a, b, d)	\
+	__must_check_overflow(__builtin_sub_overflow(a, b, d))
 
-#define check_mul_overflow(a, b, d) __must_check_overflow(({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	__builtin_mul_overflow(__a, __b, __d);	\
-}))
+/** check_mul_overflow() - Calculate multiplication with overflow checking
+ *
+ * @a: first factor
+ * @b: second factor
+ * @d: pointer to store product
+ *
+ * Returns 0 on success.
+ *
+ * *@d holds the results of the attempted multiplication, but is not
+ * considered "safe for use" on a non-zero return value, which indicates
+ * that the product has overflowed or been truncated.
+ */
+#define check_mul_overflow(a, b, d)	\
+	__must_check_overflow(__builtin_mul_overflow(a, b, d))
 
 /** check_shl_overflow() - Calculate a left-shifted value and check overflow
  *
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4 5.10.y] tracing: Define the is_signed_type() macro once
  2025-09-12 15:30 [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Eliav Farber
                   ` (2 preceding siblings ...)
  2025-09-12 15:30 ` [PATCH v2 3/4 5.10.y] overflow: Allow mixed type arguments Eliav Farber
@ 2025-09-12 15:30 ` Eliav Farber
  2025-09-12 15:46 ` [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Bart Van Assche
  2025-09-12 16:52 ` Sasha Levin
  5 siblings, 0 replies; 8+ messages in thread
From: Eliav Farber @ 2025-09-12 15:30 UTC (permalink / raw)
  To: luc.vanoostenryck, rostedt, mingo, natechancellor, ndesaulniers,
	keescook, sashal, akpm, ojeda, elver, gregkh, kbusch, sj,
	bvanassche, leon, jgg, linux-kernel, linux-sparse,
	clang-built-linux, stable
  Cc: jonnyc, farbere, Rasmus Villemoes

From: Bart Van Assche <bvanassche@acm.org>

commit a49a64b5bf195381c09202c524f0f84b5f3e816f upstream.

There are two definitions of the is_signed_type() macro: one in
<linux/overflow.h> and a second definition in <linux/trace_events.h>.

As suggested by Linus, move the definition of the is_signed_type() macro
into the <linux/compiler.h> header file.  Change the definition of the
is_signed_type() macro to make sure that it does not trigger any sparse
warnings with future versions of sparse for bitwise types.

Link: https://lore.kernel.org/all/CAHk-=whjH6p+qzwUdx5SOVVHjS3WvzJQr6mDUwhEyTf6pJWzaQ@mail.gmail.com/
Link: https://lore.kernel.org/all/CAHk-=wjQGnVfb4jehFR0XyZikdQvCZouE96xR_nnf5kqaM5qqQ@mail.gmail.com/
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
(cherry picked from commit a49a64b5bf195381c09202c524f0f84b5f3e816f)
Signed-off-by: SeongJae Park <sj@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 include/linux/compiler.h     | 6 ++++++
 include/linux/overflow.h     | 1 -
 include/linux/trace_events.h | 2 --
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index bbd74420fa21..004a030d5ad2 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -245,6 +245,12 @@ static inline void *offset_to_ptr(const int *off)
 /* &a[0] degrades to a pointer: a different type from an array */
 #define __must_be_array(a)	BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
 
+/*
+ * Whether 'type' is a signed type or an unsigned type. Supports scalar types,
+ * bool and also pointer types.
+ */
+#define is_signed_type(type) (((type)(-1)) < (__force type)1)
+
 /*
  * This is needed in functions which generate the stack canary, see
  * arch/x86/kernel/smpboot.c::start_secondary() for an example.
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index 73bc67ec2136..e6bf14f462e9 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -29,7 +29,6 @@
  * https://mail-index.netbsd.org/tech-misc/2007/02/05/0000.html -
  * credit to Christian Biere.
  */
-#define is_signed_type(type)       (((type)(-1)) < (type)1)
 #define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 - is_signed_type(type)))
 #define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
 #define type_min(T) ((T)((T)-type_max(T)-(T)1))
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 5af2acb9fb7d..0c8c3cf36f96 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -700,8 +700,6 @@ extern int trace_add_event_call(struct trace_event_call *call);
 extern int trace_remove_event_call(struct trace_event_call *call);
 extern int trace_event_get_offsets(struct trace_event_call *call);
 
-#define is_signed_type(type)	(((type)(-1)) < (type)1)
-
 int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
 int trace_set_clr_event(const char *system, const char *event, int set);
 int trace_array_set_clr_event(struct trace_array *tr, const char *system,
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros
  2025-09-12 15:30 [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Eliav Farber
                   ` (3 preceding siblings ...)
  2025-09-12 15:30 ` [PATCH v2 4/4 5.10.y] tracing: Define the is_signed_type() macro once Eliav Farber
@ 2025-09-12 15:46 ` Bart Van Assche
  2025-09-12 16:52 ` Sasha Levin
  5 siblings, 0 replies; 8+ messages in thread
From: Bart Van Assche @ 2025-09-12 15:46 UTC (permalink / raw)
  To: Eliav Farber, luc.vanoostenryck, rostedt, mingo, natechancellor,
	ndesaulniers, keescook, sashal, akpm, ojeda, elver, gregkh,
	kbusch, sj, leon, jgg, linux-kernel, linux-sparse,
	clang-built-linux, stable
  Cc: jonnyc

On 9/12/25 8:30 AM, Eliav Farber wrote:
> BarteVan Assche (1):

Please spell my name correctly in future emails.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros
  2025-09-12 15:30 [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Eliav Farber
                   ` (4 preceding siblings ...)
  2025-09-12 15:46 ` [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Bart Van Assche
@ 2025-09-12 16:52 ` Sasha Levin
  2025-09-12 18:56   ` Farber, Eliav
  5 siblings, 1 reply; 8+ messages in thread
From: Sasha Levin @ 2025-09-12 16:52 UTC (permalink / raw)
  To: Eliav Farber
  Cc: luc.vanoostenryck, rostedt, mingo, natechancellor, ndesaulniers,
	keescook, akpm, ojeda, elver, gregkh, kbusch, sj, bvanassche,
	leon, jgg, linux-kernel, linux-sparse, clang-built-linux, stable,
	jonnyc

On Fri, Sep 12, 2025 at 03:30:34PM +0000, Eliav Farber wrote:
>This series backports four commits to bring include/linux/overflow.h in
>line with v5.15.193:
> - 2541be80b1a2 ("overflow: Correct check_shl_overflow() comment")
> - 564e84663d25 ("compiler.h: drop fallback overflow checkers")
> - 1d1ac8244c22 ("overflow: Allow mixed type arguments")
> - f96cfe3e05b0 ("tracing: Define the is_signed_type() macro once")

None of these SHA1s match with what's actually in v5.15.193. What's going on
here?

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros
  2025-09-12 16:52 ` Sasha Levin
@ 2025-09-12 18:56   ` Farber, Eliav
  0 siblings, 0 replies; 8+ messages in thread
From: Farber, Eliav @ 2025-09-12 18:56 UTC (permalink / raw)
  To: Sasha Levin
  Cc: luc.vanoostenryck@gmail.com, rostedt@goodmis.org,
	mingo@redhat.com, natechancellor@gmail.com,
	ndesaulniers@google.com, keescook@chromium.org,
	akpm@linux-foundation.org, ojeda@kernel.org, elver@google.com,
	gregkh@linuxfoundation.org, kbusch@kernel.org, sj@kernel.org,
	bvanassche@acm.org, leon@kernel.org, jgg@ziepe.ca,
	linux-kernel@vger.kernel.org, linux-sparse@vger.kernel.org,
	clang-built-linux@googlegroups.com, stable@vger.kernel.org,
	Chocron, Jonathan

> On Fri, Sep 12, 2025 at 03:30:34PM +0000, Eliav Farber wrote:
> >This series backports four commits to bring include/linux/overflow.h in 
> >line with v5.15.193:
> > - 2541be80b1a2 ("overflow: Correct check_shl_overflow() comment")
> > - 564e84663d25 ("compiler.h: drop fallback overflow checkers")
> > - 1d1ac8244c22 ("overflow: Allow mixed type arguments")
> > - f96cfe3e05b0 ("tracing: Define the is_signed_type() macro once")
>
> None of these SHA1s match with what's actually in v5.15.193. What's going on here?
Fixed in v3.

---
Thanks, Eliav

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-09-12 18:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-12 15:30 [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Eliav Farber
2025-09-12 15:30 ` [PATCH v2 1/4 5.10.y] overflow: Correct check_shl_overflow() comment Eliav Farber
2025-09-12 15:30 ` [PATCH v2 2/4 5.10.y] compiler.h: drop fallback overflow checkers Eliav Farber
2025-09-12 15:30 ` [PATCH v2 3/4 5.10.y] overflow: Allow mixed type arguments Eliav Farber
2025-09-12 15:30 ` [PATCH v2 4/4 5.10.y] tracing: Define the is_signed_type() macro once Eliav Farber
2025-09-12 15:46 ` [PATCH v2 0/4 5.10.y] overflow: Allow mixed type arguments in overflow macros Bart Van Assche
2025-09-12 16:52 ` Sasha Levin
2025-09-12 18:56   ` Farber, Eliav

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).