Hi Will,
On Mon, Feb 5, 2018 at 3:16 PM, Will Deacon <[email protected]> wrote:
> For architectures providing their own implementation of
> array_index_mask_nospec in asm/barrier.h, attempting to use WARN_ONCE to
> complain about out-of-range parameters using WARN_ON results in a mess
> of mutually-dependent include files.
>
> Rather than unpick the dependencies, simply have the core code in nospec.h
> perform the checking for us.
>
> Cc: Dan Williams <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Signed-off-by: Will Deacon <[email protected]>
> ---
> include/linux/nospec.h | 36 +++++++++++++++++++++---------------
> 1 file changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/nospec.h b/include/linux/nospec.h
> index b99bced39ac2..fbc98e2c8228 100644
> --- a/include/linux/nospec.h
> +++ b/include/linux/nospec.h
> @@ -20,20 +20,6 @@ static inline unsigned long
> array_index_mask_nospec(unsigned long index,
> unsigned long size)
> {
> /*
> - * Warn developers about inappropriate array_index_nospec() usage.
> - *
> - * Even if the CPU speculates past the WARN_ONCE branch, the
> - * sign bit of @index is taken into account when generating the
> - * mask.
> - *
> - * This warning is compiled out when the compiler can infer that
> - * @index and @size are less than LONG_MAX.
> - */
> - if (WARN_ONCE(index > LONG_MAX || size > LONG_MAX,
> - "array_index_nospec() limited to range of [0,
> LONG_MAX]\n"))
> - return 0;
> -
> - /*
> * Always calculate and emit the mask even if the compiler
> * thinks the mask is not needed. The compiler does not take
> * into account the value of @index under speculation.
> @@ -44,6 +30,26 @@ static inline unsigned long
> array_index_mask_nospec(unsigned long index,
> #endif
>
> /*
> + * Warn developers about inappropriate array_index_nospec() usage.
> + *
> + * Even if the CPU speculates past the WARN_ONCE branch, the
> + * sign bit of @index is taken into account when generating the
> + * mask.
> + *
> + * This warning is compiled out when the compiler can infer that
> + * @index and @size are less than LONG_MAX.
> + */
> +#define array_index_mask_nospec_check(index, size)
> \
> +({
> \
> + if (WARN_ONCE(index > LONG_MAX || size > LONG_MAX,
> \
> + "array_index_nospec() limited to range of [0, LONG_MAX]\n"))
> \
> + _mask = 0;
> \
> + else
> \
> + _mask = array_index_mask_nospec(index, size);
> \
> + _mask;
> \
> +})
> +
> +/*
> * array_index_nospec - sanitize an array index after a bounds check
> *
> * For a code sequence like:
> @@ -61,7 +67,7 @@ static inline unsigned long
> array_index_mask_nospec(unsigned long index,
> ({ \
> typeof(index) _i = (index); \
> typeof(size) _s = (size); \
> - unsigned long _mask = array_index_mask_nospec(_i, _s); \
> + unsigned long _mask = array_index_mask_nospec_check(_i, _s); \
> \
> BUILD_BUG_ON(sizeof(_i) > sizeof(long)); \
> BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \
This change is commit 8fa80c503b484ddc ("nospec: Move array_index_nospec()
parameter checking into separate macro") in v4.16-rc2, and triggers the
following warning with gcc-4.1.2:
net/wireless/nl80211.c: In function ‘parse_txq_params’:
net/wireless/nl80211.c:2099: warning: comparison is always false
due to limited range of data type
Reverting the commit gets rid of the warning.
As kisskb hasn't completed the full build of v4.16-rc2 yet, I don't know if
other compiler versions are affected.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
