| Message ID | 20190930182437.25478-2-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/nospec: Add Kconfig options for speculative hardening | expand |
On 30.09.2019 20:24, Andrew Cooper wrote: > --- a/xen/common/Kconfig > +++ b/xen/common/Kconfig > @@ -77,6 +77,27 @@ config HAS_CHECKPOLICY > string > option env="XEN_HAS_CHECKPOLICY" > > +menu "Speculative hardening" > + > +config SPECULATIVE_ARRAY_HARDEN Seeing also the new item in patch 2 - wouldn't it be better for them all to have (just) a common prefix, rather than common prefix and a common suffix. E.g. SPECULATIVE_HARDEN_ARRAYS here and SPECULATIVE_HARDEN_BRANCHES there? > --- a/xen/include/xen/nospec.h > +++ b/xen/include/xen/nospec.h > @@ -33,6 +33,7 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, > } > #endif > > +#ifdef CONFIG_SPECULATIVE_ARRAY_HARDEN > /* > * array_index_nospec - sanitize an array index after a bounds check > * > @@ -58,6 +59,17 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, > \ > (typeof(_i)) (_i & _mask); \ > }) > +#else > +/* No index hardening. */ > +#define array_index_nospec(index, size) \ > +({ \ > + typeof(index) _i = (index); \ > + typeof(size) _s = (size); \ > + \ > + (void)_s; \ > + _i; \ > +}) Why not the simpler #define array_index_nospec(index, size) \ ({ \ (void)(size); \ (index); \ }) at which point it would seem feasible to avoid the use of compiler extensions altogether by making it #define array_index_nospec(index, size) ((void)(size), (index)) ? Jan
On 01/10/2019 11:45, Jan Beulich wrote: > On 30.09.2019 20:24, Andrew Cooper wrote: >> --- a/xen/common/Kconfig >> +++ b/xen/common/Kconfig >> @@ -77,6 +77,27 @@ config HAS_CHECKPOLICY >> string >> option env="XEN_HAS_CHECKPOLICY" >> >> +menu "Speculative hardening" >> + >> +config SPECULATIVE_ARRAY_HARDEN > Seeing also the new item in patch 2 - wouldn't it be better for them all > to have (just) a common prefix, rather than common prefix and a common > suffix. E.g. SPECULATIVE_HARDEN_ARRAYS here and SPECULATIVE_HARDEN_BRANCHES > there? Can do. > >> --- a/xen/include/xen/nospec.h >> +++ b/xen/include/xen/nospec.h >> @@ -33,6 +33,7 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, >> } >> #endif >> >> +#ifdef CONFIG_SPECULATIVE_ARRAY_HARDEN >> /* >> * array_index_nospec - sanitize an array index after a bounds check >> * >> @@ -58,6 +59,17 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, >> \ >> (typeof(_i)) (_i & _mask); \ >> }) >> +#else >> +/* No index hardening. */ >> +#define array_index_nospec(index, size) \ >> +({ \ >> + typeof(index) _i = (index); \ >> + typeof(size) _s = (size); \ >> + \ >> + (void)_s; \ >> + _i; \ >> +}) > Why not the simpler > > #define array_index_nospec(index, size) \ > ({ \ > (void)(size); \ > (index); \ > }) > > at which point it would seem feasible to avoid the use of compiler > extensions altogether by making it > > #define array_index_nospec(index, size) ((void)(size), (index)) Huh - I tried that first, and GCC was distinctly unhappy. It turns out to be the bracketing of size, which when omitted, causes: /local/xen.git/xen/include/xen/nospec.h:66:42: error: void value not ignored as it ought to be #define array_index_nospec(index, size) ((void)size, (index)) argo.c:2174:16: note: in expansion of macro ‘array_index_nospec’ niov = array_index_nospec(arg3, XEN_ARGO_MAXIOV + 1); ^~~~~~~~~~~~~~~~~~ I'll switch to this version. ~Andrew
diff --git a/xen/common/Kconfig b/xen/common/Kconfig index 16829f6274..9644cc9911 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -77,6 +77,27 @@ config HAS_CHECKPOLICY string option env="XEN_HAS_CHECKPOLICY" +menu "Speculative hardening" + +config SPECULATIVE_ARRAY_HARDEN + bool "Speculative Array Hardening" + default y + ---help--- + Contemporary processors may use speculative execution as a + performance optimisation, but this can potentially be abused by an + attacker to leak data via speculative sidechannels. + + One source of data leakage is via speculative out-of-bounds array + accesses. + + When enabled, specific array accesses which have been deemed liable + to be speculatively abused will be hardened to avoid out-of-bounds + accesses. + + If unsure, say Y. + +endmenu + config KEXEC bool "kexec support" default y diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h index 2ac8feccc2..e627a4da52 100644 --- a/xen/include/xen/nospec.h +++ b/xen/include/xen/nospec.h @@ -33,6 +33,7 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, } #endif +#ifdef CONFIG_SPECULATIVE_ARRAY_HARDEN /* * array_index_nospec - sanitize an array index after a bounds check * @@ -58,6 +59,17 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, \ (typeof(_i)) (_i & _mask); \ }) +#else +/* No index hardening. */ +#define array_index_nospec(index, size) \ +({ \ + typeof(index) _i = (index); \ + typeof(size) _s = (size); \ + \ + (void)_s; \ + _i; \ +}) +#endif /* CONFIG_SPECULATIVE_ARRAY_HARDEN */ /* * array_access_nospec - allow nospec access for static size arrays
There are legitimate circumstance where array hardening is not wanted or needed. Allow it to be turned off. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wl@xen.org> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Juergen Gross <jgross@suse.com> --- xen/common/Kconfig | 21 +++++++++++++++++++++ xen/include/xen/nospec.h | 12 ++++++++++++ 2 files changed, 33 insertions(+)