kernel.h: Retain constant expression output for max()/min()
PATCH v6 1/1 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 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
---
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(-)
drivers/char/tpm/tpm_tis_core.h +4 -4
@@ -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))
include/linux/kernel.h +41 -30
102 lines not on this page · raw message
Signature · 6 lines
2.7.4 -- Kees Cook Pixel Security
Quoted 3 lines · Kees Cook
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
Quoted 7 lines · Kees Cook
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.
Quoted 8 lines
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!
How many warnings are left in an allmodconfig build?
Thanks,
Ingo
From: Kees Cook
Quoted 1 line
Sent: 26 March 2018 23:16
...
include/linux/kernel.h static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } · line 798
Quoted 2 lines
include/linux/kernel.h +2
-0+#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
Quoted 88 lines · Kees Cook
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 ^-(quoted diff) +0
-6-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/845Co-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(-)drivers/char/tpm/tpm_tis_core.h +4
-4@@ -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))include/linux/kernel.h +16
-3@@ -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
include/linux/kernel.h static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } · line 806
Quoted 10 lines
include/linux/kernel.h +4
-5+* 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.
Cheers,
Miguel
Quoted 79 lines
include/linux/kernel.h +21
-23+ +#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
kbuild test robot
build botI love your patch! Perhaps something to improve:
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:
Quoted 24 lines
Quoted 3 lines · Kees Cook
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
kbuild test robot
build botI love your patch! Perhaps something to improve:
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
Quoted 19 lines · Ingo Molnar
Quoted 7 lines · Kees Cook
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.Quoted 8 lines
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:
Quoted 5 lines
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
Signature · 2 lines
Kees Cook Pixel Security
Quoted 24 lines · Kees Cook
Quoted 23 lines · Ingo Molnar
Quoted 7 lines · Kees Cook
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.Quoted 8 lines
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]>
Quoted 7 lines
Quoted 1 line
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 ininclude/crypto/skcipher.handinclude/crypto/hash.h.
Great - once the number of warnings is zero, is the plan to enable the warning unconditionally?
Thanks,
Ingo
Quoted 29 lines · Ingo Molnar
Quoted 26 lines · Kees Cook
Quoted 24 lines · Ingo Molnar
Quoted 7 lines · Kees Cook
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.Quoted 8 lines
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?
Quoted 8 lines
Quoted 5 lines
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 ininclude/crypto/skcipher.handinclude/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
Signature · 2 lines
Kees Cook Pixel Security