* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.