* Re: [PATCH] nospec: Move array_index_nospec parameter checking into separate macro
2018-02-05 14:16 [PATCH] nospec: Move array_index_nospec parameter checking into separate macro Will Deacon
@ 2018-02-05 18:54 ` Dan Williams
2018-02-13 15:25 ` [tip:x86/pti] nospec: Move array_index_nospec() " tip-bot for Will Deacon
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Dan Williams @ 2018-02-05 18:54 UTC (permalink / raw)
To: Will Deacon; +Cc: Linux Kernel Mailing List, Ingo Molnar
On Mon, Feb 5, 2018 at 6:16 AM, Will Deacon <will.deacon@arm.com> 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 <dan.j.williams@intel.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
Looks ok to me and produces the same assembly.
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* [tip:x86/pti] nospec: Move array_index_nospec() parameter checking into separate macro
2018-02-05 14:16 [PATCH] nospec: Move array_index_nospec parameter checking into separate macro Will Deacon
2018-02-05 18:54 ` Dan Williams
@ 2018-02-13 15:25 ` tip-bot for Will Deacon
2018-02-15 0:28 ` tip-bot for Will Deacon
2018-02-19 11:47 ` [PATCH] nospec: Move array_index_nospec " Geert Uytterhoeven
3 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Will Deacon @ 2018-02-13 15:25 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, mingo, peterz, tglx, hpa, torvalds, dan.j.williams,
will.deacon
Commit-ID: 2963962dc910e57becfa0bb3df013b1d5c23179a
Gitweb: https://git.kernel.org/tip/2963962dc910e57becfa0bb3df013b1d5c23179a
Author: Will Deacon <will.deacon@arm.com>
AuthorDate: Mon, 5 Feb 2018 14:16:06 +0000
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Feb 2018 14:22:44 +0100
nospec: Move array_index_nospec() parameter checking into separate macro
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.
Signed-off-by: Will Deacon <will.deacon@arm.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1517840166-15399-1-git-send-email-will.deacon@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
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 b99bced..fbc98e2 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)); \
^ permalink raw reply related [flat|nested] 6+ messages in thread* [tip:x86/pti] nospec: Move array_index_nospec() parameter checking into separate macro
2018-02-05 14:16 [PATCH] nospec: Move array_index_nospec parameter checking into separate macro Will Deacon
2018-02-05 18:54 ` Dan Williams
2018-02-13 15:25 ` [tip:x86/pti] nospec: Move array_index_nospec() " tip-bot for Will Deacon
@ 2018-02-15 0:28 ` tip-bot for Will Deacon
2018-02-19 11:47 ` [PATCH] nospec: Move array_index_nospec " Geert Uytterhoeven
3 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Will Deacon @ 2018-02-15 0:28 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, torvalds, will.deacon, peterz, dan.j.williams, tglx, hpa,
linux-kernel
Commit-ID: 8fa80c503b484ddc1abbd10c7cb2ab81f3824a50
Gitweb: https://git.kernel.org/tip/8fa80c503b484ddc1abbd10c7cb2ab81f3824a50
Author: Will Deacon <will.deacon@arm.com>
AuthorDate: Mon, 5 Feb 2018 14:16:06 +0000
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 15 Feb 2018 01:15:51 +0100
nospec: Move array_index_nospec() parameter checking into separate macro
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.
Signed-off-by: Will Deacon <will.deacon@arm.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1517840166-15399-1-git-send-email-will.deacon@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
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 b99bced..fbc98e2 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)); \
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] nospec: Move array_index_nospec parameter checking into separate macro
2018-02-05 14:16 [PATCH] nospec: Move array_index_nospec parameter checking into separate macro Will Deacon
` (2 preceding siblings ...)
2018-02-15 0:28 ` tip-bot for Will Deacon
@ 2018-02-19 11:47 ` Geert Uytterhoeven
2018-02-19 11:54 ` Will Deacon
3 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-02-19 11:47 UTC (permalink / raw)
To: Will Deacon
Cc: Linux Kernel Mailing List, Dan Williams, Ingo Molnar,
Arnd Bergmann
Hi Will,
On Mon, Feb 5, 2018 at 3:16 PM, Will Deacon <will.deacon@arm.com> 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 <dan.j.williams@intel.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
> 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 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] nospec: Move array_index_nospec parameter checking into separate macro
2018-02-19 11:47 ` [PATCH] nospec: Move array_index_nospec " Geert Uytterhoeven
@ 2018-02-19 11:54 ` Will Deacon
0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2018-02-19 11:54 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux Kernel Mailing List, Dan Williams, Ingo Molnar,
Arnd Bergmann
Hi Geert,
On Mon, Feb 19, 2018 at 12:47:08PM +0100, Geert Uytterhoeven wrote:
> On Mon, Feb 5, 2018 at 3:16 PM, Will Deacon <will.deacon@arm.com> 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 <dan.j.williams@intel.com>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
[...]
> > @@ -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.
This is all getting ripped out, so stay tuned. The check is bogus, generates
crappy code and I did a poor job at macro-ising it. Apart from that, it's
great.
https://git.kernel.org/tip/1d91c1d2c80cb70e2e553845e278b87a960c04da
Will
^ permalink raw reply [flat|nested] 6+ messages in thread