Commit 3c8ba0d6 authored by Kees Cook's avatar Kees Cook Committed by Linus Torvalds

kernel.h: Retain constant expression output for max()/min()

In the effort to remove all VLAs from the kernel[1], it is desirable to
build with -Wvla.  However, this warning is overly pessimistic, in that
it is only happy with stack array sizes that are declared as constant
expressions, and not constant values.  One case of this is the
evaluation of the max() macro which, due to its construction, ends up
converting constant expression arguments into a constant value result.

All attempts to rewrite this macro with __builtin_constant_p() failed
with older compilers (e.g.  gcc 4.4)[2].  However, Martin Uecker,
constructed[3] a mind-shattering solution that works everywhere.
Cthulhu fhtagn!

This patch updates the min()/max() macros to evaluate to a constant
expression when called on constant expression arguments.  This removes
several false-positive stack VLA warnings from an x86 allmodconfig build
when -Wvla is added:

  $ diff -u before.txt after.txt | grep ^-
  -drivers/input/touchscreen/cyttsp4_core.c:871:2: warning: ISO C90 forbids variable length array ‘ids’ [-Wvla]
  -fs/btrfs/tree-checker.c:344:4: warning: ISO C90 forbids variable length array ‘namebuf’ [-Wvla]
  -lib/vsprintf.c:747:2: warning: ISO C90 forbids variable length array ‘sym’ [-Wvla]
  -net/ipv4/proc.c:403:2: warning: ISO C90 forbids variable length array ‘buff’ [-Wvla]
  -net/ipv6/proc.c:198:2: warning: ISO C90 forbids variable length array ‘buff’ [-Wvla]
  -net/ipv6/proc.c:218:2: warning: ISO C90 forbids variable length array ‘buff64’ [-Wvla]

This also updates two cases where different enums were being compared
and explicitly casts them to int (which matches the old side-effect of
the single-evaluation code): one in tpm/tpm_tis_core.h, and one in
drm/drm_color_mgmt.c.

 [1] https://lkml.org/lkml/2018/3/7/621
 [2] https://lkml.org/lkml/2018/3/10/170
 [3] https://lkml.org/lkml/2018/3/20/845Co-Developed-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
Co-Developed-by: default avatarMartin Uecker <Martin.Uecker@med.uni-goettingen.de>
Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Acked-by: default avatarIngo Molnar <mingo@kernel.org>
Acked-by: default avatarMiguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 5414ab31
...@@ -62,10 +62,10 @@ enum tis_defaults { ...@@ -62,10 +62,10 @@ enum tis_defaults {
/* Some timeout values are needed before it is known whether the chip is /* Some timeout values are needed before it is known whether the chip is
* TPM 1.0 or TPM 2.0. * TPM 1.0 or TPM 2.0.
*/ */
#define TIS_TIMEOUT_A_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A) #define TIS_TIMEOUT_A_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
#define TIS_TIMEOUT_B_MAX max(TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B) #define TIS_TIMEOUT_B_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
#define TIS_TIMEOUT_C_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C) #define TIS_TIMEOUT_C_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
#define TIS_TIMEOUT_D_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D) #define TIS_TIMEOUT_D_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
#define TPM_ACCESS(l) (0x0000 | ((l) << 12)) #define TPM_ACCESS(l) (0x0000 | ((l) << 12))
#define TPM_INT_ENABLE(l) (0x0008 | ((l) << 12)) #define TPM_INT_ENABLE(l) (0x0008 | ((l) << 12))
......
...@@ -417,8 +417,8 @@ int drm_plane_create_color_properties(struct drm_plane *plane, ...@@ -417,8 +417,8 @@ int drm_plane_create_color_properties(struct drm_plane *plane,
{ {
struct drm_device *dev = plane->dev; struct drm_device *dev = plane->dev;
struct drm_property *prop; struct drm_property *prop;
struct drm_prop_enum_list enum_list[max(DRM_COLOR_ENCODING_MAX, struct drm_prop_enum_list enum_list[max_t(int, DRM_COLOR_ENCODING_MAX,
DRM_COLOR_RANGE_MAX)]; DRM_COLOR_RANGE_MAX)];
int i, len; int i, len;
if (WARN_ON(supported_encodings == 0 || if (WARN_ON(supported_encodings == 0 ||
......
...@@ -792,41 +792,58 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } ...@@ -792,41 +792,58 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
#endif /* CONFIG_TRACING */ #endif /* CONFIG_TRACING */
/* /*
* min()/max()/clamp() macros that also do * min()/max()/clamp() macros must accomplish three things:
* strict type-checking.. See the *
* "unnecessary" pointer comparison. * - avoid multiple evaluations of the arguments (so side-effects like
* "x++" happen only once) when non-constant.
* - perform strict type-checking (to generate warnings instead of
* nasty runtime surprises). See the "unnecessary" pointer comparison
* in __typecheck().
* - retain result as a constant expressions when called with only
* constant expressions (to avoid tripping VLA warnings in stack
* allocation usage).
*/
#define __typecheck(x, y) \
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
/*
* This returns a constant expression while determining if an argument is
* a constant expression, most importantly without evaluating the argument.
* Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
*/ */
#define __min(t1, t2, min1, min2, x, y) ({ \ #define __is_constexpr(x) \
t1 min1 = (x); \ (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
t2 min2 = (y); \
(void) (&min1 == &min2); \ #define __no_side_effects(x, y) \
min1 < min2 ? min1 : min2; }) (__is_constexpr(x) && __is_constexpr(y))
#define __safe_cmp(x, y) \
(__typecheck(x, y) && __no_side_effects(x, y))
#define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
#define __cmp_once(x, y, op) ({ \
typeof(x) __x = (x); \
typeof(y) __y = (y); \
__cmp(__x, __y, op); })
#define __careful_cmp(x, y, op) \
__builtin_choose_expr(__safe_cmp(x, y), \
__cmp(x, y, op), __cmp_once(x, y, op))
/** /**
* min - return minimum of two values of the same or compatible types * min - return minimum of two values of the same or compatible types
* @x: first value * @x: first value
* @y: second value * @y: second value
*/ */
#define min(x, y) \ #define min(x, y) __careful_cmp(x, y, <)
__min(typeof(x), typeof(y), \
__UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \
x, y)
#define __max(t1, t2, max1, max2, x, y) ({ \
t1 max1 = (x); \
t2 max2 = (y); \
(void) (&max1 == &max2); \
max1 > max2 ? max1 : max2; })
/** /**
* max - return maximum of two values of the same or compatible types * max - return maximum of two values of the same or compatible types
* @x: first value * @x: first value
* @y: second value * @y: second value
*/ */
#define max(x, y) \ #define max(x, y) __careful_cmp(x, y, >)
__max(typeof(x), typeof(y), \
__UNIQUE_ID(max1_), __UNIQUE_ID(max2_), \
x, y)
/** /**
* min3 - return minimum of three values * min3 - return minimum of three values
...@@ -878,10 +895,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } ...@@ -878,10 +895,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
* @x: first value * @x: first value
* @y: second value * @y: second value
*/ */
#define min_t(type, x, y) \ #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <)
__min(type, type, \
__UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \
x, y)
/** /**
* max_t - return maximum of two values, using the specified type * max_t - return maximum of two values, using the specified type
...@@ -889,10 +903,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } ...@@ -889,10 +903,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
* @x: first value * @x: first value
* @y: second value * @y: second value
*/ */
#define max_t(type, x, y) \ #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >)
__max(type, type, \
__UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \
x, y)
/** /**
* clamp_t - return a value clamped to a given range using a given type * clamp_t - return a value clamped to a given range using a given type
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment