* SBB V10 Bundle
@ 2018-05-01 22:53 Thomas Gleixner
2018-05-02 11:04 ` [MODERATED] " Ingo Molnar
2018-05-02 12:07 ` Ingo Molnar
0 siblings, 2 replies; 9+ messages in thread
From: Thomas Gleixner @ 2018-05-01 22:53 UTC (permalink / raw)
To: speck
[-- Attachment #1: Type: text/plain, Size: 2794 bytes --]
Hi!
Find attached the V10 bundle.
Changes from V9:
- Fix the AMD CPU hotplug fallout
- Add non vulnerable AMD models to the whitelist (Borislav)
I've update all three branches in the repository as well.
Delta patch to V9 below.
Thanks,
tglx
8<------------------
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 6cd982709daf..50c6ba6d031b 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -555,6 +555,26 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
rdmsrl(MSR_FAM10H_NODE_ID, value);
nodes_per_socket = ((value >> 3) & 7) + 1;
}
+
+ if (c->x86 >= 0x15 && c->x86 <= 0x17) {
+ unsigned int bit;
+
+ switch (c->x86) {
+ case 0x15: bit = 54; break;
+ case 0x16: bit = 33; break;
+ case 0x17: bit = 10; break;
+ default: return;
+ }
+ /*
+ * Try to cache the base value so further operations can
+ * avoid RMW. If that faults, do not enable RDS.
+ */
+ if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &x86_amd_ls_cfg_base)) {
+ setup_force_cpu_cap(X86_FEATURE_RDS);
+ setup_force_cpu_cap(X86_FEATURE_AMD_RDS);
+ x86_amd_ls_cfg_rds_mask = (1ULL << bit);
+ }
+ }
}
static void early_detect_mem_encrypt(struct cpuinfo_x86 *c)
@@ -674,26 +694,6 @@ static void early_init_amd(struct cpuinfo_x86 *c)
set_cpu_bug(c, X86_BUG_AMD_E400);
early_detect_mem_encrypt(c);
-
- if (c->x86 >= 0x15 && c->x86 <= 0x17) {
- unsigned int bit;
-
- switch (c->x86) {
- case 0x15: bit = 54; break;
- case 0x16: bit = 33; break;
- case 0x17: bit = 10; break;
- default: return;
- }
- /*
- * Try to cache the base value so further operations can
- * avoid RMW. If that faults, do not enable RDS.
- */
- if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &x86_amd_ls_cfg_base)) {
- set_cpu_cap(c, X86_FEATURE_RDS);
- set_cpu_cap(c, X86_FEATURE_AMD_RDS);
- x86_amd_ls_cfg_rds_mask = (1ULL << bit);
- }
- }
}
static void init_amd_k8(struct cpuinfo_x86 *c)
@@ -919,6 +919,11 @@ static void init_amd(struct cpuinfo_x86 *c)
/* AMD CPUs don't reset SS attributes on SYSRET, Xen does. */
if (!cpu_has(c, X86_FEATURE_XENPV))
set_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
+
+ if (boot_cpu_has(X86_FEATURE_AMD_RDS)) {
+ set_cpu_cap(c, X86_FEATURE_RDS);
+ set_cpu_cap(c, X86_FEATURE_AMD_RDS);
+ }
}
#ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 6101e3dba2f6..f3dbdde978a4 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -939,6 +939,10 @@ static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
{ X86_VENDOR_CENTAUR, 5 },
{ X86_VENDOR_INTEL, 5 },
{ X86_VENDOR_NSC, 5 },
+ { X86_VENDOR_AMD, 0xf },
+ { X86_VENDOR_AMD, 0x10 },
+ { X86_VENDOR_AMD, 0x11 },
+ { X86_VENDOR_AMD, 0x12 },
{ X86_VENDOR_ANY, 4 },
{}
};
[-- Attachment #2: Type: application/octet-stream, Size: 27800 bytes --]
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [MODERATED] Re: SBB V10 Bundle
2018-05-01 22:53 SBB V10 Bundle Thomas Gleixner
@ 2018-05-02 11:04 ` Ingo Molnar
2018-05-02 12:07 ` Ingo Molnar
1 sibling, 0 replies; 9+ messages in thread
From: Ingo Molnar @ 2018-05-02 11:04 UTC (permalink / raw)
To: speck
A quick review of the v10 series:
==============>
b6f76374d45a: x86/nospec: Simplify alternative_msr_write()
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
3ec366cfa844: x86/bugs: Concentrate bug detection into a separate function
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
3b06f42b82ad: x86/bugs: Concentrate bug reporting into a separate function
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
edb984611076: x86/bugs: Read SPEC_CTRL MSR during boot and re-use reserved bits
This:
if (val & ~(SPEC_CTRL_IBRS))
... can lose the parantheses I suspect?
Also:
s/Our boot-time value of SPEC_CTRL MSR.
/Our boot-time value of the SPEC_CTRL MSR.
With those fixed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Also, a bike-shed painting suggestion, could we flip over the spec_ctrl namespace
to be top-down hiearchical, i.e. continue the nomenclature started by:
x86_spec_ctrl_base
x86_spec_ctrl_mask
... and make the API functions use that namespace as well:
x86_get_spec_ctrl() => x86_spec_ctrl_get()
x86_set_spec_ctrl() => x86_spec_ctrl_set()
etc.?
In a similar vein:
PR_GET_SPECULATION_CTRL => PR_SPECULATION_CTRL_GET
PR_SET_SPECULATION_CTRL => PR_SPECULATION_CTRL_SET
(but no strong feelings and not a condition of an Ack.)
==============>
33078b163c08: KVM/SVM/VMX/x86/bugs: Support the combination of guest and host IBRS
s/Note: This uses wrmsrl instead of native_wrmsl.
/Note: This uses wrmsrl() instead of native_wrmsl().
s/into the functions
/to the functions
With those fixed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
and if we do the renames in the previous patch then we should probably also do:
x86_set_guest_spec_ctrl() => x86_spec_ctrl_guest_set()
x86_restore_host_spec_ctrl() => x86_spec_ctrl_host_restore()
==============>
db77b0d7700e: x86/bugs: Expose the /sys/../spec_store_bypass and X86_BUG_SPEC_STORE_BYPASS
Title should I suspect be extended to:
x86/bugs: Expose the /sys/../spec_store_bypass and X86_BUG_SPEC_STORE_BYPASS values
?
Also, could we change this:
+static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT1 },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT2 },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_MERRIFIELD },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_CORE_YONAH },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNL },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNM },
... to something like:
+static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT1 },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT2 },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_MERRIFIELD },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_CORE_YONAH },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNL },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNM },
... to make misplaced commas and other asymmetries easier to see during review?
With those fixed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
b6e792e6f30d: x86/cpufeatures: Add X86_FEATURE_RDS
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
33c97592f020: x86/bugs: Provide boot parameters for the spec_store_bypass_disable mitigation
s/Provide two command line control knobs:
/As a first step to mitigate against such attacks, provide two boot command line control knobs:
s/a implementation
/an implementation
I think the boot flags are confusing. We have:
nospec_store_bypass_disable == Disable all mitigations for the Speculative Store
Bypass vulerability, i.e. keep "speculative store
bypass" enabled.
Note the 'no ... bypass disable' double negation which results in the hardware
feature of remaining enabled.
We also have:
spec_store_bypass_disable=off
the 'disable the disabling' results in 'enabling' the hardware feature.
I realize that this is correctly specified and explained via:
"By default affected x86 processors will power on with Speculative
Store Bypass enabled. Hence the provided kernel parameters are written
from the point of view of whether to enable a mitigation or not."
... but that does not make the parameters any less confusing.
Why not control the hardware feature directly, i.e.:
spec_store_bypass # equivalent to 'on'
spec_store_bypass=off
spec_store_bypass=on
spec_store_bypass=auto
?
This would also remove the standalone and dissonant nospec_store_bypass_disable
boot parameter.
Note how this is already implemented internally AFAICS:
+enum ssb_mitigation_cmd {
+ SPEC_STORE_BYPASS_CMD_NONE,
+ SPEC_STORE_BYPASS_CMD_AUTO,
+ SPEC_STORE_BYPASS_CMD_ON,
+};
+
+static const char *ssb_strings[] = {
+ [SPEC_STORE_BYPASS_NONE] = "Vulnerable",
+ [SPEC_STORE_BYPASS_DISABLE] = "Mitigation: Speculative Store Bypass disabled"
+
this already has everything simplified down to the hardware feature level.
Am I confused?
Also:
+ char arg[20];
the largest string that has to fit is "auto", so this could be 5 or just a
self-documenting, open-coded sizeof("auto"+1), right?
==============>
0e9930e8fa7d: x86/bugs/Intel: Set proper CPU features and setup RDS
s/Intel
/intel
because we typically write it lowercase in title prefixes.
s/setup
/set up
Also, could we please write out "Reduced Data Speculation (RDS)" in the title,
which is the first time people will see this acronym?
Within the changelog we can then write 'RDS' instead of writing it out every time.
While attention span is short, it does usually last from title to end of
changelog. This is doubly true since we use the RDS acronym in actual source
code...
This paragraph in the changelog:
+ Note that this does not fix the KVM case where the SPEC_CTRL is exposed to
+ guests which can muck with, see patch titled : KVM/SVM/VMX/x86/spectre_v2:
+ Support the combination of guest and host IBRS.
Took me three attempts to parse correctly. Could we please clarify it to:
Note that this does not fix the KVM case where the SPEC_CTRL is exposed to
guests which can muck with it, see the following patch:
KVM/SVM/VMX/x86/spectre_v2: Support the combination of guest and host IBRS
or so? (Also note the extra 'it' inserted for good measure.)
This bit in the code:
+#define ARCH_CAP_RDS_NO (1 << 4) /* Not susceptible to speculative store bypass */
... is hanging in the air a bit, the specifier has 'RDS' but the comment is
talking about 'speculative store bypass' (which should be capitalized in any
case).
How about:
/*
* Not susceptible to the Speculative Store Bypass attack, so
* no Reduced Data Speculative control required:
*/
#define ARCH_CAP_RDS_NO (1 << 4)
or so?
Finally, if we do the function namespace renames for the previous patches, we
should probably also do:
x86_setup_ap_spec_ctrl() => x86_spec_ctrl_setup_ap()
With these fixed/changed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
c9c863a0e592: x86/bugs: Whitelist allowed SPEC_CTRL MSR values
+static u64 __ro_after_init x86_spec_ctrl_mask = ~(SPEC_CTRL_IBRS);
No parantheses needed I suspect.
+ x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS);
ditto.
With those fixed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
c20ee3e76f12: x86/bugs/AMD: Add support to disable RDS on Fam[15,16,17]h if requested
s/x86/bugs/AMD: Add support to disable RDS on Fam[15,16,17]h if requested
/x86/bugs/AMD: Add support to disable RDS on Fam[15,16,17]h CPUs if requested
Also, I suspect 'Reduced Data Speculation' should be written out once in the
changelog.
Question: does the x86_amd_ls_cfg_base only include RDS bits? If yes then I
suspect we should rename:
x86_amd_ls_cfg_base => x86_amd_ls_cfg_rds_base
?
If it's used for other bits then never mind.
+ x86_amd_ls_cfg_rds_mask = (1ULL << bit);
No parentheses needed I think.
+ * AMD specific MSR info for Store Bypass control.
s/for Speculative Store Bypass control.
/for Store Bypass control.
s/AMD64_LS_CFG msr
The AMD64_LS_CFG MSR
With these resolved:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
16544ca81942: x86/KVM/VMX: Expose SPEC_CTRL Bit(2) to the guest
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
83b2439da7e8: x86/speculation: Create spec-ctrl.h to avoid include hell
s/and fixup the includes
/and fix up the includes
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
b4fe2032d782: prctl: Add speculation control prctls
+/* Per task speculation control */
+#define PR_SET_SPECULATION_CTRL 52
+#define PR_GET_SPECULATION_CTRL 53
Please flip this around, i.e.:
/* Per task speculation control */
#define PR_GET_SPECULATION_CTRL 52
#define PR_SET_SPECULATION_CTRL 53
because GET/SET is the canonical ABI order, used in most early prctl()s as well,
until someone messed it up in PR_SET_TIMERSLACK, which bad example then everyone
after that cargo-cult-copied ... Let's go back to the original ABI sanity.
+ case PR_SET_SPECULATION_CTRL:
+ if (arg4 || arg5)
+ return -EINVAL;
So I think returning -ENOSYS might be better, to allow future extensions to use
-EINVAL for a real invalid value - and allow -ENOSYS to be the "older kernel"
disambiguation. For tools that care.
I'd also add this to the changelog and to Documentation/userspace-api/spec_ctrl.rst:
arg4 and arg5 must be zero, for future ABI compatibility. This condition is
enforced by the system call.
Misc nits/typos:
s/The return value uses bit 0-2
/The return value uses bits 0-2
s/There is also a class of mitigations which is very expensive,
There is also a class of mitigations which are very expensive,
Plus the potential namespace hiearchy changes. With all of the resolved:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
f746bf3179f2: x86/process: Allow runtime control of Speculative Store Bypass
s/Reduced Data Speculation control (RDS)
/Reduced Data Speculation (RDS) feature
?
s/Reduced data speculation
/Reduced Data Speculation
Also, could we write this:
+ return tifn & _TIF_RDS ? x86_amd_ls_cfg_rds_mask : 0ULL;
as:
+ return (tifn & _TIF_RDS) ? x86_amd_ls_cfg_rds_mask : 0ULL;
because while the operator priorities are unambiguous, I had to look twice.
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
d41eec59d419: x86/speculation: Add prctl for Speculative Store Bypass mitigation
This paragraph of the changelog does not parse for me:
On the other hand if there is the ability to switch it freely gives more
flexibility to do the protection which is needed for JITed code without
impacting the overall system performance.
Could we please split this into two sentences or at least add a comma or two or
otherwise create something more readable - and also make the 'it' more
unambiguous?
Also, a naive reading of the boot options:
auto - Kernel detects whether the CPU model contains a
implementation of Speculative Store Bypass and
picks the most appropriate mitigation
+ prctl - Control Speculative Store Bypass for a thread
+ via prctl. By default it is enabled. The state
+ is inherited on fork.
does not resolve the question of whether 'on', 'on', 'auto' or 'prctl' is the best
option to use. I.e. the interaction of on/off/auto/prctl isn't clear IMHO.
Another question is:
+static int ssb_prctl_set(unsigned long ctrl)
+{
+ bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
+
+ if (ssb_mode != SPEC_STORE_BYPASS_PRCTL)
+ return -ENXIO;
This might be overly permissive in the 'auto' case which defaulted to 'on', where
an application might still want to disable it for good reasons?
I.e. we should only deny a prctl specifying a more restrictive speculation model
when the user has forced all speculation on, i.e. "=on".
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [MODERATED] Re: SBB V10 Bundle
2018-05-01 22:53 SBB V10 Bundle Thomas Gleixner
2018-05-02 11:04 ` [MODERATED] " Ingo Molnar
@ 2018-05-02 12:07 ` Ingo Molnar
2018-05-02 13:23 ` Thomas Gleixner
1 sibling, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2018-05-02 12:07 UTC (permalink / raw)
To: speck
A quick review of the v10 series:
==============>
b6f76374d45a: x86/nospec: Simplify alternative_msr_write()
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
3ec366cfa844: x86/bugs: Concentrate bug detection into a separate function
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
3b06f42b82ad: x86/bugs: Concentrate bug reporting into a separate function
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
edb984611076: x86/bugs: Read SPEC_CTRL MSR during boot and re-use reserved bits
This:
if (val & ~(SPEC_CTRL_IBRS))
... can lose the parantheses I suspect?
Also:
s/Our boot-time value of SPEC_CTRL MSR.
/Our boot-time value of the SPEC_CTRL MSR.
With those fixed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Also, a bike-shed painting suggestion, could we flip over the spec_ctrl namespace
to be top-down hiearchical, i.e. continue the nomenclature started by:
x86_spec_ctrl_base
x86_spec_ctrl_mask
... and make the API functions use that namespace as well:
x86_get_spec_ctrl() => x86_spec_ctrl_get()
x86_set_spec_ctrl() => x86_spec_ctrl_set()
etc.?
In a similar vein:
PR_GET_SPECULATION_CTRL => PR_SPECULATION_CTRL_GET
PR_SET_SPECULATION_CTRL => PR_SPECULATION_CTRL_SET
(but no strong feelings and not a condition of an Ack.)
==============>
33078b163c08: KVM/SVM/VMX/x86/bugs: Support the combination of guest and host IBRS
s/Note: This uses wrmsrl instead of native_wrmsl.
/Note: This uses wrmsrl() instead of native_wrmsl().
s/into the functions
/to the functions
With those fixed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
and if we do the renames in the previous patch then we should probably also do:
x86_set_guest_spec_ctrl() => x86_spec_ctrl_guest_set()
x86_restore_host_spec_ctrl() => x86_spec_ctrl_host_restore()
==============>
db77b0d7700e: x86/bugs: Expose the /sys/../spec_store_bypass and X86_BUG_SPEC_STORE_BYPASS
Title should I suspect be extended to:
x86/bugs: Expose the /sys/../spec_store_bypass and X86_BUG_SPEC_STORE_BYPASS values
?
Also, could we change this:
+static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT1 },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT2 },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_MERRIFIELD },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_CORE_YONAH },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNL },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNM },
... to something like:
+static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT1 },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_SILVERMONT2 },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_MERRIFIELD },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_CORE_YONAH },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNL },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_XEON_PHI_KNM },
... to make misplaced commas and other asymmetries easier to see during review?
With those fixed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
b6e792e6f30d: x86/cpufeatures: Add X86_FEATURE_RDS
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
33c97592f020: x86/bugs: Provide boot parameters for the spec_store_bypass_disable mitigation
s/Provide two command line control knobs:
/As a first step to mitigate against such attacks, provide two boot command line control knobs:
s/a implementation
/an implementation
I think the boot flags are confusing. We have:
nospec_store_bypass_disable == Disable all mitigations for the Speculative Store
Bypass vulerability, i.e. keep "speculative store
bypass" enabled.
Note the 'no ... bypass disable' double negation which results in the hardware
feature of remaining enabled.
We also have:
spec_store_bypass_disable=off
the 'disable the disabling' results in 'enabling' the hardware feature.
I realize that this is correctly specified and explained via:
"By default affected x86 processors will power on with Speculative
Store Bypass enabled. Hence the provided kernel parameters are written
from the point of view of whether to enable a mitigation or not."
... but that does not make the parameters any less confusing.
Why not control the hardware feature directly, i.e.:
spec_store_bypass # equivalent to 'on'
spec_store_bypass=off
spec_store_bypass=on
spec_store_bypass=auto
?
This would also remove the standalone and dissonant nospec_store_bypass_disable
boot parameter.
Note how this is already implemented internally AFAICS:
+enum ssb_mitigation_cmd {
+ SPEC_STORE_BYPASS_CMD_NONE,
+ SPEC_STORE_BYPASS_CMD_AUTO,
+ SPEC_STORE_BYPASS_CMD_ON,
+};
+
+static const char *ssb_strings[] = {
+ [SPEC_STORE_BYPASS_NONE] = "Vulnerable",
+ [SPEC_STORE_BYPASS_DISABLE] = "Mitigation: Speculative Store Bypass disabled"
+
this already has everything simplified down to the hardware feature level.
Am I confused?
Also:
+ char arg[20];
the largest string that has to fit is "auto", so this could be 5 or just a
self-documenting, open-coded sizeof("auto"+1), right?
==============>
0e9930e8fa7d: x86/bugs/Intel: Set proper CPU features and setup RDS
s/Intel
/intel
because we typically write it lowercase in title prefixes.
s/setup
/set up
Also, could we please write out "Reduced Data Speculation (RDS)" in the title,
which is the first time people will see this acronym?
Within the changelog we can then write 'RDS' instead of writing it out every time.
While attention span is short, it does usually last from title to end of
changelog. This is doubly true since we use the RDS acronym in actual source
code...
This paragraph in the changelog:
+ Note that this does not fix the KVM case where the SPEC_CTRL is exposed to
+ guests which can muck with, see patch titled : KVM/SVM/VMX/x86/spectre_v2:
+ Support the combination of guest and host IBRS.
Took me three attempts to parse correctly. Could we please clarify it to:
Note that this does not fix the KVM case where the SPEC_CTRL is exposed to
guests which can muck with it, see the following patch:
KVM/SVM/VMX/x86/spectre_v2: Support the combination of guest and host IBRS
or so? (Also note the extra 'it' inserted for good measure.)
This bit in the code:
+#define ARCH_CAP_RDS_NO (1 << 4) /* Not susceptible to speculative store bypass */
... is hanging in the air a bit, the specifier has 'RDS' but the comment is
talking about 'speculative store bypass' (which should be capitalized in any
case).
How about:
/*
* Not susceptible to the Speculative Store Bypass attack, so
* no Reduced Data Speculative control required:
*/
#define ARCH_CAP_RDS_NO (1 << 4)
or so?
Finally, if we do the function namespace renames for the previous patches, we
should probably also do:
x86_setup_ap_spec_ctrl() => x86_spec_ctrl_setup_ap()
With these fixed/changed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
c9c863a0e592: x86/bugs: Whitelist allowed SPEC_CTRL MSR values
+static u64 __ro_after_init x86_spec_ctrl_mask = ~(SPEC_CTRL_IBRS);
No parantheses needed I suspect.
+ x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS);
ditto.
With those fixed:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
c20ee3e76f12: x86/bugs/AMD: Add support to disable RDS on Fam[15,16,17]h if requested
s/x86/bugs/AMD: Add support to disable RDS on Fam[15,16,17]h if requested
/x86/bugs/AMD: Add support to disable RDS on Fam[15,16,17]h CPUs if requested
Also, I suspect 'Reduced Data Speculation' should be written out once in the
changelog.
Question: does the x86_amd_ls_cfg_base only include RDS bits? If yes then I
suspect we should rename:
x86_amd_ls_cfg_base => x86_amd_ls_cfg_rds_base
?
If it's used for other bits then never mind.
+ x86_amd_ls_cfg_rds_mask = (1ULL << bit);
No parentheses needed I think.
+ * AMD specific MSR info for Store Bypass control.
s/for Speculative Store Bypass control.
/for Store Bypass control.
s/AMD64_LS_CFG msr
The AMD64_LS_CFG MSR
With these resolved:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
16544ca81942: x86/KVM/VMX: Expose SPEC_CTRL Bit(2) to the guest
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
83b2439da7e8: x86/speculation: Create spec-ctrl.h to avoid include hell
s/and fixup the includes
/and fix up the includes
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
b4fe2032d782: prctl: Add speculation control prctls
+/* Per task speculation control */
+#define PR_SET_SPECULATION_CTRL 52
+#define PR_GET_SPECULATION_CTRL 53
Please flip this around, i.e.:
/* Per task speculation control */
#define PR_GET_SPECULATION_CTRL 52
#define PR_SET_SPECULATION_CTRL 53
because GET/SET is the canonical ABI order, used in most early prctl()s as well,
until someone messed it up in PR_SET_TIMERSLACK, which bad example then everyone
after that cargo-cult-copied ... Let's go back to the original ABI sanity.
+ case PR_SET_SPECULATION_CTRL:
+ if (arg4 || arg5)
+ return -EINVAL;
So I think returning -ENOSYS might be better, to allow future extensions to use
-EINVAL for a real invalid value - and allow -ENOSYS to be the "older kernel"
disambiguation. For tools that care.
I'd also add this to the changelog and to Documentation/userspace-api/spec_ctrl.rst:
arg4 and arg5 must be zero, for future ABI compatibility. This condition is
enforced by the system call.
Misc nits/typos:
s/The return value uses bit 0-2
/The return value uses bits 0-2
s/There is also a class of mitigations which is very expensive,
There is also a class of mitigations which are very expensive,
Plus the potential namespace hiearchy changes. With all of the resolved:
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
f746bf3179f2: x86/process: Allow runtime control of Speculative Store Bypass
s/Reduced Data Speculation control (RDS)
/Reduced Data Speculation (RDS) feature
?
s/Reduced data speculation
/Reduced Data Speculation
Also, could we write this:
+ return tifn & _TIF_RDS ? x86_amd_ls_cfg_rds_mask : 0ULL;
as:
+ return (tifn & _TIF_RDS) ? x86_amd_ls_cfg_rds_mask : 0ULL;
because while the operator priorities are unambiguous, I had to look twice.
Reviewed-by: Ingo Molnar <mingo@kernel.org>
==============>
d41eec59d419: x86/speculation: Add prctl for Speculative Store Bypass mitigation
This paragraph of the changelog does not parse for me:
On the other hand if there is the ability to switch it freely gives more
flexibility to do the protection which is needed for JITed code without
impacting the overall system performance.
Could we please split this into two sentences or at least add a comma or two or
otherwise create something more readable - and also make the 'it' more
unambiguous?
Also, a naive reading of the boot options:
auto - Kernel detects whether the CPU model contains a
implementation of Speculative Store Bypass and
picks the most appropriate mitigation
+ prctl - Control Speculative Store Bypass for a thread
+ via prctl. By default it is enabled. The state
+ is inherited on fork.
does not resolve the question of whether 'on', 'on', 'auto' or 'prctl' is the best
option to use. I.e. the interaction of on/off/auto/prctl isn't clear IMHO.
Another question is:
+static int ssb_prctl_set(unsigned long ctrl)
+{
+ bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
+
+ if (ssb_mode != SPEC_STORE_BYPASS_PRCTL)
+ return -ENXIO;
This might be overly permissive in the 'auto' case which defaulted to 'on', where
an application might still want to disable it for good reasons?
I.e. we should only deny a prctl specifying a more restrictive speculation model
when the user has forced all speculation on, i.e. "=on".
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SBB V10 Bundle
2018-05-02 12:07 ` Ingo Molnar
@ 2018-05-02 13:23 ` Thomas Gleixner
2018-05-03 5:58 ` [MODERATED] " Ingo Molnar
2018-05-03 6:07 ` Ingo Molnar
0 siblings, 2 replies; 9+ messages in thread
From: Thomas Gleixner @ 2018-05-02 13:23 UTC (permalink / raw)
To: speck
On Wed, 2 May 2018, speck for Ingo Molnar wrote:
> Also, a bike-shed painting suggestion, could we flip over the spec_ctrl namespace
> to be top-down hiearchical, i.e. continue the nomenclature started by:
>
> x86_spec_ctrl_base
> x86_spec_ctrl_mask
>
> ... and make the API functions use that namespace as well:
>
> x86_get_spec_ctrl() => x86_spec_ctrl_get()
> x86_set_spec_ctrl() => x86_spec_ctrl_set()
>
> etc.?
That's doable
> In a similar vein:
>
> PR_GET_SPECULATION_CTRL => PR_SPECULATION_CTRL_GET
> PR_SET_SPECULATION_CTRL => PR_SPECULATION_CTRL_SET
The PRCTL stuff has this PR_GET_XXX PR_SET_XXX all over the place, so I
rather keep it that way.
> 33c97592f020: x86/bugs: Provide boot parameters for the spec_store_bypass_disable mitigation
>
> s/Provide two command line control knobs:
> /As a first step to mitigate against such attacks, provide two boot command line control knobs:
>
> s/a implementation
> /an implementation
>
> I think the boot flags are confusing. We have:
>
> nospec_store_bypass_disable == Disable all mitigations for the Speculative Store
> Bypass vulerability, i.e. keep "speculative store
> bypass" enabled.
>
> Note the 'no ... bypass disable' double negation which results in the hardware
> feature of remaining enabled.
>
> We also have:
>
> spec_store_bypass_disable=off
>
> the 'disable the disabling' results in 'enabling' the hardware feature.
>
> I realize that this is correctly specified and explained via:
>
> "By default affected x86 processors will power on with Speculative
> Store Bypass enabled. Hence the provided kernel parameters are written
> from the point of view of whether to enable a mitigation or not."
>
> ... but that does not make the parameters any less confusing.
>
> Why not control the hardware feature directly, i.e.:
>
> spec_store_bypass # equivalent to 'on'
> spec_store_bypass=off
> spec_store_bypass=on
> spec_store_bypass=auto
You're late to that party. This has been discussed before and I shot it
down for the following reason:
For all other vulnerabilities we control the mitigation on the kernel
command line and we should stay consistent with that and not start to turn
it around into a performance feature control knob for this one.
> This would also remove the standalone and dissonant nospec_store_bypass_disable
> boot parameter.
That is redundant anyway, so it could just go away.
> ==============>
> c20ee3e76f12: x86/bugs/AMD: Add support to disable RDS on Fam[15,16,17]h if requested
>
> s/x86/bugs/AMD: Add support to disable RDS on Fam[15,16,17]h if requested
> /x86/bugs/AMD: Add support to disable RDS on Fam[15,16,17]h CPUs if requested
>
> Also, I suspect 'Reduced Data Speculation' should be written out once in the
> changelog.
>
> Question: does the x86_amd_ls_cfg_base only include RDS bits? If yes then I
> suspect we should rename:
>
> x86_amd_ls_cfg_base => x86_amd_ls_cfg_rds_base
No, it's a magic configuration MSR with other control bits.
> ==============>
> b4fe2032d782: prctl: Add speculation control prctls
>
> +/* Per task speculation control */
> +#define PR_SET_SPECULATION_CTRL 52
> +#define PR_GET_SPECULATION_CTRL 53
>
> Please flip this around, i.e.:
>
> /* Per task speculation control */
> #define PR_GET_SPECULATION_CTRL 52
> #define PR_SET_SPECULATION_CTRL 53
>
> because GET/SET is the canonical ABI order, used in most early prctl()s as well,
> until someone messed it up in PR_SET_TIMERSLACK, which bad example then everyone
> after that cargo-cult-copied ... Let's go back to the original ABI sanity.
> + case PR_SET_SPECULATION_CTRL:
> + if (arg4 || arg5)
> + return -EINVAL;
>
> So I think returning -ENOSYS might be better, to allow future extensions to use
> -EINVAL for a real invalid value - and allow -ENOSYS to be the "older kernel"
> disambiguation. For tools that care.
Take that up with Linus. See his reply on that :)
> Also, a naive reading of the boot options:
>
> auto - Kernel detects whether the CPU model contains a
> implementation of Speculative Store Bypass and
> picks the most appropriate mitigation
> + prctl - Control Speculative Store Bypass for a thread
> + via prctl. By default it is enabled. The state
> + is inherited on fork.
>
> does not resolve the question of whether 'on', 'on', 'auto' or 'prctl' is the best
> option to use. I.e. the interaction of on/off/auto/prctl isn't clear IMHO.
Can you come up with something better?
> Another question is:
>
> +static int ssb_prctl_set(unsigned long ctrl)
> +{
> + bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
> +
> + if (ssb_mode != SPEC_STORE_BYPASS_PRCTL)
> + return -ENXIO;
>
> This might be overly permissive in the 'auto' case which defaulted to 'on', where
> an application might still want to disable it for good reasons?
Permissive? Its restrictive.
Also the default is 'prctl' now and I don't think that you want applications
let override the admin decision, which might be 'on' == global mitigation or
'off' == 'no mitigation at all'.
So when the mitigation mode is 'prctl' which is the default then
applications can fiddle with it. If the admin decided global on or off then
it's rightfully rejected.
Hmm?
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [MODERATED] Re: SBB V10 Bundle
2018-05-02 13:23 ` Thomas Gleixner
@ 2018-05-03 5:58 ` Ingo Molnar
2018-05-03 6:07 ` Ingo Molnar
1 sibling, 0 replies; 9+ messages in thread
From: Ingo Molnar @ 2018-05-03 5:58 UTC (permalink / raw)
To: speck
* speck for Thomas Gleixner <speck@linutronix.de> wrote:
> > Another question is:
> >
> > +static int ssb_prctl_set(unsigned long ctrl)
> > +{
> > + bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
> > +
> > + if (ssb_mode != SPEC_STORE_BYPASS_PRCTL)
> > + return -ENXIO;
> >
> > This might be overly permissive in the 'auto' case which defaulted to 'on', where
> > an application might still want to disable it for good reasons?
>
> Permissive? Its restrictive.
I mean when 'auto' set it to 'off', i.e. turned off mitigation. (Darn all the
negatives ...)
> Also the default is 'prctl' now and I don't think that you want applications
> let override the admin decision, which might be 'on' == global mitigation or
> 'off' == 'no mitigation at all'.
Sure, explicit 'on' and 'off' is an admin choice - but 'auto' isn't really: it's
the admin saying "whatever you think is right".
> So when the mitigation mode is 'prctl' which is the default then
> applications can fiddle with it. If the admin decided global on or off then
> it's rightfully rejected.
>
> Hmm?
Correct, the only question is when 'auto' decides 'off' and app uses the prctl().
Do we even care, or do we think if the kernel thinks 'off' is correct then it
should be forced?
I'd be fine with "we don't care" if "prctl" is the expected distro default.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [MODERATED] Re: SBB V10 Bundle
2018-05-02 13:23 ` Thomas Gleixner
2018-05-03 5:58 ` [MODERATED] " Ingo Molnar
@ 2018-05-03 6:07 ` Ingo Molnar
2018-05-03 6:18 ` Thomas Gleixner
2018-05-03 6:27 ` Thomas Gleixner
1 sibling, 2 replies; 9+ messages in thread
From: Ingo Molnar @ 2018-05-03 6:07 UTC (permalink / raw)
To: speck
* speck for Thomas Gleixner <speck@linutronix.de> wrote:
> > ==============>
> > b4fe2032d782: prctl: Add speculation control prctls
> >
> > +/* Per task speculation control */
> > +#define PR_SET_SPECULATION_CTRL 52
> > +#define PR_GET_SPECULATION_CTRL 53
> >
> > Please flip this around, i.e.:
> >
> > /* Per task speculation control */
> > #define PR_GET_SPECULATION_CTRL 52
> > #define PR_SET_SPECULATION_CTRL 53
> >
> > because GET/SET is the canonical ABI order, used in most early prctl()s as well,
> > until someone messed it up in PR_SET_TIMERSLACK, which bad example then everyone
> > after that cargo-cult-copied ... Let's go back to the original ABI sanity.
JFYI, you didn't reply to this one - forgot to reply?
> > + case PR_SET_SPECULATION_CTRL:
> > + if (arg4 || arg5)
> > + return -EINVAL;
> >
> > So I think returning -ENOSYS might be better, to allow future extensions to use
> > -EINVAL for a real invalid value - and allow -ENOSYS to be the "older kernel"
> > disambiguation. For tools that care.
>
> Take that up with Linus. See his reply on that :)
Sorry, if that was on this list then his remarks are either not in my archive,
or Mutt doesn't properly search inside encrypted mails, or the list wasn't
Cc:-ed to that reply. Mind quoting that reply here?
Without having seen his reply:
- When introducing _new_ ABIs then using -ENOSYS is entirely canonical for "no
support exists that, yet".
- For _existing_ driver and syscall interfaces using -ENOSYS can be indeed be
confusing to apps and thus be the wrong thing to use.
> > Also, a naive reading of the boot options:
> >
> > auto - Kernel detects whether the CPU model contains a
> > implementation of Speculative Store Bypass and
> > picks the most appropriate mitigation
> > + prctl - Control Speculative Store Bypass for a thread
> > + via prctl. By default it is enabled. The state
> > + is inherited on fork.
> >
> > does not resolve the question of whether 'on', 'on', 'auto' or 'prctl' is the best
> > option to use. I.e. the interaction of on/off/auto/prctl isn't clear IMHO.
>
> Can you come up with something better?
Sure:
auto - Kernel detects whether the CPU model contains a
implementation of Speculative Store Bypass and
picks the most appropriate mitigation global
mitigation policy, which cannot be influenced
by apps even via prctl.
prctl - Control Speculative Store Bypass for a thread
via prctl. By default mitigation is enabled.
The state is inherited on fork.
Btw., is it true that in the 'prctl' case mitigation is always enabled? If it's
still CPU family/model sensitive like 'auto', which I think it is, then it might
be more accurate to write:
prctl - Control Speculative Store Bypass for a thread
via prctl. By default mitigation is enabled
on affected CPUs. The state is inherited on fork.
or so?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SBB V10 Bundle
2018-05-03 6:07 ` Ingo Molnar
@ 2018-05-03 6:18 ` Thomas Gleixner
2018-05-03 6:27 ` Thomas Gleixner
1 sibling, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2018-05-03 6:18 UTC (permalink / raw)
To: speck
On Thu, 3 May 2018, speck for Ingo Molnar wrote:
>
> * speck for Thomas Gleixner <speck@linutronix.de> wrote:
>
> > > ==============>
> > > b4fe2032d782: prctl: Add speculation control prctls
> > >
> > > +/* Per task speculation control */
> > > +#define PR_SET_SPECULATION_CTRL 52
> > > +#define PR_GET_SPECULATION_CTRL 53
> > >
> > > Please flip this around, i.e.:
> > >
> > > /* Per task speculation control */
> > > #define PR_GET_SPECULATION_CTRL 52
> > > #define PR_SET_SPECULATION_CTRL 53
> > >
> > > because GET/SET is the canonical ABI order, used in most early prctl()s as well,
> > > until someone messed it up in PR_SET_TIMERSLACK, which bad example then everyone
> > > after that cargo-cult-copied ... Let's go back to the original ABI sanity.
>
> JFYI, you didn't reply to this one - forgot to reply?
Why so, I didn't disagree and fixed it. Like other stuff :)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SBB V10 Bundle
2018-05-03 6:07 ` Ingo Molnar
2018-05-03 6:18 ` Thomas Gleixner
@ 2018-05-03 6:27 ` Thomas Gleixner
2018-05-03 6:31 ` Thomas Gleixner
1 sibling, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2018-05-03 6:27 UTC (permalink / raw)
To: speck
On Thu, 3 May 2018, speck for Ingo Molnar wrote:
> * speck for Thomas Gleixner <speck@linutronix.de> wrote:
> > > So I think returning -ENOSYS might be better, to allow future extensions to use
> > > -EINVAL for a real invalid value - and allow -ENOSYS to be the "older kernel"
> > > disambiguation. For tools that care.
> >
> > Take that up with Linus. See his reply on that :)
>
> Sorry, if that was on this list then his remarks are either not in my archive,
> or Mutt doesn't properly search inside encrypted mails, or the list wasn't
> Cc:-ed to that reply. Mind quoting that reply here?
>
> Without having seen his reply:
>
> - When introducing _new_ ABIs then using -ENOSYS is entirely canonical for "no
> support exists that, yet".
>
> - For _existing_ driver and syscall interfaces using -ENOSYS can be indeed be
> confusing to apps and thus be the wrong thing to use.
Here is the relevant snippet:
> On Mon, 30 Apr 2018, speck for Linus Torvalds wrote:
> > On Mon, 30 Apr 2018, speck for Kees Cook wrote:
> > >
> > > I think EINVAL is more expected in this case (and is what Michael Kerrisk
> > > has asked for in the past). In this specific case, users are able to test
> > > PR_GET_SPECULATION_CTRL to check for the prctl. Having arg3/4/5 forced
> > > to zero for that right now will ensure they're checking correctly from
> > > the start. :)
> >
> > Yeah. If you want to test whether the support is there or not, just use
> > the right arguments.
>
> Fair enough. Though I would have chosen -EMORON if that would have not been
> prevented by the political correctness crowd from being merged.
>
> tglx
I just went with EINVAL. I guess EMORON would have raised that discussion.
> > > Also, a naive reading of the boot options:
> > >
> > > auto - Kernel detects whether the CPU model contains a
> > > implementation of Speculative Store Bypass and
> > > picks the most appropriate mitigation
> > > + prctl - Control Speculative Store Bypass for a thread
> > > + via prctl. By default it is enabled. The state
> > > + is inherited on fork.
> > >
> > > does not resolve the question of whether 'on', 'on', 'auto' or 'prctl' is the best
> > > option to use. I.e. the interaction of on/off/auto/prctl isn't clear IMHO.
> >
> > Can you come up with something better?
>
> Sure:
>
> auto - Kernel detects whether the CPU model contains a
> implementation of Speculative Store Bypass and
> picks the most appropriate mitigation global
> mitigation policy, which cannot be influenced
> by apps even via prctl.
> prctl - Control Speculative Store Bypass for a thread
> via prctl. By default mitigation is enabled.
> The state is inherited on fork.
>
> Btw., is it true that in the 'prctl' case mitigation is always enabled? If it's
> still CPU family/model sensitive like 'auto', which I think it is, then it might
> be more accurate to write:
No. It's tthe other way round. Here is how I reworded it in V11
on - Unconditionally disable Speculative Store Bypass
off - Unconditionally enable Speculative Store Bypass
auto - Kernel detects whether the CPU model contains an
implementation of Speculative Store Bypass and
picks the most appropriate mitigation.
prctl - Control Speculative Store Bypass per thread
via prctl. Speculative Store Bypass is enabled
for a process by default. The state of the control
is inherited on fork.
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SBB V10 Bundle
2018-05-03 6:27 ` Thomas Gleixner
@ 2018-05-03 6:31 ` Thomas Gleixner
0 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2018-05-03 6:31 UTC (permalink / raw)
To: speck
On Thu, 3 May 2018, speck for Thomas Gleixner wrote:
> On Thu, 3 May 2018, speck for Ingo Molnar wrote:
> Here is the relevant snippet:
>
> > On Mon, 30 Apr 2018, speck for Linus Torvalds wrote:
> > > On Mon, 30 Apr 2018, speck for Kees Cook wrote:
> > > >
> > > > I think EINVAL is more expected in this case (and is what Michael Kerrisk
> > > > has asked for in the past). In this specific case, users are able to test
> > > > PR_GET_SPECULATION_CTRL to check for the prctl. Having arg3/4/5 forced
> > > > to zero for that right now will ensure they're checking correctly from
> > > > the start. :)
> > >
> > > Yeah. If you want to test whether the support is there or not, just use
> > > the right arguments.
> >
> > Fair enough. Though I would have chosen -EMORON if that would have not been
> > prevented by the political correctness crowd from being merged.
> >
> > tglx
>
> I just went with EINVAL. I guess EMORON would have raised that discussion.
wouldn't :)
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-05-03 6:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-01 22:53 SBB V10 Bundle Thomas Gleixner
2018-05-02 11:04 ` [MODERATED] " Ingo Molnar
2018-05-02 12:07 ` Ingo Molnar
2018-05-02 13:23 ` Thomas Gleixner
2018-05-03 5:58 ` [MODERATED] " Ingo Molnar
2018-05-03 6:07 ` Ingo Molnar
2018-05-03 6:18 ` Thomas Gleixner
2018-05-03 6:27 ` Thomas Gleixner
2018-05-03 6:31 ` Thomas Gleixner
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.