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 the one case where different enums were being compared
and explicitly casts them to int (which matches the old side-effect of
the single-evaluation code).
[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/845
Co-Developed-by: Linus Torvalds <[email protected]>
Co-Developed-by: Martin Uecker <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
Let's see if this one sticks!
---
drivers/char/tpm/tpm_tis_core.h | 8 ++---
include/linux/kernel.h | 71 ++++++++++++++++++++++++-----------------
2 files changed, 45 insertions(+), 34 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index d5c6a2e952b3..f6e1dbe212a7 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -62,10 +62,10 @@ enum tis_defaults {
/* Some timeout values are needed before it is known whether the chip is
* TPM 1.0 or TPM 2.0.
*/
-#define TIS_TIMEOUT_A_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
-#define TIS_TIMEOUT_B_MAX max(TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
-#define TIS_TIMEOUT_C_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
-#define TIS_TIMEOUT_D_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
+#define TIS_TIMEOUT_A_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
+#define TIS_TIMEOUT_B_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
+#define TIS_TIMEOUT_C_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
+#define TIS_TIMEOUT_D_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
#define TPM_ACCESS(l) (0x0000 | ((l) << 12))
#define TPM_INT_ENABLE(l) (0x0008 | ((l) << 12))
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3fd291503576..a2c1b2a382dd 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -783,41 +783,58 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
#endif /* CONFIG_TRACING */
/*
- * min()/max()/clamp() macros that also do
- * strict type-checking.. See the
- * "unnecessary" pointer comparison.
+ * min()/max()/clamp() macros must accomplish three things:
+ *
+ * - 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 <[email protected]>
*/
-#define __min(t1, t2, min1, min2, x, y) ({ \
- t1 min1 = (x); \
- t2 min2 = (y); \
- (void) (&min1 == &min2); \
- min1 < min2 ? min1 : min2; })
+#define __is_constant(x) \
+ (sizeof(int) == sizeof(*(1 ? ((void*)((long)(x) * 0l)) : (int*)1)))
+
+#define __no_side_effects(x, y) \
+ (__is_constant(x) && __is_constant(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
* @x: first value
* @y: second value
*/
-#define min(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; })
+#define min(x, y) __careful_cmp(x, y, <)
/**
* max - return maximum of two values of the same or compatible types
* @x: first value
* @y: second value
*/
-#define max(x, y) \
- __max(typeof(x), typeof(y), \
- __UNIQUE_ID(max1_), __UNIQUE_ID(max2_), \
- x, y)
+#define max(x, y) __careful_cmp(x, y, >)
/**
* min3 - return minimum of three values
@@ -869,10 +886,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
* @x: first value
* @y: second value
*/
-#define min_t(type, x, y) \
- __min(type, type, \
- __UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \
- x, y)
+#define min_t(t, x, y) __careful_cmp((t)(x), (t)(y), <)
/**
* max_t - return maximum of two values, using the specified type
@@ -880,10 +894,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
* @x: first value
* @y: second value
*/
-#define max_t(type, x, y) \
- __max(type, type, \
- __UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \
- x, y)
+#define max_t(t, x, y) __careful_cmp((t)(x), (t)(y), >)
/**
* clamp_t - return a value clamped to a given range using a given type
--
2.7.4
--
Kees Cook
Pixel Security
On Mon, Mar 26, 2018 at 12:15 PM, Kees Cook <[email protected]> wrote:
>
> This patch updates the min()/max() macros to evaluate to a constant
> expression when called on constant expression arguments.
Ack.
I'm of two minds whether that "__is_constant()" macro should be
explained or not.
A small voice in my head says "that wants a comment".
But a bigger voice disagrees.
It is a work of art, and maybe the best documentation is just the
name. It does what it says it does.
Art shouldn't be explained. It should be appreciated.
Nobody sane really should care about how it works, and if somebody
cares it is "left as an exercise to the reader".
Linus
* Kees Cook <[email protected]> wrote:
> 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:
Cool!
Acked-by: Ingo Molnar <[email protected]>
How many warnings are left in an allmodconfig build?
Thanks,
Ingo
From: Kees Cook
> Sent: 26 March 2018 23:16
...
> +#define __typecheck(x, y) \
> + (!!(sizeof((typeof(x)*)1 == (typeof(y)*)1)))
The two 1 should probably be at least 8 before the compiler starts
bleating about accesses to misaligned addresses being undefined.
David
On Tue, Mar 27, 2018 at 12:15 AM, Kees Cook <[email protected]> wrote:
> 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 the one case where different enums were being compared
> and explicitly casts them to int (which matches the old side-effect of
> the single-evaluation code).
>
> [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/845
>
> Co-Developed-by: Linus Torvalds <[email protected]>
> Co-Developed-by: Martin Uecker <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> Let's see if this one sticks!
> ---
> drivers/char/tpm/tpm_tis_core.h | 8 ++---
> include/linux/kernel.h | 71 ++++++++++++++++++++++++-----------------
> 2 files changed, 45 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
> index d5c6a2e952b3..f6e1dbe212a7 100644
> --- a/drivers/char/tpm/tpm_tis_core.h
> +++ b/drivers/char/tpm/tpm_tis_core.h
> @@ -62,10 +62,10 @@ enum tis_defaults {
> /* Some timeout values are needed before it is known whether the chip is
> * TPM 1.0 or TPM 2.0.
> */
> -#define TIS_TIMEOUT_A_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
> -#define TIS_TIMEOUT_B_MAX max(TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
> -#define TIS_TIMEOUT_C_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
> -#define TIS_TIMEOUT_D_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
> +#define TIS_TIMEOUT_A_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
> +#define TIS_TIMEOUT_B_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
> +#define TIS_TIMEOUT_C_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
> +#define TIS_TIMEOUT_D_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
>
> #define TPM_ACCESS(l) (0x0000 | ((l) << 12))
> #define TPM_INT_ENABLE(l) (0x0008 | ((l) << 12))
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 3fd291503576..a2c1b2a382dd 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -783,41 +783,58 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
> #endif /* CONFIG_TRACING */
>
> /*
> - * min()/max()/clamp() macros that also do
> - * strict type-checking.. See the
> - * "unnecessary" pointer comparison.
> + * min()/max()/clamp() macros must accomplish three things:
> + *
> + * - 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 <[email protected]>
> */
> -#define __min(t1, t2, min1, min2, x, y) ({ \
> - t1 min1 = (x); \
> - t2 min2 = (y); \
> - (void) (&min1 == &min2); \
> - min1 < min2 ? min1 : min2; })
> +#define __is_constant(x) \
> + (sizeof(int) == sizeof(*(1 ? ((void*)((long)(x) * 0l)) : (int*)1)))
I think Linus suggested __is_constant, but what about __is_constexpr
instead? It makes the intention a bit more clear and follows the C++'s
specifier.
Acked-by: Miguel Ojeda <[email protected]>
Cheers,
Miguel
> +
> +#define __no_side_effects(x, y) \
> + (__is_constant(x) && __is_constant(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
> * @x: first value
> * @y: second value
> */
> -#define min(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; })
> +#define min(x, y) __careful_cmp(x, y, <)
>
> /**
> * max - return maximum of two values of the same or compatible types
> * @x: first value
> * @y: second value
> */
> -#define max(x, y) \
> - __max(typeof(x), typeof(y), \
> - __UNIQUE_ID(max1_), __UNIQUE_ID(max2_), \
> - x, y)
> +#define max(x, y) __careful_cmp(x, y, >)
>
> /**
> * min3 - return minimum of three values
> @@ -869,10 +886,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
> * @x: first value
> * @y: second value
> */
> -#define min_t(type, x, y) \
> - __min(type, type, \
> - __UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \
> - x, y)
> +#define min_t(t, x, y) __careful_cmp((t)(x), (t)(y), <)
>
> /**
> * max_t - return maximum of two values, using the specified type
> @@ -880,10 +894,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
> * @x: first value
> * @y: second value
> */
> -#define max_t(type, x, y) \
> - __max(type, type, \
> - __UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \
> - x, y)
> +#define max_t(t, x, y) __careful_cmp((t)(x), (t)(y), >)
>
> /**
> * clamp_t - return a value clamped to a given range using a given type
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
Hi Kees,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc7 next-20180326]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/kernel-h-Retain-constant-expression-output-for-max-min/20180327-110916
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
include/linux/kernel.h:889: warning: Excess function parameter 'type' description in 'min_t'
include/linux/kernel.h:897: warning: Excess function parameter 'type' description in 'max_t'
include/linux/kernel.h:889: warning: Excess function parameter 'type' description in 'min_t'
include/linux/kernel.h:897: warning: Excess function parameter 'type' description in 'max_t'
include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.ablkcipher' not described in 'crypto_alg'
include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.blkcipher' not described in 'crypto_alg'
include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.cipher' not described in 'crypto_alg'
include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.compress' not described in 'crypto_alg'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.prev_bssid' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev'
include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev'
include/net/mac80211.h:2263: warning: Function parameter or member 'radiotap_timestamp.units_pos' not described in 'ieee80211_hw'
include/net/mac80211.h:2263: warning: Function parameter or member 'radiotap_timestamp.accuracy' not described in 'ieee80211_hw'
include/net/mac80211.h:950: warning: Function parameter or member 'rates' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.use_rts' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.use_cts_prot' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.short_preamble' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.skip_table' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.jiffies' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.vif' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.hw_key' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.flags' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'control.enqueue_time' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'ack' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'status_driver_data' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info'
include/net/mac80211.h:950: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'msdu_retries' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'msdu_failed' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
net/mac80211/sta_info.h:584: warning: Function parameter or member 'msdu' not described in 'sta_info'
>> include/linux/kernel.h:890: warning: Function parameter or member 't' not described in 'min_t'
include/linux/kernel.h:890: warning: Excess function parameter 'type' description in 'min_t'
>> include/linux/kernel.h:898: warning: Function parameter or member 't' not described in 'max_t'
include/linux/kernel.h:898: warning: Excess function parameter 'type' description in 'max_t'
include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf'
include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf'
include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf'
include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf'
include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf'
include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf'
include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array'
include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip'
include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.sign' not described in 'iio_chan_spec'
include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.realbits' not described in 'iio_chan_spec'
include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.storagebits' not described in 'iio_chan_spec'
include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.shift' not described in 'iio_chan_spec'
include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.repeat' not described in 'iio_chan_spec'
include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.endianness' not described in 'iio_chan_spec'
include/linux/iio/hw-consumer.h:1: warning: no structured comments found
include/linux/device.h:294: warning: Function parameter or member 'coredump' not described in 'device_driver'
include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry'
include/linux/mtd/rawnand.h:709: warning: Function parameter or member 'timings.sdr' not described in 'nand_data_interface'
include/linux/mtd/rawnand.h:774: warning: Function parameter or member 'buf' not described in 'nand_op_data_instr'
include/linux/mtd/rawnand.h:774: warning: Function parameter or member 'buf.in' not described in 'nand_op_data_instr'
include/linux/mtd/rawnand.h:774: warning: Function parameter or member 'buf.out' not described in 'nand_op_data_instr'
include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx' not described in 'nand_op_instr'
include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.cmd' not described in 'nand_op_instr'
include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_instr'
include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.data' not described in 'nand_op_instr'
include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.waitrdy' not described in 'nand_op_instr'
include/linux/mtd/rawnand.h:967: warning: Function parameter or member 'ctx' not described in 'nand_op_parser_pattern_elem'
include/linux/mtd/rawnand.h:967: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_parser_pattern_elem'
include/linux/mtd/rawnand.h:967: warning: Function parameter or member 'ctx.data' not described in 'nand_op_parser_pattern_elem'
include/linux/mtd/rawnand.h:1281: warning: Function parameter or member 'manufacturer.desc' not described in 'nand_chip'
include/linux/mtd/rawnand.h:1281: warning: Function parameter or member 'manufacturer.priv' not described in 'nand_chip'
include/linux/regulator/driver.h:221: warning: Function parameter or member 'resume_early' not described in 'regulator_ops'
drivers/regulator/core.c:4299: warning: Excess function parameter 'state' description in 'regulator_suspend_late'
arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb'
arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb'
arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb'
arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb'
arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb'
include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver'
include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver'
include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver'
include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver'
include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver'
include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver'
include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver'
include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.left' not described in 'drm_tv_connector_state'
include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.right' not described in 'drm_tv_connector_state'
include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.top' not described in 'drm_tv_connector_state'
include/drm/drm_connector.h:370: warning: Function parameter or member 'margins.bottom' not described in 'drm_tv_connector_state'
include/drm/drm_vblank.h:63: warning: Function parameter or member 'event.base' not described in 'drm_pending_vblank_event'
include/drm/drm_vblank.h:63: warning: Function parameter or member 'event.vbl' not described in 'drm_pending_vblank_event'
include/drm/drm_vblank.h:63: warning: Function parameter or member 'event.seq' not described in 'drm_pending_vblank_event'
include/linux/skbuff.h:846: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'skb_mstamp' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member '__unused' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'pfmemalloc' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
include/linux/skbuff.h:846: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
include/net/sock.h:234: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
include/net/sock.h:234: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock'
include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.len' not described in 'sock'
include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.head' not described in 'sock'
include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock'
include/net/sock.h:487: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
include/net/sock.h:487: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
include/linux/netdevice.h:1940: warning: Function parameter or member 'adj_list.upper' not described in 'net_device'
include/linux/netdevice.h:1940: warning: Function parameter or member 'adj_list.lower' not described in 'net_device'
include/linux/netdevice.h:1940: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
include/linux/netdevice.h:1940: warning: Function parameter or member 'switchdev_ops' not described in 'net_device'
include/linux/netdevice.h:1940: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
include/linux/netdevice.h:1940: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
include/linux/netdevice.h:1940: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
include/linux/netdevice.h:1940: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
include/linux/netdevice.h:1940: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
include/linux/netdevice.h:1940: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
vim +890 include/linux/kernel.h
589a9785e Johannes Berg 2016-10-07 @890
e8c97af0c Randy Dunlap 2017-10-13 891 /**
e8c97af0c Randy Dunlap 2017-10-13 892 * max_t - return maximum of two values, using the specified type
e8c97af0c Randy Dunlap 2017-10-13 893 * @type: data type to use
e8c97af0c Randy Dunlap 2017-10-13 894 * @x: first value
e8c97af0c Randy Dunlap 2017-10-13 895 * @y: second value
e8c97af0c Randy Dunlap 2017-10-13 896 */
2ac4e7c3a Kees Cook 2018-03-26 897 #define max_t(t, x, y) __careful_cmp((t)(x), (t)(y), >)
bdf4bbaae Harvey Harrison 2008-04-30 @898
:::::: The code at line 890 was first introduced by commit
:::::: 589a9785ee3a7cb85f1dedc3dad1c9754c691880 min/max: remove sparse warnings when they're nested
:::::: TO: Johannes Berg <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
To give credit where credit is due, this hack was inspired by
an equally insane (but different) use of the ?: operator to choose
the right return type for type-generic macros in tgmath.h.
https://sourceware.org/git/?p=glibc.git;a=blob;f=math/tgmath.h;h=a709a5
9d0fa1168ef03349561169fc5bd27d65aa;hb=d8742dd82f6a00601155c69bad3012e90
5591e1f
(recommendation: don't look)
Martin
Am Montag, den 26.03.2018, 14:52 -1000 schrieb Linus Torvalds:
> On Mon, Mar 26, 2018 at 12:15 PM, Kees Cook <[email protected]>
> wrote:
> >
> > This patch updates the min()/max() macros to evaluate to a constant
> > expression when called on constant expression arguments.
>
> Ack.
>
> I'm of two minds whether that "__is_constant()" macro should be
> explained or not.
>
> A small voice in my head says "that wants a comment".
>
> But a bigger voice disagrees.
>
> It is a work of art, and maybe the best documentation is just the
> name. It does what it says it does.
>
> Art shouldn't be explained. It should be appreciated.
>
> Nobody sane really should care about how it works, and if somebody
> cares it is "left as an exercise to the reader".
>
> Linus
Hi Kees,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc7 next-20180327]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/kernel-h-Retain-constant-expression-output-for-max-min/20180327-110916
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
vim +124 drivers/infiniband/hw/vmw_pvrdma/pvrdma_doorbell.c
29c8d9eb Adit Ranadive 2016-10-02 114
29c8d9eb Adit Ranadive 2016-10-02 115 void pvrdma_uar_free(struct pvrdma_dev *dev, struct pvrdma_uar_map *uar)
29c8d9eb Adit Ranadive 2016-10-02 116 {
29c8d9eb Adit Ranadive 2016-10-02 117 struct pvrdma_id_table *tbl = &dev->uar_table.tbl;
29c8d9eb Adit Ranadive 2016-10-02 118 unsigned long flags;
29c8d9eb Adit Ranadive 2016-10-02 119 u32 obj;
29c8d9eb Adit Ranadive 2016-10-02 120
29c8d9eb Adit Ranadive 2016-10-02 121 obj = uar->index & (tbl->max - 1);
29c8d9eb Adit Ranadive 2016-10-02 122 spin_lock_irqsave(&tbl->lock, flags);
29c8d9eb Adit Ranadive 2016-10-02 123 clear_bit(obj, tbl->table);
29c8d9eb Adit Ranadive 2016-10-02 @124 tbl->last = min(tbl->last, obj);
:::::: The code at line 124 was first introduced by commit
:::::: 29c8d9eba550c6d73d17cc1618a9f5f2a7345aa1 IB: Add vmw_pvrdma driver
:::::: TO: Adit Ranadive <[email protected]>
:::::: CC: Doug Ledford <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, Mar 26, 2018 at 10:47 PM, Ingo Molnar <[email protected]> wrote:
>
> * Kees Cook <[email protected]> wrote:
>
>> 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:
>
> Cool!
>
> Acked-by: Ingo Molnar <[email protected]>
>
> How many warnings are left in an allmodconfig build?
For -Wvla? Out of the original 112 files with VLAs, 42 haven't had a
patch applied yet. Doing a linux-next allmodconfig build with the
max() patch and my latest ecc patch, we've gone from 316 warning
instances to 205. More than half of those are in
include/crypto/skcipher.h and include/crypto/hash.h.
-Kees
--
Kees Cook
Pixel Security
* Kees Cook <[email protected]> wrote:
> On Mon, Mar 26, 2018 at 10:47 PM, Ingo Molnar <[email protected]> wrote:
> >
> > * Kees Cook <[email protected]> wrote:
> >
> >> 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:
> >
> > Cool!
> >
> > Acked-by: Ingo Molnar <[email protected]>
> >
> > How many warnings are left in an allmodconfig build?
>
> For -Wvla? Out of the original 112 files with VLAs, 42 haven't had a
> patch applied yet. Doing a linux-next allmodconfig build with the
> max() patch and my latest ecc patch, we've gone from 316 warning
> instances to 205. More than half of those are in
> include/crypto/skcipher.h and include/crypto/hash.h.
Great - once the number of warnings is zero, is the plan to enable the warning
unconditionally?
Thanks,
Ingo
On Fri, Mar 30, 2018 at 10:34 PM, Ingo Molnar <[email protected]> wrote:
>
> * Kees Cook <[email protected]> wrote:
>
>> On Mon, Mar 26, 2018 at 10:47 PM, Ingo Molnar <[email protected]> wrote:
>> >
>> > * Kees Cook <[email protected]> wrote:
>> >
>> >> 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:
>> >
>> > Cool!
>> >
>> > Acked-by: Ingo Molnar <[email protected]>
>> >
>> > How many warnings are left in an allmodconfig build?
>>
>> For -Wvla? Out of the original 112 files with VLAs, 42 haven't had a
>> patch applied yet. Doing a linux-next allmodconfig build with the
>> max() patch and my latest ecc patch, we've gone from 316 warning
>> instances to 205. More than half of those are in
>> include/crypto/skcipher.h and include/crypto/hash.h.
>
> Great - once the number of warnings is zero, is the plan to enable the warning
> unconditionally?
That's the plan, yes. Like inbox-zero but for VLAs. ;)
-Kees
--
Kees Cook
Pixel Security