* [PATCH 0/3] nospec: Various fix-ups for include/linux/nospec.h
@ 2018-02-16 21:20 Dan Williams
2018-02-16 21:20 ` [PATCH 1/3] nospec: Kill array_index_nospec_mask_check() Dan Williams
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Dan Williams @ 2018-02-16 21:20 UTC (permalink / raw)
To: mingo
Cc: linux-arch, Rasmus Villemoes, Will Deacon, linux-kernel, stable,
Christian Borntraeger, Thomas Gleixner, Linus Torvalds
Hi Ingo,
Here is a small pile of cleanups and fixes for nospec.h after inspection
from Linus, Rasmus, and Christian. Full changelogs below:
These have received a build success notification from 0day across 126
configs.
---
Dan Williams (2):
nospec: Kill array_index_nospec_mask_check()
nospec: Include asm/barrier.h dependency
Rasmus Villemoes (1):
nospec: Allow index argument to have const-qualified type
include/linux/nospec.h | 26 +++-----------------------
1 file changed, 3 insertions(+), 23 deletions(-)
--
nospec: Kill array_index_nospec_mask_check()
There are multiple problems with the dynamic sanity checking in
array_index_nospec_mask_check():
* It causes unnecessary overhead in the 32-bit case since integer sized
@index values will no longer cause the check to be compiled away like
in the 64-bit case.
* In the 32-bit case it may trigger with user controllable input when
the expectation is that should only trigger during development of new
kernel enabling.
* The macro reuses the input parameter in multiple locations which is
broken if someone passes an expression like 'index++' to
array_index_nospec().
nospec: Allow index argument to have const-qualified type
The last expression in a statement expression need not be a bare
variable, quoting gcc docs
The last thing in the compound statement should be an expression
followed by a semicolon; the value of this subexpression serves as the
value of the entire construct.
and we already use that in e.g. the min/max macros which end with a
ternary expression.
This way, we can allow index to have const-qualified type, which will in
some cases avoid the need for introducing a local copy of index of
non-const qualified type. That, in turn, can prevent readers not
familiar with the internals of array_index_nospec from wondering about
the seemingly redundant extra variable, and I think that's worthwhile
considering how confusing the whole _nospec business is.
The expression _i&_mask has type unsigned long (since that is the type
of _mask, and the BUILD_BUG_ONs guarantee that _i will get promoted to
that), so in order not to change the type of the whole expression, add
a cast back to typeof(_i).
nospec: Include asm/barrier.h dependency
The nospec.h header expects the per-architecture header file
asm/barrier.h to optionally define array_index_mask_nospec(). Include
that dependency to prevent inadvertent fallback to the default
array_index_mask_nospec() implementation. The default implementation may
not provide a full mitigation on architectures that perform data value
speculation.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] nospec: Kill array_index_nospec_mask_check()
2018-02-16 21:20 [PATCH 0/3] nospec: Various fix-ups for include/linux/nospec.h Dan Williams
@ 2018-02-16 21:20 ` Dan Williams
2018-02-17 10:10 ` [tip:x86/pti] " tip-bot for Dan Williams
2018-02-16 21:20 ` [PATCH 2/3] nospec: Allow index argument to have const-qualified type Dan Williams
2018-02-16 21:20 ` [PATCH 3/3] nospec: Include asm/barrier.h dependency Dan Williams
2 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2018-02-16 21:20 UTC (permalink / raw)
To: mingo
Cc: linux-arch, Will Deacon, Thomas Gleixner, Linus Torvalds, linux-kernel
There are multiple problems with the dynamic sanity checking in
array_index_nospec_mask_check():
* It causes unnecessary overhead in the 32-bit case since integer sized
@index values will no longer cause the check to be compiled away like
in the 64-bit case.
* In the 32-bit case it may trigger with user controllable input when
the expectation is that should only trigger during development of new
kernel enabling.
* The macro reuses the input parameter in multiple locations which is
broken if someone passes an expression like 'index++' to
array_index_nospec().
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
include/linux/nospec.h | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/include/linux/nospec.h b/include/linux/nospec.h
index fbc98e2c8228..d6701e34424f 100644
--- a/include/linux/nospec.h
+++ b/include/linux/nospec.h
@@ -30,26 +30,6 @@ 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:
@@ -67,7 +47,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_check(_i, _s); \
+ unsigned long _mask = array_index_mask_nospec(_i, _s); \
\
BUILD_BUG_ON(sizeof(_i) > sizeof(long)); \
BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] nospec: Allow index argument to have const-qualified type
2018-02-16 21:20 [PATCH 0/3] nospec: Various fix-ups for include/linux/nospec.h Dan Williams
2018-02-16 21:20 ` [PATCH 1/3] nospec: Kill array_index_nospec_mask_check() Dan Williams
@ 2018-02-16 21:20 ` Dan Williams
2018-02-17 10:11 ` [tip:x86/pti] " tip-bot for Rasmus Villemoes
2018-02-16 21:20 ` [PATCH 3/3] nospec: Include asm/barrier.h dependency Dan Williams
2 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2018-02-16 21:20 UTC (permalink / raw)
To: mingo
Cc: linux-arch, Rasmus Villemoes, Will Deacon, linux-kernel, stable,
Thomas Gleixner, Linus Torvalds
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
The last expression in a statement expression need not be a bare
variable, quoting gcc docs
The last thing in the compound statement should be an expression
followed by a semicolon; the value of this subexpression serves as the
value of the entire construct.
and we already use that in e.g. the min/max macros which end with a
ternary expression.
This way, we can allow index to have const-qualified type, which will in
some cases avoid the need for introducing a local copy of index of
non-const qualified type. That, in turn, can prevent readers not
familiar with the internals of array_index_nospec from wondering about
the seemingly redundant extra variable, and I think that's worthwhile
considering how confusing the whole _nospec business is.
The expression _i&_mask has type unsigned long (since that is the type
of _mask, and the BUILD_BUG_ONs guarantee that _i will get promoted to
that), so in order not to change the type of the whole expression, add
a cast back to typeof(_i).
Cc: stable@vger.kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
include/linux/nospec.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/nospec.h b/include/linux/nospec.h
index d6701e34424f..172a19dc35ab 100644
--- a/include/linux/nospec.h
+++ b/include/linux/nospec.h
@@ -52,7 +52,6 @@ static inline unsigned long array_index_mask_nospec(unsigned long index,
BUILD_BUG_ON(sizeof(_i) > sizeof(long)); \
BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \
\
- _i &= _mask; \
- _i; \
+ (typeof(_i)) (_i & _mask); \
})
#endif /* _LINUX_NOSPEC_H */
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] nospec: Include asm/barrier.h dependency
2018-02-16 21:20 [PATCH 0/3] nospec: Various fix-ups for include/linux/nospec.h Dan Williams
2018-02-16 21:20 ` [PATCH 1/3] nospec: Kill array_index_nospec_mask_check() Dan Williams
2018-02-16 21:20 ` [PATCH 2/3] nospec: Allow index argument to have const-qualified type Dan Williams
@ 2018-02-16 21:20 ` Dan Williams
2018-02-17 10:11 ` [tip:x86/pti] nospec: Include <asm/barrier.h> dependency tip-bot for Dan Williams
2 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2018-02-16 21:20 UTC (permalink / raw)
To: mingo
Cc: linux-arch, Will Deacon, linux-kernel, Christian Borntraeger,
Thomas Gleixner, Linus Torvalds
The nospec.h header expects the per-architecture header file
asm/barrier.h to optionally define array_index_mask_nospec(). Include
that dependency to prevent inadvertent fallback to the default
array_index_mask_nospec() implementation. The default implementation may
not provide a full mitigation on architectures that perform data value
speculation.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
include/linux/nospec.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/nospec.h b/include/linux/nospec.h
index 172a19dc35ab..e791ebc65c9c 100644
--- a/include/linux/nospec.h
+++ b/include/linux/nospec.h
@@ -5,6 +5,7 @@
#ifndef _LINUX_NOSPEC_H
#define _LINUX_NOSPEC_H
+#include <asm/barrier.h>
/**
* array_index_mask_nospec() - generate a ~0 mask when index < size, 0 otherwise
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:x86/pti] nospec: Kill array_index_nospec_mask_check()
2018-02-16 21:20 ` [PATCH 1/3] nospec: Kill array_index_nospec_mask_check() Dan Williams
@ 2018-02-17 10:10 ` tip-bot for Dan Williams
0 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Dan Williams @ 2018-02-17 10:10 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, hpa, dave.hansen, bp, peterz, linux-kernel, arjan, gregkh,
dwmw2, dan.j.williams, jpoimboe, torvalds, will.deacon, luto,
mingo
Commit-ID: 1d91c1d2c80cb70e2e553845e278b87a960c04da
Gitweb: https://git.kernel.org/tip/1d91c1d2c80cb70e2e553845e278b87a960c04da
Author: Dan Williams <dan.j.williams@intel.com>
AuthorDate: Fri, 16 Feb 2018 13:20:42 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 17 Feb 2018 08:40:59 +0100
nospec: Kill array_index_nospec_mask_check()
There are multiple problems with the dynamic sanity checking in
array_index_nospec_mask_check():
* It causes unnecessary overhead in the 32-bit case since integer sized
@index values will no longer cause the check to be compiled away like
in the 64-bit case.
* In the 32-bit case it may trigger with user controllable input when
the expectation is that should only trigger during development of new
kernel enabling.
* The macro reuses the input parameter in multiple locations which is
broken if someone passes an expression like 'index++' to
array_index_nospec().
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-arch@vger.kernel.org
Link: http://lkml.kernel.org/r/151881604278.17395.6605847763178076520.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/nospec.h | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/include/linux/nospec.h b/include/linux/nospec.h
index fbc98e2..d6701e3 100644
--- a/include/linux/nospec.h
+++ b/include/linux/nospec.h
@@ -30,26 +30,6 @@ 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:
@@ -67,7 +47,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_check(_i, _s); \
+ unsigned long _mask = array_index_mask_nospec(_i, _s); \
\
BUILD_BUG_ON(sizeof(_i) > sizeof(long)); \
BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:x86/pti] nospec: Allow index argument to have const-qualified type
2018-02-16 21:20 ` [PATCH 2/3] nospec: Allow index argument to have const-qualified type Dan Williams
@ 2018-02-17 10:11 ` tip-bot for Rasmus Villemoes
0 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Rasmus Villemoes @ 2018-02-17 10:11 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, will.deacon, peterz, linux, arjan, dan.j.williams, bp,
torvalds, gregkh, dwmw2, jpoimboe, linux-kernel, mingo,
dave.hansen, hpa, luto
Commit-ID: b98c6a160a057d5686a8c54c79cc6c8c94a7d0c8
Gitweb: https://git.kernel.org/tip/b98c6a160a057d5686a8c54c79cc6c8c94a7d0c8
Author: Rasmus Villemoes <linux@rasmusvillemoes.dk>
AuthorDate: Fri, 16 Feb 2018 13:20:48 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 17 Feb 2018 08:40:59 +0100
nospec: Allow index argument to have const-qualified type
The last expression in a statement expression need not be a bare
variable, quoting gcc docs
The last thing in the compound statement should be an expression
followed by a semicolon; the value of this subexpression serves as the
value of the entire construct.
and we already use that in e.g. the min/max macros which end with a
ternary expression.
This way, we can allow index to have const-qualified type, which will in
some cases avoid the need for introducing a local copy of index of
non-const qualified type. That, in turn, can prevent readers not
familiar with the internals of array_index_nospec from wondering about
the seemingly redundant extra variable, and I think that's worthwhile
considering how confusing the whole _nospec business is.
The expression _i&_mask has type unsigned long (since that is the type
of _mask, and the BUILD_BUG_ONs guarantee that _i will get promoted to
that), so in order not to change the type of the whole expression, add
a cast back to typeof(_i).
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-arch@vger.kernel.org
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/151881604837.17395.10812767547837568328.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/nospec.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/nospec.h b/include/linux/nospec.h
index d6701e3..172a19d 100644
--- a/include/linux/nospec.h
+++ b/include/linux/nospec.h
@@ -52,7 +52,6 @@ static inline unsigned long array_index_mask_nospec(unsigned long index,
BUILD_BUG_ON(sizeof(_i) > sizeof(long)); \
BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \
\
- _i &= _mask; \
- _i; \
+ (typeof(_i)) (_i & _mask); \
})
#endif /* _LINUX_NOSPEC_H */
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:x86/pti] nospec: Include <asm/barrier.h> dependency
2018-02-16 21:20 ` [PATCH 3/3] nospec: Include asm/barrier.h dependency Dan Williams
@ 2018-02-17 10:11 ` tip-bot for Dan Williams
0 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Dan Williams @ 2018-02-17 10:11 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, borntraeger, jpoimboe, hpa, bp, linux-kernel, arjan,
peterz, torvalds, gregkh, luto, dwmw2, dan.j.williams, tglx,
will.deacon, dave.hansen
Commit-ID: eb6174f6d1be16b19cfa43dac296bfed003ce1a6
Gitweb: https://git.kernel.org/tip/eb6174f6d1be16b19cfa43dac296bfed003ce1a6
Author: Dan Williams <dan.j.williams@intel.com>
AuthorDate: Fri, 16 Feb 2018 13:20:54 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 17 Feb 2018 08:40:59 +0100
nospec: Include <asm/barrier.h> dependency
The nospec.h header expects the per-architecture header file
<asm/barrier.h> to optionally define array_index_mask_nospec(). Include
that dependency to prevent inadvertent fallback to the default
array_index_mask_nospec() implementation.
The default implementation may not provide a full mitigation
on architectures that perform data value speculation.
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-arch@vger.kernel.org
Link: http://lkml.kernel.org/r/151881605404.17395.1341935530792574707.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/nospec.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/nospec.h b/include/linux/nospec.h
index 172a19d..e791ebc 100644
--- a/include/linux/nospec.h
+++ b/include/linux/nospec.h
@@ -5,6 +5,7 @@
#ifndef _LINUX_NOSPEC_H
#define _LINUX_NOSPEC_H
+#include <asm/barrier.h>
/**
* array_index_mask_nospec() - generate a ~0 mask when index < size, 0 otherwise
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-02-17 10:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-16 21:20 [PATCH 0/3] nospec: Various fix-ups for include/linux/nospec.h Dan Williams
2018-02-16 21:20 ` [PATCH 1/3] nospec: Kill array_index_nospec_mask_check() Dan Williams
2018-02-17 10:10 ` [tip:x86/pti] " tip-bot for Dan Williams
2018-02-16 21:20 ` [PATCH 2/3] nospec: Allow index argument to have const-qualified type Dan Williams
2018-02-17 10:11 ` [tip:x86/pti] " tip-bot for Rasmus Villemoes
2018-02-16 21:20 ` [PATCH 3/3] nospec: Include asm/barrier.h dependency Dan Williams
2018-02-17 10:11 ` [tip:x86/pti] nospec: Include <asm/barrier.h> dependency tip-bot for Dan Williams
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).