v3 more sampling fun 0
Subsystems (4)
cover v3 more sampling fun 0
From: mark gross <[email protected]>
Subject: [PATCH 0/2] V3: more sampling funn
This is a significantly reworked version of the mitigation control I think
I've addressed all the high quality feedback give on the first 2 versions.
This version of the mitigation no longer needs to change the structure of cpu_vuln_whitelist as it creates a similar "affected_processors" list.
---
Special Register Buffer Data Sampling is a sampling type of vulnerability that leaks data across cores sharing the HW-RNG for vulnerable processors.
This leak is fixed by a microcode update and is enabled by default.
This new microcode serializes processor access during execution of RDRAND or RDSEED. It ensures that the shared buffer is overwritten before it is released for reuse.
The mitigation impacts the throughput of the RDRAND and RDSEED instructions and latency of RT processing running on the socket while executing RDRAND or RDSEED. The micro benchmark of calling RDRAND many times shows a slowdown.
This patch set enables kernel command line control of this mitigation and exports vulnerability and mitigation status.
This patch set includes 2 patches:
The fist patch enables the command line control of the mitigation as well as the sysfs export of vulnerability status.
The second patch has the Documentation/admin-guide/hw-vuln documentation for the issue and the control over the mitigation.
The microcode defaults to enabling the mitigation.
mark gross (2):
x86/speculation: Special Register Buffer Data Sampling (SRBDS)
mitigation control.
x86/speculation: SRBDS vulnerability and mitigation documentation
Documentation/admin-guide/hw-vuln/index.rst | 2 + .../special-register-buffer-data-sampling.rst | 145 ++++++++++++++++++ .../admin-guide/kernel-parameters.txt | 11 ++ arch/x86/include/asm/cpu_device_id.h | 12 ++ arch/x86/include/asm/cpufeatures.h | 2 + arch/x86/include/asm/msr-index.h | 4 + arch/x86/kernel/cpu/bugs.c | 110 +++++++++++++ arch/x86/kernel/cpu/common.c | 41 +++++ arch/x86/kernel/cpu/cpu.h | 13 ++ arch/x86/kernel/cpu/intel.c | 2 + arch/x86/kernel/cpu/match.c | 26 ++++ drivers/base/cpu.c | 8 + 12 files changed, 376 insertions(+) create mode 100644 Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Signature · 1 line
2.17.1
PATCH 1/2 v3 more sampling fun 1
From: mark gross <[email protected]>
Subject: [PATCH 1/2] x86/speculation: Special Register Buffer Data Sampling
(SRBDS) mitigation control.
SRBDS is an MDS-like speculative side channel that can leak bits from the RNG across cores and threads. New microcode serializes the processor access during the execution of RDRAND and RDSEED ensures that the shared buffer is overwritten before it is released for reuse.
This vulnerability comes in two classes:
Processor models that are vulnerable and models with TSX enumerating MDS_NO which are only vulnerable when TSX is enabled.
The mitigation is activated by default on affected processors and it increases latency for RDRAND and RDSEED instructions. Among other effects this will reduce throughput from /dev/urandom.
This patch:
* enables administrator to configure the mitigation off when desired using either mitigations=off or srbds=off.
* exports vulnerability status via sysfs
--- .../admin-guide/kernel-parameters.txt | 11 ++ arch/x86/include/asm/cpu_device_id.h | 12 ++ arch/x86/include/asm/cpufeatures.h | 2 + arch/x86/include/asm/msr-index.h | 4 + arch/x86/kernel/cpu/bugs.c | 110 ++++++++++++++++++ arch/x86/kernel/cpu/common.c | 41 +++++++ arch/x86/kernel/cpu/cpu.h | 13 +++ arch/x86/kernel/cpu/intel.c | 2 + arch/x86/kernel/cpu/match.c | 26 +++++ drivers/base/cpu.c | 8 ++ 10 files changed, 229 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +11 -0
@@ -4659,6 +4659,17 @@ spia_pedr= spia_peddr= + srbds= [x86] + Special Register Buffer Data Sampling mitigation + control. + + On CPUs vulnerable to this issue and users impacted by + the mitigation slowing down RDRAND and RDSEED can + disable by setting this: + + off: disable mitigation and remove performance + impact to rdrand and rdseed + srcutree.counter_wrap_check [KNL] Specifies how frequently to check for grace-period sequence counter wrap for the
arch/x86/include/asm/cpu_device_id.h +12 -0
@@ -35,7 +35,19 @@ struct x86_cpu_desc { .x86_microcode_rev = (revision), \ } +/* +* Match a range of steppings +*/ + +struct x86_cpu_id_ext { +struct x86_cpu_id id; +__u16 steppings;/* bit map of steppings to match against */ +}; + +#define X86_STEPPING_ANY GENMASK(15, 0) +extern const struct x86_cpu_id *x86_match_cpu (const struct x86_cpu_id *match); +const struct x86_cpu_id_ext *x86_match_cpu_ext (const struct x86_cpu_id_ext *match);extern bool x86_cpu_has_min_microcode_rev (const struct x86_cpu_desc *table);#endif /* _ASM_X86_CPU_DEVICE_ID */
arch/x86/include/asm/cpufeatures.h +2 -0
@@ -360,6 +360,7 @@#define X86_FEATURE_AVX512_4FMAPS (18*32+ 3)/* AVX-512 Multiply Accumulation Single precision */ #define X86_FEATURE_FSRM (18*32+ 4)/* Fast Short Rep Mov */ #define X86_FEATURE_AVX512_VP2INTERSECT (18*32+ 8)/* AVX-512 Intersect for D/Q */ +#define X86_FEATURE_SRBDS_CTRL (18*32+ 9)/* "" SRBDS mitigation MSR available */ #define X86_FEATURE_MD_CLEAR (18*32+10)/* VERW clears CPU buffers */ #define X86_FEATURE_TSX_FORCE_ABORT (18*32+13)/* "" TSX_FORCE_ABORT */ #define X86_FEATURE_PCONFIG (18*32+18)/* Intel PCONFIG */
@@ -404,5 +405,6 @@#define X86_BUG_SWAPGS X86_BUG(21)/* CPU is affected by speculation through SWAPGS */ #define X86_BUG_TAA X86_BUG(22)/* CPU is affected by TSX Async Abort(TAA) */ #define X86_BUG_ITLB_MULTIHIT X86_BUG(23)/* CPU may incur MCE during certain page attribute changes */ +#define X86_BUG_SRBDS X86_BUG(24)/* CPU may leak RNG bits if not mitigated */ #endif /* _ASM_X86_CPUFEATURES_H */
arch/x86/include/asm/msr-index.h +4 -0
@@ -119,6 +119,10 @@#define TSX_CTRL_RTM_DISABLE BIT(0)/* Disable RTM feature */ #define TSX_CTRL_CPUID_CLEAR BIT(1)/* Disable TSX enumeration */ +/* SRBDS support */ +#define MSR_IA32_MCU_OPT_CTRL 0x00000123 +#define RNGDS_MITG_DIS BIT(0) +#define MSR_IA32_SYSENTER_CS 0x00000174#define MSR_IA32_SYSENTER_ESP 0x00000175#define MSR_IA32_SYSENTER_EIP 0x00000176
arch/x86/kernel/cpu/bugs.c +110 -0
144 lines not on this page · raw message
arch/x86/kernel/cpu/common.c +41 -0
71 lines not on this page · raw message
arch/x86/kernel/cpu/cpu.h +13 -0
@@ -44,7 +44,20 @@ struct _tlb_table {extern const struct cpu_dev *const __x86_cpu_dev_start[], *const __x86_cpu_dev_end[]; +enum srbds_mitigations { +SRBDS_NOT_AFFECTED , +SRBDS_MITIGATION_OFF , +SRBDS_MITIGATION_UCODE_NEEDED , +SRBDS_MITIGATION_FULL , +SRBDS_TSX_NOT_AFFECTED , +SRBDS_HYPERVISOR , +}; + +extern __ro_after_init enum srbds_mitigations srbds_mitigation; +void srbds_configure_mitigation (void ); +#ifdef CONFIG_CPU_SUP_INTEL +enum tsx_ctrl_states {TSX_CTRL_ENABLE , TSX_CTRL_DISABLE,
arch/x86/kernel/cpu/intel.c +2 -0
@@ -684,6 +684,8 @@ static void init_intel(struct cpuinfo_x86 *c)tsx_enable ();if (tsx_ctrl_state ==TSX_CTRL_DISABLE )tsx_disable (); + +srbds_configure_mitigation (); }#ifdef CONFIG_X86_32
arch/x86/kernel/cpu/match.c +26 -0
@@ -49,6 +49,32 @@ const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match) }EXPORT_SYMBOL (x86_match_cpu); +/* +* Extend x86_match_cpu to support matching a range of steppings. +*/ +const struct x86_cpu_id_ext *x86_match_cpu_ext (const struct x86_cpu_id_ext *match) +{ +const struct x86_cpu_id_ext *m; +struct cpuinfo_x86 *c = &boot_cpu_data; + +for (m = match; m->id.vendor | m->id.family | m->id.model | m->id.feature; m++) { +if (m->id.vendor !=X86_VENDOR_ANY && c->x86_vendor != m->id.vendor) +continue ; +if (m->id.family !=X86_FAMILY_ANY && c->x86 != m->id.family) +continue ; +if (m->id.model !=X86_MODEL_ANY && c->x86_model != m->id.model) +continue ; +if (m->steppings !=X86_STEPPING_ANY && + !(BIT (c->x86_stepping) & m->steppings)) +continue ; +if (m->id.feature !=X86_FEATURE_ANY && !cpu_has (c, m->id.feature)) +continue ; +return m; + } +return NULL ; +} +EXPORT_SYMBOL (x86_match_cpu_ext); +static const struct x86_cpu_desc *x86_match_cpu_with_stepping (const struct x86_cpu_desc *match) {
drivers/base/cpu.c +8 -0
@@ -567,6 +567,12 @@ ssize_t __weak cpu_show_itlb_multihit(struct device *dev,return sprintf (buf,"Not affected\n" ); } +ssize_t __weak cpu_show_special_register_data_sampling (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"Not affected\n" ); +} + static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL); static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL); static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);
@@ -575,6 +581,7 @@ static DEVICE_ATTR(l1tf, 0444, cpu_show_l1tf, NULL); static DEVICE_ATTR(mds, 0444, cpu_show_mds, NULL); static DEVICE_ATTR(tsx_async_abort, 0444, cpu_show_tsx_async_abort, NULL); static DEVICE_ATTR(itlb_multihit, 0444, cpu_show_itlb_multihit, NULL); +static DEVICE_ATTR(special_register_data_sampling, 0444, cpu_show_special_register_data_sampling, NULL);static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_meltdown.attr,
@@ -585,6 +592,7 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_mds.attr, &dev_attr_tsx_async_abort.attr, &dev_attr_itlb_multihit.attr, + &dev_attr_special_register_data_sampling.attr, NULL };
Signature · 1 line
2.17.1
PATCH 2/2 v3 more sampling fun 2
In reply to mark grossFrom: mark gross <[email protected]>
Subject: [PATCH 2/2] x86/speculation: SRBDS vulnerability and mitigation
documentation
Adds documentation for the SRBDS vulnerability and its mitigation.
--- Documentation/admin-guide/hw-vuln/index.rst | 2 + .../special-register-buffer-data-sampling.rst | 145 ++++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Documentation/admin-guide/hw-vuln/index.rst +2 -0
@@ -14,3 +14,5 @@ are configurable at compile, boot or run time. mds tsx_async_abort multihit.rst + special-register-buffer-data-sampling.rst +
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +145 -0
145 lines not on this page · raw message
Signature · 1 line
2.17.1
A message that is not in the archive (sent off-list or not yet received): [email protected]
Greg KH
maintainer[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
Quoted 24 lines · speck for mark gross
drivers/base/cpu.c +7
-0@@ -567,6 +567,12 @@ ssize_t __weak cpu_show_itlb_multihit(struct device *dev,return sprintf (buf,"Not affected\n" ); } +ssize_t __weak cpu_show_special_register_data_sampling (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"Not affected\n" ); +} + static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL); static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL); static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);@@ -575,6 +581,7 @@ static DEVICE_ATTR(l1tf, 0444, cpu_show_l1tf, NULL); static DEVICE_ATTR(mds, 0444, cpu_show_mds, NULL); static DEVICE_ATTR(tsx_async_abort, 0444, cpu_show_tsx_async_abort, NULL); static DEVICE_ATTR(itlb_multihit, 0444, cpu_show_itlb_multihit, NULL); +static DEVICE_ATTR(special_register_data_sampling, 0444, cpu_show_special_register_data_sampling, NULL);static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_meltdown.attr,
drivers/base/cpu.c static struct attribute *cpu_root_vulnerabilities_attrs[] = { · line 597
Quoted 6 lines
drivers/base/cpu.c +1
-0@@ -585,6 +592,7 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_mds.attr, &dev_attr_tsx_async_abort.attr, &dev_attr_itlb_multihit.attr, + &dev_attr_special_register_data_sampling.attr, NULL };
You forgot a Documentation/ABI/ entry for this new sysfs file like I pointed out previously :(
thanks,
greg k-h
[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
Quoted 22 lines · speck for Greg KH
Quoted 21 lines · speck for mark gross
drivers/base/cpu.c +7
-0@@ -567,6 +567,12 @@ ssize_t __weak cpu_show_itlb_multihit(struct device *dev,return sprintf (buf,"Not affected\n" ); } +ssize_t __weak cpu_show_special_register_data_sampling (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"Not affected\n" ); +} + static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL); static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL); static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);@@ -575,6 +581,7 @@ static DEVICE_ATTR(l1tf, 0444, cpu_show_l1tf, NULL); static DEVICE_ATTR(mds, 0444, cpu_show_mds, NULL); static DEVICE_ATTR(tsx_async_abort, 0444, cpu_show_tsx_async_abort, NULL); static DEVICE_ATTR(itlb_multihit, 0444, cpu_show_itlb_multihit, NULL); +static DEVICE_ATTR(special_register_data_sampling, 0444, cpu_show_special_register_data_sampling, NULL);
Quoted 12 lines
drivers/base/cpu.c static struct attribute *cpu_root_vulnerabilities_attrs[] = { · line 597
Quoted 9 lines
drivers/base/cpu.c +1
-0static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_meltdown.attr,@@ -585,6 +592,7 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_mds.attr, &dev_attr_tsx_async_abort.attr, &dev_attr_itlb_multihit.attr, + &dev_attr_special_register_data_sampling.attr, NULL };You forgot a
Documentation/ABI/entry for this new sysfs file like I pointed out previously :(
I thought you where talking about the command line arguments not being documented.
I'll add this.
--mark
Re: [PATCH 1/2] v3 more sampling fun 1
In reply to mark grossMark,
Quoted 3 lines · speck for mark gross
From: mark gross <[email protected]>
This patch:
git grep 'This patch' Documentation/process/
Review
Quoted 11 lines
* enables administrator to configure the mitigation off when desired using either mitigations=off or srbds=off.
* exports vulnerability status via sysfs(quoted diff) +7
-0+/* + * Match a range of steppings + */ + +struct x86_cpu_id_ext { + struct x86_cpu_id id; + __u16 steppings; /* bit map of steppings to match against */
IIRC, we asked for adding the stepping to the existing data structure, but I can't find any rationale somewhere why this is still separate.
If you really think hard about it then this is not needed at all. See below.
Quoted 10 lines
(quoted diff) +10
-0+static bool srbds_off; + +void srbds_configure_mitigation(void) +{ + u64 mcu_ctrl; + + if (srbds_mitigation == SRBDS_NOT_AFFECTED) + return; + + if (srbds_mitigation == SRBDS_HYPERVISOR)
Review
Quoted 10 lines
(quoted diff) +10
-0+ return; + + if (srbds_mitigation == SRBDS_MITIGATION_UCODE_NEEDED) + return; + + rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); + + switch (srbds_mitigation) { + case SRBDS_MITIGATION_OFF: + case SRBDS_TSX_NOT_AFFECTED:
This mitigation state confuses the hell out of me. The text says:
+ [SRBDS_TSX_NOT_AFFECTED] = "Not affected (TSX disabled)",
But the enum value reads to me: TSX is not affected....
SRBDS_NOT_AFFECTED_TSX_OFF
is a bit more intuitive. Hmm?
Quoted 28 lines
(quoted diff) +28
-0+ mcu_ctrl |= RNGDS_MITG_DIS; + break; + case SRBDS_MITIGATION_FULL: + mcu_ctrl &= ~RNGDS_MITG_DIS; + break; + default: + break; + } + + wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); +} + +static void __init srbds_select_mitigation(void) +{ + u64 ia32_cap; + + if (!boot_cpu_has_bug(X86_BUG_SRBDS)) { + srbds_mitigation = SRBDS_NOT_AFFECTED; + return; + } + + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) { + srbds_mitigation = SRBDS_HYPERVISOR; + return; + } + + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) { + srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED;
Review
Quoted 10 lines
(quoted diff) +10
-0+ return; + } + + if (boot_cpu_has_bug(X86_BUG_SRBDS)) { + srbds_mitigation = SRBDS_MITIGATION_FULL; + + ia32_cap = x86_read_arch_cap_msr(); + if (ia32_cap & ARCH_CAP_MDS_NO) { + if (!boot_cpu_has(X86_FEATURE_RTM)) + srbds_mitigation = SRBDS_TSX_NOT_AFFECTED;
This logic comes with an awesome amount of comments...
Quoted 12 lines
(quoted diff) +12
-0+ } + } + + if (cpu_mitigations_off() || srbds_off) { + if (srbds_mitigation != SRBDS_TSX_NOT_AFFECTED) + srbds_mitigation = SRBDS_MITIGATION_OFF; + } + + srbds_configure_mitigation(); +} + +static int __init srbds_parse_cmdline(char *str)
Review
Quoted 10 lines
(quoted diff) +10
-0+{ + if (!str) + return -EINVAL; + + if (!strcmp(str, "off")) + srbds_off = true; + + return 0; +} +
stray newline
Review static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c) · line 1026
Quoted 12 lines
(quoted diff) +5
-0+early_param("srbds", srbds_parse_cmdline); + #define VULNWL(_vendor, _family, _model, _whitelist) \ { X86_VENDOR_##_vendor, _family, _model, X86_FEATURE_ANY, _whitelist }@@ -1020,6 +1021,15 @@ static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c) #define VULNWL_HYGON(family, whitelist) \ VULNWL(HYGON, family, X86_MODEL_ANY, whitelist) +#define VULNWL_EXT(_vendor, _family, _model, _steppings, _whitelist) \ + { VULNWL(_vendor, _family, _model, _whitelist), _steppings } +
And because this is used for a blacklist the prefix VULNWL, aka VULNerability White List, and the last argument make a lot of sense, right?
Quoted 10 lines
(quoted diff) +6
-0+#define VULNWL_INTEL_EXT(model, whitelist) \ + VULNWL_EXT(INTEL, 6, INTEL_FAM6_##model, X86_STEPPING_ANY, whitelist) + +#define VULNWL_INTEL_STEPPING(model, stepping, whitelist) \ + VULNWL_EXT(INTEL, 6, INTEL_FAM6_##model, stepping, whitelist) + static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { VULNWL(ANY, 4, X86_MODEL_ANY, NO_SPECULATION), VULNWL(CENTAUR, 5, X86_MODEL_ANY, NO_SPECULATION),
Review static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { · line 1089
Quoted 5 lines
(quoted diff) +2
-0@@ -1075,6 +1085,27 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { {} }; +/* + * to avoide corrupting the whiltelist with blacklist items lets create a list
Sentences start with uppercase letters and spell checking is available in most editors. Now for the content:
There is nothing to corrupt. Blacklists and whitelists do not mix.
Also what means 'lets create' here? This is a comment describing what the following array is used for. Facts please.
Quoted 7 lines
(quoted diff) +5
-0+ * of affected processors for issues that cannot be enumerated other than by + * family/model/stepping + */ +static const struct x86_cpu_id_ext affected_cpus[] __initconst = { + VULNWL_INTEL_EXT(IVYBRIDGE, SRBDS),
Review static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { · line 1106
Quoted 10 lines
(quoted diff) +9
-0+ VULNWL_INTEL_EXT(HASWELL, SRBDS), + VULNWL_INTEL_EXT(HASWELL_L, SRBDS), + VULNWL_INTEL_EXT(HASWELL_G, SRBDS), + VULNWL_INTEL_EXT(BROADWELL_G, SRBDS), + VULNWL_INTEL_EXT(BROADWELL, SRBDS), + VULNWL_INTEL_EXT(SKYLAKE_L, SRBDS), + VULNWL_INTEL_EXT(SKYLAKE, SRBDS), + VULNWL_INTEL_STEPPING(KABYLAKE_L, GENMASK(0xA, 0), SRBDS), /*06_8E steppings <=A*/ + VULNWL_INTEL_STEPPING(KABYLAKE_L, GENMASK(0xC, 0xB), SRBDS), /*06_8E stepping = 0xB|0xC if TSX enabled*/
This is beyond confusing because this should either be expressed in the vulnerability itself, i.e. SRBDS_TSX_ONLY, or just commented along with the comment in srbds_select_mitigation()
Review static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { · line 1108
Quoted 2 lines
(quoted diff) +2
-0+ VULNWL_INTEL_STEPPING(KABYLAKE, GENMASK(0xB, 0), SRBDS), /*06_9E steppings <=B*/ + VULNWL_INTEL_STEPPING(KABYLAKE, GENMASK(0xD, 0xC), SRBDS), /*06_9E stepping = 0xC if TSX enabled*/
Comment and code do not match.
Aside of this whole thing is utter garbage, really.
#define X86_STEPPING_MAX 15
#define STEPSHIFT 16
#define ISVULN(_vendor, family, model, minstep, maxstep, vulns) \
{ X86_VENDOR_##_vendor, _family, _model, X86_FEATURE_ANY, \
GENMASK(minstep, maxstep) << STEPSHIFT | vulns }#define ISVULN_INTEL(model, minstep, maxstep, vulns) \
ISVULN(INTEL, 6, INTEL_FAM6_##model, minstep, maxstep, vulns)
....
/* List of affected CPUs identified by model and stepping range. */
static const struct x86_cpu_id affected_cpus[] __initconst = {
ISVULN_INTEL(HASWELL, 0, X86_STEPPING_MAX, SRBDS),
ISVULN_INTEL(BROADWELL_G, 0, X86_STEPPING_MAX, SRBDS),
/* Kabylake L steppings 0xB, 0xC only affected when TSX in on */
ISVULN_INTEL(KABYLAKE_L, 0, 0xC, SRBDS),
/* Kabylake steppings 0xC, 0xD only affected when TSX in on */
ISVULN_INTEL(KABYLAKE, 0, 0xD, SRBDS),
{}
};Now:
static bool __init cpu_matches(unsigned long which)
{
const struct x86_cpu_id *m = x86_match_cpu(cpu_vuln_whitelist);
- return m && !!(m->driver_data & which);
+ return m && (m->driver_data & which) == which;
}
Review static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = {
Quoted 6 lines
(quoted diff) +6
-0+static bool __init cpu_affected(unsigned long which) +{ + const struct x86_cpu_id_ext *m = x86_match_cpu_ext(affected_cpus); + + return m && !!(m->id.driver_data & which); +}
Which makes this go away
Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1166
Quoted 9 lines
u64
x86_read_arch_cap_msr(void)
{(quoted diff) +2
-0u64 ia32_cap = 0;@@ -1124,6 +1162,9 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) if (!cpu_matches(NO_SWAPGS)) setup_force_cpu_bug(X86_BUG_SWAPGS); + if (cpu_affected(SRBDS)) + setup_force_cpu_bug(X86_BUG_SRBDS);
and this becomes:
+ if (cpu_matches(BIT(boot_cpu_data.x86_stepping + STEPSHIFT) | which))
+ setup_force_cpu_bug(X86_BUG_SRBDS);
Too much code reuse, right?
Quoted 11 lines
(quoted diff) +0
-0/* * When the CPU is not mitigated for TAA (TAA_NO=0) set TAA bug when: * - TSX is supported orarch/x86/kernel/cpu/cpu.h +0
-0@@ -44,7 +44,20 @@ struct _tlb_table {extern const struct cpu_dev *const __x86_cpu_dev_start[], *const __x86_cpu_dev_end[];
arch/x86/kernel/cpu/cpu.h struct _tlb_table { · line 56
Quoted 10 lines
arch/x86/kernel/cpu/cpu.h +10
-0+enum srbds_mitigations { +SRBDS_NOT_AFFECTED , +SRBDS_MITIGATION_OFF , +SRBDS_MITIGATION_UCODE_NEEDED , +SRBDS_MITIGATION_FULL , +SRBDS_TSX_NOT_AFFECTED , +SRBDS_HYPERVISOR , +}; + +extern __ro_after_init enum srbds_mitigations srbds_mitigation;
And this needs to be public because the only user is in bugs.c, right?
Quoted 43 lines
arch/x86/kernel/cpu/cpu.h +3
-0+void srbds_configure_mitigation (void ); +#ifdef CONFIG_CPU_SUP_INTEL +enum tsx_ctrl_states {TSX_CTRL_ENABLE , TSX_CTRL_DISABLE,arch/x86/kernel/cpu/intel.c +2
-0@@ -684,6 +684,8 @@ static void init_intel(struct cpuinfo_x86 *c)tsx_enable ();if (tsx_ctrl_state ==TSX_CTRL_DISABLE )tsx_disable (); + +srbds_configure_mitigation (); }#ifdef CONFIG_X86_32 arch/x86/kernel/cpu/match.c +15
-0@@ -49,6 +49,32 @@ const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match) }EXPORT_SYMBOL (x86_match_cpu); +/* +* Extend x86_match_cpu to support matching a range of steppings. +*/ +const struct x86_cpu_id_ext *x86_match_cpu_ext (const struct x86_cpu_id_ext *match) +{ +const struct x86_cpu_id_ext *m; +struct cpuinfo_x86 *c = &boot_cpu_data; + +for (m = match; m->id.vendor | m->id.family | m->id.model | m->id.feature; m++) { +if (m->id.vendor !=X86_VENDOR_ANY && c->x86_vendor != m->id.vendor) +continue ; +if (m->id.family !=X86_FAMILY_ANY && c->x86 != m->id.family) +continue ; +if (m->id.model !=X86_MODEL_ANY && c->x86_model != m->id.model) +continue ;
arch/x86/kernel/cpu/match.c const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match) · line 76
Quoted 10 lines
arch/x86/kernel/cpu/match.c +10
-0+if (m->steppings !=X86_STEPPING_ANY && + !(BIT (c->x86_stepping) & m->steppings)) +continue ; +if (m->id.feature !=X86_FEATURE_ANY && !cpu_has (c, m->id.feature)) +continue ; +return m; + } +return NULL ; +} +EXPORT_SYMBOL (x86_match_cpu_ext);
Sigh, aside of being pointless duplicated code:
If we'd really need this then it can share most of the code with x86_match_cpu(), but copy and paste is more fancy, right? You even copied the export just in case ...
Quoted 29 lines
arch/x86/kernel/cpu/match.c +0
-0static const struct x86_cpu_desc *x86_match_cpu_with_stepping (const struct x86_cpu_desc *match) {drivers/base/cpu.c +7
-0@@ -567,6 +567,12 @@ ssize_t __weak cpu_show_itlb_multihit(struct device *dev,return sprintf (buf,"Not affected\n" ); } +ssize_t __weak cpu_show_special_register_data_sampling (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"Not affected\n" ); +} + static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL); static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL); static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);@@ -575,6 +581,7 @@ static DEVICE_ATTR(l1tf, 0444, cpu_show_l1tf, NULL); static DEVICE_ATTR(mds, 0444, cpu_show_mds, NULL); static DEVICE_ATTR(tsx_async_abort, 0444, cpu_show_tsx_async_abort, NULL); static DEVICE_ATTR(itlb_multihit, 0444, cpu_show_itlb_multihit, NULL); +static DEVICE_ATTR(special_register_data_sampling, 0444, cpu_show_special_register_data_sampling, NULL);static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_meltdown.attr,
drivers/base/cpu.c static struct attribute *cpu_root_vulnerabilities_attrs[] = { · line 595
Quoted 4 lines
drivers/base/cpu.c +1
-0@@ -585,6 +592,7 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_mds.attr, &dev_attr_tsx_async_abort.attr, &dev_attr_itlb_multihit.attr, + &dev_attr_special_register_data_sampling.attr,
This still lacks an entry in:
Documentation/ABI/testing/sysfs-devices-system-cpu
as requested by Greg several times.
Thanks,
tglx
[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
Quoted 8 lines · speck for Thomas Gleixner
Mark,
Quoted 3 lines · speck for mark gross
From: mark gross <[email protected]>
This patch:
git grep 'This patch'
Documentation/process/
Interpretive mood / give orders to the codebase to change its behavior.
ok.
Quoted 5 lines
Quoted 5 lines
* enables administrator to configure the mitigation off when desired using either mitigations=off or srbds=off.
* exports vulnerability status via sysfs
Quoted 10 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+/* + * Match a range of steppings + */ + +struct x86_cpu_id_ext { + struct x86_cpu_id id; + __u16 steppings; /* bit map of steppings to match against */IIRC, we asked for adding the stepping to the existing data structure, but I can't find any rationale somewhere why this is still separate.
Changed to change the x86_cpu_id to append a "steppings" member at the end.
Quoted 27 lines
If you really think hard about it then this is not needed at all. See below.
Quoted 10 lines
(quoted diff) +10
-0+static bool srbds_off; + +void srbds_configure_mitigation(void) +{ + u64 mcu_ctrl; + + if (srbds_mitigation == SRBDS_NOT_AFFECTED) + return; + + if (srbds_mitigation == SRBDS_HYPERVISOR)Review
Quoted 10 lines
(quoted diff) +10
-0+ return; + + if (srbds_mitigation == SRBDS_MITIGATION_UCODE_NEEDED) + return; + + rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); + + switch (srbds_mitigation) { + case SRBDS_MITIGATION_OFF: + case SRBDS_TSX_NOT_AFFECTED:This mitigation state confuses the hell out of me. The text says:
Quoted 7 lines
+ [
SRBDS_TSX_NOT_AFFECTED] = "Not affected (TSX disabled)",But the enum value reads to me: TSX is not affected....
SRBDS_NOT_AFFECTED_TSX_OFFis a bit more intuitive. Hmm?
changed to SRBDS_NOT_AFFECTED_TSX_OFF.
Quoted 33 lines
Quoted 33 lines
(quoted diff) +32
-0+ mcu_ctrl |= RNGDS_MITG_DIS; + break; + case SRBDS_MITIGATION_FULL: + mcu_ctrl &= ~RNGDS_MITG_DIS; + break; + default: + break; + } + + wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); +} + +static void __init srbds_select_mitigation(void) +{ + u64 ia32_cap; + + if (!boot_cpu_has_bug(X86_BUG_SRBDS)) { + srbds_mitigation = SRBDS_NOT_AFFECTED; + return; + } + + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) { + srbds_mitigation = SRBDS_HYPERVISOR; + return; + } + + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) { + srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED; + return; + } + + if (boot_cpu_has_bug(X86_BUG_SRBDS)) {
Quoted 8 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ srbds_mitigation = SRBDS_MITIGATION_FULL; + + ia32_cap = x86_read_arch_cap_msr(); + if (ia32_cap & ARCH_CAP_MDS_NO) { + if (!boot_cpu_has(X86_FEATURE_RTM)) + srbds_mitigation = SRBDS_TSX_NOT_AFFECTED;This logic comes with an awesome amount of comments...
Added comment about checking to see if this is one of the MDS_NO systems that supports TSX where they are only vulnerable if TSX is enabled.
Quoted 35 lines
Quoted 13 lines
(quoted diff) +12
-0+ } + } + + if (cpu_mitigations_off() || srbds_off) { + if (srbds_mitigation != SRBDS_TSX_NOT_AFFECTED) + srbds_mitigation = SRBDS_MITIGATION_OFF; + } + + srbds_configure_mitigation(); +} + +static int __init srbds_parse_cmdline(char *str)Review
Quoted 10 lines
(quoted diff) +10
-0+{ + if (!str) + return -EINVAL; + + if (!strcmp(str, "off")) + srbds_off = true; + + return 0; +} +stray newline
Quoted 9 lines
(quoted diff) +2
-0+early_param("srbds", srbds_parse_cmdline); + #define VULNWL(_vendor, _family, _model, _whitelist) \ { X86_VENDOR_##_vendor, _family, _model, X86_FEATURE_ANY, _whitelist }@@ -1020,6 +1021,15 @@ static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c) #define VULNWL_HYGON(family, whitelist) \ VULNWL(HYGON, family, X86_MODEL_ANY, whitelist)
Quoted 7 lines
Review static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c) · line 1026
Quoted 3 lines
(quoted diff) +3
-0+#define VULNWL_EXT(_vendor, _family, _model, _steppings, _whitelist) \ + { VULNWL(_vendor, _family, _model, _whitelist), _steppings } +And because this is used for a blacklist the prefix VULNWL, aka VULNerability White List, and the last argument make a lot of sense, right?
right. Would it be ok to s/whitelist/issues or issue_mask for all these? That structure is a bitmask and not a list anyway.
Quoted 20 lines
Quoted 11 lines
(quoted diff) +6
-0+#define VULNWL_INTEL_EXT(model, whitelist) \ + VULNWL_EXT(INTEL, 6, INTEL_FAM6_##model, X86_STEPPING_ANY, whitelist) + +#define VULNWL_INTEL_STEPPING(model, stepping, whitelist) \ + VULNWL_EXT(INTEL, 6, INTEL_FAM6_##model, stepping, whitelist) + static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { VULNWL(ANY, 4, X86_MODEL_ANY, NO_SPECULATION), VULNWL(CENTAUR, 5, X86_MODEL_ANY, NO_SPECULATION),Review static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { · line 1089
Quoted 5 lines
(quoted diff) +2
-0@@ -1075,6 +1085,27 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { {} }; +/* + * to avoide corrupting the whiltelist with blacklist items lets create a listSentences start with uppercase letters and spell checking is available in most editors. Now for the content:
Quoted 4 lines
There is nothing to corrupt. Blacklists and whitelists do not mix.
Also what means 'lets create' here? This is a comment describing what the following array is used for. Facts please.
Changed / reworded and spell checked.
Quoted 8 lines
Quoted 6 lines
(quoted diff) +5
-0+ * of affected processors for issues that cannot be enumerated other than by + * family/model/stepping + */ +static const struct x86_cpu_id_ext affected_cpus[] __initconst = { + VULNWL_INTEL_EXT(IVYBRIDGE, SRBDS),
Quoted 14 lines
Review static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { · line 1104
Quoted 10 lines
(quoted diff) +9
-0+ VULNWL_INTEL_EXT(HASWELL, SRBDS), + VULNWL_INTEL_EXT(HASWELL_L, SRBDS), + VULNWL_INTEL_EXT(HASWELL_G, SRBDS), + VULNWL_INTEL_EXT(BROADWELL_G, SRBDS), + VULNWL_INTEL_EXT(BROADWELL, SRBDS), + VULNWL_INTEL_EXT(SKYLAKE_L, SRBDS), + VULNWL_INTEL_EXT(SKYLAKE, SRBDS), + VULNWL_INTEL_STEPPING(KABYLAKE_L, GENMASK(0xA, 0), SRBDS), /*06_8E steppings <=A*/ + VULNWL_INTEL_STEPPING(KABYLAKE_L, GENMASK(0xC, 0xB), SRBDS), /*06_8E stepping = 0xB|0xC if TSX enabled*/This is beyond confusing because this should either be expressed in the vulnerability itself, i.e.
SRBDS_TSX_ONLY, or just commented along with the comment insrbds_select_mitigation()
reduced by 2 entries and removed talk of TSX enabled
Quoted 5 lines
Review static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { · line 1106
Quoted 3 lines
(quoted diff) +2
-0+ VULNWL_INTEL_STEPPING(KABYLAKE, GENMASK(0xB, 0), SRBDS), /*06_9E steppings <=B*/ + VULNWL_INTEL_STEPPING(KABYLAKE, GENMASK(0xD, 0xC), SRBDS), /*06_9E stepping = 0xC if TSX enabled*/Comment and code do not match.
Fixed.
Quoted 36 lines
Aside of this whole thing is utter garbage, really.
#define
X86_STEPPING_MAX15
#define STEPSHIFT 16#define ISVULN(_vendor, family, model, minstep, maxstep, vulns) \ { X86_VENDOR_##_vendor, _family, _model, X86_FEATURE_ANY, \ GENMASK(minstep, maxstep) << STEPSHIFT | vulns }#define
ISVULN_INTEL(model, minstep, maxstep, vulns)\
ISVULN(INTEL, 6, INTEL_FAM6_##model, minstep, maxstep, vulns)....
/* List of affected CPUs identified by model and stepping range. */
static const struct x86_cpu_id affected_cpus[] __initconst = { ISVULN_INTEL(HASWELL, 0, X86_STEPPING_MAX, SRBDS), ISVULN_INTEL(BROADWELL_G, 0, X86_STEPPING_MAX, SRBDS), /* Kabylake L steppings 0xB, 0xC only affected when TSX in on */ ISVULN_INTEL(KABYLAKE_L, 0, 0xC, SRBDS), /* Kabylake steppings 0xC, 0xD only affected when TSX in on */ ISVULN_INTEL(KABYLAKE, 0, 0xD, SRBDS), {} };Now:
static bool
__initcpu_matches(unsigned long which)
{
const structx86_cpu_id*m =x86_match_cpu(cpu_vuln_whitelist);(quoted diff) +1
-1- return m && !!(m->driver_data & which); + return m && (m->driver_data & which) == which;
Quoted 2 lines
}
Yes, this is a clever use of encoding steppings into 16 bits of the driver_data member but, its not very straight forward compared to adding a steppings data member to the end of the existing structure.
Quoted 24 lines
Review static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = {
Quoted 6 lines
(quoted diff) +6
-0+static bool __init cpu_affected(unsigned long which) +{ + const struct x86_cpu_id_ext *m = x86_match_cpu_ext(affected_cpus); + + return m && !!(m->id.driver_data & which); +}Which makes this go away
Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1166
Quoted 9 lines
u64
x86_read_arch_cap_msr(void)
{(quoted diff) +2
-0u64 ia32_cap = 0;@@ -1124,6 +1162,9 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) if (!cpu_matches(NO_SWAPGS)) setup_force_cpu_bug(X86_BUG_SWAPGS); + if (cpu_affected(SRBDS)) + setup_force_cpu_bug(X86_BUG_SRBDS);and this becomes:
(quoted diff) +2
-0+ if (cpu_matches(BIT(boot_cpu_data.x86_stepping + STEPSHIFT) | which)) + setup_force_cpu_bug(X86_BUG_SRBDS);
Quoted 1 line
Too much code reuse, right?
Maybe.
Quoted 12 lines
Quoted 12 lines
(quoted diff) +0
-0/* * When the CPU is not mitigated for TAA (TAA_NO=0) set TAA bug when: * - TSX is supported orarch/x86/kernel/cpu/cpu.h +0
-0@@ -44,7 +44,20 @@ struct _tlb_table {extern const struct cpu_dev *const __x86_cpu_dev_start[], *const __x86_cpu_dev_end[];
Quoted 12 lines
arch/x86/kernel/cpu/cpu.h struct _tlb_table { · line 56
Quoted 10 lines
arch/x86/kernel/cpu/cpu.h +10
-0+enum srbds_mitigations { +SRBDS_NOT_AFFECTED , +SRBDS_MITIGATION_OFF , +SRBDS_MITIGATION_UCODE_NEEDED , +SRBDS_MITIGATION_FULL , +SRBDS_TSX_NOT_AFFECTED , +SRBDS_HYPERVISOR , +}; + +extern __ro_after_init enum srbds_mitigations srbds_mitigation;And this needs to be public because the only user is in
bugs.c, right?
right. this is now gone.
Quoted 57 lines
Quoted 44 lines
arch/x86/kernel/cpu/cpu.h +3
-0+void srbds_configure_mitigation (void ); +#ifdef CONFIG_CPU_SUP_INTEL +enum tsx_ctrl_states {TSX_CTRL_ENABLE , TSX_CTRL_DISABLE,arch/x86/kernel/cpu/intel.c +2
-0@@ -684,6 +684,8 @@ static void init_intel(struct cpuinfo_x86 *c)tsx_enable ();if (tsx_ctrl_state ==TSX_CTRL_DISABLE )tsx_disable (); + +srbds_configure_mitigation (); }#ifdef CONFIG_X86_32 arch/x86/kernel/cpu/match.c +15
-0@@ -49,6 +49,32 @@ const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match) }EXPORT_SYMBOL (x86_match_cpu); +/* +* Extend x86_match_cpu to support matching a range of steppings. +*/ +const struct x86_cpu_id_ext *x86_match_cpu_ext (const struct x86_cpu_id_ext *match) +{ +const struct x86_cpu_id_ext *m; +struct cpuinfo_x86 *c = &boot_cpu_data; + +for (m = match; m->id.vendor | m->id.family | m->id.model | m->id.feature; m++) { +if (m->id.vendor !=X86_VENDOR_ANY && c->x86_vendor != m->id.vendor) +continue ; +if (m->id.family !=X86_FAMILY_ANY && c->x86 != m->id.family) +continue ; +if (m->id.model !=X86_MODEL_ANY && c->x86_model != m->id.model) +continue ;arch/x86/kernel/cpu/match.c const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match) · line 76
Quoted 10 lines
arch/x86/kernel/cpu/match.c +10
-0+if (m->steppings !=X86_STEPPING_ANY && + !(BIT (c->x86_stepping) & m->steppings)) +continue ; +if (m->id.feature !=X86_FEATURE_ANY && !cpu_has (c, m->id.feature)) +continue ; +return m; + } +return NULL ; +} +EXPORT_SYMBOL (x86_match_cpu_ext);Sigh, aside of being pointless duplicated code:
Quoted 3 lines
If we'd really need this then it can share most of the code with
x86_match_cpu(), but copy and paste is more fancy, right? You even copied the export just in case ...
duplication is removed. function refactored.
Quoted 37 lines
Quoted 30 lines
arch/x86/kernel/cpu/match.c +0
-0static const struct x86_cpu_desc *x86_match_cpu_with_stepping (const struct x86_cpu_desc *match) {drivers/base/cpu.c +7
-0@@ -567,6 +567,12 @@ ssize_t __weak cpu_show_itlb_multihit(struct device *dev,return sprintf (buf,"Not affected\n" ); } +ssize_t __weak cpu_show_special_register_data_sampling (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"Not affected\n" ); +} + static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL); static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL); static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);@@ -575,6 +581,7 @@ static DEVICE_ATTR(l1tf, 0444, cpu_show_l1tf, NULL); static DEVICE_ATTR(mds, 0444, cpu_show_mds, NULL); static DEVICE_ATTR(tsx_async_abort, 0444, cpu_show_tsx_async_abort, NULL); static DEVICE_ATTR(itlb_multihit, 0444, cpu_show_itlb_multihit, NULL); +static DEVICE_ATTR(special_register_data_sampling, 0444, cpu_show_special_register_data_sampling, NULL);static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_meltdown.attr,drivers/base/cpu.c static struct attribute *cpu_root_vulnerabilities_attrs[] = { · line 595
Quoted 4 lines
drivers/base/cpu.c +1
-0@@ -585,6 +592,7 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_mds.attr, &dev_attr_tsx_async_abort.attr, &dev_attr_itlb_multihit.attr, + &dev_attr_special_register_data_sampling.attr,This still lacks an entry in:
Quoted 3 lines
Documentation/ABI/testing/sysfs-devices-system-cpuas requested by Greg several times.
Done.
I like your review feedback, its not ambiguous.
Thank you.
I should have an updated posting hopefully tomorrow. --mark
Re: [PATCH 2/2] v3 more sampling fun 2
In reply to mark grossDocumentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 4
Quoted 10 lines · speck for mark gross
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0@@ -0,0 +1,145 @@ +.. SPDX-License-Identifier: GPL-2.0 + +SRBDS - Special Register Buffer Data Sampling +======================================
I doubt that this builds w/o warnings. And right:
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:4: WARNING: Title underline too short.
SRBDS - Special Register Buffer Data Sampling
======================================^
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:56: WARNING: Title underline too short.
Attack scenarios
---------------^
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:76: WARNING: Unexpected indentation. Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:77: WARNING: Block quote ends without a blank line; unexpected unindent.
Sigh...
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 5
Quoted 1 line
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +1
-0+SRBDS is a hardware vulnerability that allows MDS techniques to infer values
lacks a link to the MDS documentation
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 9
Quoted 4 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+returned from special register accesses. Special register accesses are +accesses to off core registers. According to Intels evaluation, the special +register reads that have a security expectation of privacy are: +RDRAND, RDSEED and SGX EGETKEY.
And what
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 12
Quoted 3 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +3
-0+ +When RDRAND and RDSEED instructions are used the data is moved to the core +through the special register mechanism.
This sentence is not providing any new information and can be packed in the above where you mention RDRAND, RDSEED already. EGETKEY is no instruction?
Quoted 5 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+Affected processors +-------------------- +Core models (desktop, mobile, Xeon-E3) that implement RDRAND and/or RDSEED and +are vulnerable to MFBDS (Micro architectural Fill Buffer DataSampling) variant
s/to/to the/
Quoted 18 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +18
-0+of MDS (Micro architectural Data Sampling) or to TAA (TSX Asynchronous Abort) +when TSX is enabled, + + ============= ============ ======== + common name Family_Model Stepping + ============= ============ ======== + Ivybridge 06_3AH All + + Haswell 06_3CH All + Haswell_L 06_45H All + Haswell_G 06_46H All + + Broadwell_G 06_47H All + Broadwell 06_3DH All + + Skylake_L 06_4EH All + Skylake 06_5EH All +
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 10 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +9
-0+ Kabylake_L 06_8EH <=A + Kabylake_L 06_8EH 0xB only if TSX is enabled + Kabylake_L 06_8EH 0xC only if TSX is enabled + + Kabylake 06_9EH <=B + Kabylake 06_9EH 0xC only if TSX is enabled + Kabylake 06_9EH 0xD only if TSX is enabled + ============= ============ ======== + ============= ============ ===========================
ditto at the top of the table.
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 9 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +9
-0+ +Related CVEs +------------ + +The following CVE entry is related to this SRBDS issue: + + ============== ===== =================================================== + CVE-2020--0543 + ============== ===== ===================================================
Is the void filled at some point?
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 4 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+Attack scenarios +--------------- +An unprivileged user can extract returned values from RDRAND and RDSEED +executed on another core or sibling thread using MDS techniques.
Lacks EGETKEY again.
Quoted 11 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +11
-0+ + +Mitigtion mechanism +------------------- +Intel will release microcode updates that modify the RDRAND, RDSEED, and +EGETKEY instructions to overwrite secret special register data in the shared +staging buffer before the secret data can be accessed by another logical +processor. + +During execution of the RDRAND, RDSEED, or EGETKEY instruction, off-core +accesses from other logical processors will be delayed until the special
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 10 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +10
-0+register read is complete and the secret data in the shared staging buffer is +overwritten. + +This has three effects on performance: +1 RDRAND, RDSEED, or EGETKEY instruction have higher latency. +2 Executing RDRAND at the same time on multiple logical processors will be + serialized, resulting in an overall reduction in the maximum RDRAND bandwidth. +3 Executing RDRAND, RDSEED or EGETKEY will delay memory accesses from other + logical processors that miss their core caches, with an impact similar to + legacy locked cache-line-split accesses.
I seriously doubt that you ever looked at the build output of this.
Quoted 30 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +30
-0+ +Because of the performance impact of the mitigation, the microcode updates also +provide an opt-out mechanism (RNGDS_MITG_DIS) to disable the mitigation for +RDRAND and RDSEED instructions executed outside of Intel Software Guard +Extensions (Intel SGX) enclaves. On logical processors that disable the +mitigation using this opt-out mechanism, RDRAND and RDSEED do not take longer +to execute and do not impact performance of sibling logical processors memory +accesses. The opt-out mechanism does not affect Intel SGX enclaves (including +execution of RDRAND or RDSEED inside of an enclave, as well as EGETKEY +execution). + +IA32_MCU_OPT_CTRL MSR Definition +-------------------------------- +Along with the mitigation for this issue, Intel added a new thread-scope +IA32_MCU_OPT_CTRL MSR, (address 0x123). The presence of this MSR and +RNGDS_MITG_DIS (bit 0) is enumerated by CPUID.(EAX=07H,ECX=0).EDX[SRBDS_CTRL = +9]==1. This MSR is introduced through the microcode update. + +Setting IA32_MCU_OPT_CTRL[0] (RNGDS_MITG_DIS) to 1 for a logical processor +disables the mitigation for RDRAND and RDSEED executed outside of an Intel SGX +enclave on that logical processor. Opting out of the mitigation for a +particular logical processor does not affect the RDRAND and RDSEED mitigations +for other logical processors. + +Note that inside of an Intel SGX enclave, the mitigation is applied regardless +of the value of RNGDS_MITG_DS. + +Mitigation control on the kernel command line +--------------------------------------------- +The kernel command line allows for control over the SRBDS mitigation
Quoted 1 line
at boot
allows control (methinks)
Quoted 25 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +25
-0+time with the option "srbds=". The option for this is: + + ============= ============================================================ + off This option disables SRBS mitigation for RDRAND and RDSEED on + affected platforms. + ============= ============================================================ + +SRBS System Information +----------------------- +The Linux kernel provides vulnerability status information through sysfs. For +SRBDS this can be accessed by the following sysfs file: +/sys/devices/system/cpu/vulnerabilities/special_register_data_sampling + +The possible values contained in this file are: + + ============================== =========================================== + Not affected Processor not vulnerable + Vulnerable Processor vulnerable and mitigation disabled + Vulnerable: no microcode Processor vulnerable and microcode is missing + mitigation + Mitigated Processor is vulnerable and mitigation is in + effect. + Not affected (TSX disabled) Processor is only vulnerable when TSX is + enabled while this system was booted with TSX + disabled.
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 10 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +10
-0+ Unknown Running on virtual guest processor that is + affected but with no way to know if host + processor is mitigated or vulnerable. + ============================== =========================================== + +Default mitigations +------------------- +This new microcode serializes processor access during execution of RDRAND, +RDSEED ensures that the shared buffer is overwritten before it is released for +reuse.
Errm. What has this to do with the default chosen by the kernel?
Thanks,
tglx
[MODERATED] Re: [PATCH 2/2] v3 more sampling fun 2
Quoted 8 lines · speck for Thomas Gleixner
Quoted 7 lines
(quoted diff) +6
-0+Note that inside of an Intel SGX enclave, the mitigation is applied regardless +of the value of RNGDS_MITG_DS. + +Mitigation control on the kernel command line +--------------------------------------------- +The kernel command line allows for control over the SRBDS mitigationat boot
allows control (methinks)
Both are fine from a grammar point of view. It's simply down to preference.
~Andrew
[MODERATED] Re: [PATCH 2/2] v3 more sampling fun 2
In reply to Thomas GleixnerThanks for the valueble feedback. I'll go over it in more detail tomorrow. But, one thing I want to point out is that EGETKEY is always mitigate and only RDRAND and RDSEED can have the mitigation disabled.
This is the first time I've worked with the Documentation. I did build it but I only looked to see if it completed :(
--mark
Quoted 204 lines · speck for Thomas Gleixner
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 4
Quoted 10 lines · speck for mark gross
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0@@ -0,0 +1,145 @@ +.. SPDX-License-Identifier: GPL-2.0 + +SRBDS - Special Register Buffer Data Sampling +======================================I doubt that this builds w/o warnings. And right:
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:4: WARNING: Title underline too short.SRBDS - Special Register Buffer Data Sampling
======================================^
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:56: WARNING: Title underline too short.Attack scenarios
---------------^
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:76: WARNING: Unexpected indentation.Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:77: WARNING: Block quote ends without a blank line; unexpected unindent.Sigh...
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 5
Quoted 1 line
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +1
-0+SRBDS is a hardware vulnerability that allows MDS techniques to infer valueslacks a link to the MDS documentation
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 9
Quoted 4 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+returned from special register accesses. Special register accesses are +accesses to off core registers. According to Intels evaluation, the special +register reads that have a security expectation of privacy are: +RDRAND, RDSEED and SGX EGETKEY.And what
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 12
Quoted 3 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +3
-0+ +When RDRAND and RDSEED instructions are used the data is moved to the core +through the special register mechanism.This sentence is not providing any new information and can be packed in the above where you mention RDRAND, RDSEED already. EGETKEY is no instruction?
Quoted 5 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+Affected processors +-------------------- +Core models (desktop, mobile, Xeon-E3) that implement RDRAND and/or RDSEED and +are vulnerable to MFBDS (Micro architectural Fill Buffer DataSampling) variant
s/to/to the/
Quoted 18 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +18
-0+of MDS (Micro architectural Data Sampling) or to TAA (TSX Asynchronous Abort) +when TSX is enabled, + + ============= ============ ======== + common name Family_Model Stepping + ============= ============ ======== + Ivybridge 06_3AH All + + Haswell 06_3CH All + Haswell_L 06_45H All + Haswell_G 06_46H All + + Broadwell_G 06_47H All + Broadwell 06_3DH All + + Skylake_L 06_4EH All + Skylake 06_5EH All +Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 10 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +9
-0+ Kabylake_L 06_8EH <=A + Kabylake_L 06_8EH 0xB only if TSX is enabled + Kabylake_L 06_8EH 0xC only if TSX is enabled + + Kabylake 06_9EH <=B + Kabylake 06_9EH 0xC only if TSX is enabled + Kabylake 06_9EH 0xD only if TSX is enabled + ============= ============ ======== + ============= ============ ===========================ditto at the top of the table.
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 9 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +9
-0+ +Related CVEs +------------ + +The following CVE entry is related to this SRBDS issue: + + ============== ===== =================================================== + CVE-2020--0543 + ============== ===== ===================================================Is the void filled at some point?
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 4 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+Attack scenarios +--------------- +An unprivileged user can extract returned values from RDRAND and RDSEED +executed on another core or sibling thread using MDS techniques.Lacks EGETKEY again.
Quoted 11 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +11
-0+ + +Mitigtion mechanism +------------------- +Intel will release microcode updates that modify the RDRAND, RDSEED, and +EGETKEY instructions to overwrite secret special register data in the shared +staging buffer before the secret data can be accessed by another logical +processor. + +During execution of the RDRAND, RDSEED, or EGETKEY instruction, off-core +accesses from other logical processors will be delayed until the specialDocumentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 10 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +10
-0+register read is complete and the secret data in the shared staging buffer is +overwritten. + +This has three effects on performance: +1 RDRAND, RDSEED, or EGETKEY instruction have higher latency. +2 Executing RDRAND at the same time on multiple logical processors will be + serialized, resulting in an overall reduction in the maximum RDRAND bandwidth. +3 Executing RDRAND, RDSEED or EGETKEY will delay memory accesses from other + logical processors that miss their core caches, with an impact similar to + legacy locked cache-line-split accesses.I seriously doubt that you ever looked at the build output of this.
Quoted 30 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +30
-0+ +Because of the performance impact of the mitigation, the microcode updates also +provide an opt-out mechanism (RNGDS_MITG_DIS) to disable the mitigation for +RDRAND and RDSEED instructions executed outside of Intel Software Guard +Extensions (Intel SGX) enclaves. On logical processors that disable the +mitigation using this opt-out mechanism, RDRAND and RDSEED do not take longer +to execute and do not impact performance of sibling logical processors memory +accesses. The opt-out mechanism does not affect Intel SGX enclaves (including +execution of RDRAND or RDSEED inside of an enclave, as well as EGETKEY +execution). + +IA32_MCU_OPT_CTRL MSR Definition +-------------------------------- +Along with the mitigation for this issue, Intel added a new thread-scope +IA32_MCU_OPT_CTRL MSR, (address 0x123). The presence of this MSR and +RNGDS_MITG_DIS (bit 0) is enumerated by CPUID.(EAX=07H,ECX=0).EDX[SRBDS_CTRL = +9]==1. This MSR is introduced through the microcode update. + +Setting IA32_MCU_OPT_CTRL[0] (RNGDS_MITG_DIS) to 1 for a logical processor +disables the mitigation for RDRAND and RDSEED executed outside of an Intel SGX +enclave on that logical processor. Opting out of the mitigation for a +particular logical processor does not affect the RDRAND and RDSEED mitigations +for other logical processors. + +Note that inside of an Intel SGX enclave, the mitigation is applied regardless +of the value of RNGDS_MITG_DS. + +Mitigation control on the kernel command line +--------------------------------------------- +The kernel command line allows for control over the SRBDS mitigationQuoted 1 line
at boot
allows control (methinks)
Quoted 25 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +25
-0+time with the option "srbds=". The option for this is: + + ============= ============================================================ + off This option disables SRBS mitigation for RDRAND and RDSEED on + affected platforms. + ============= ============================================================ + +SRBS System Information +----------------------- +The Linux kernel provides vulnerability status information through sysfs. For +SRBDS this can be accessed by the following sysfs file: +/sys/devices/system/cpu/vulnerabilities/special_register_data_sampling + +The possible values contained in this file are: + + ============================== =========================================== + Not affected Processor not vulnerable + Vulnerable Processor vulnerable and mitigation disabled + Vulnerable: no microcode Processor vulnerable and microcode is missing + mitigation + Mitigated Processor is vulnerable and mitigation is in + effect. + Not affected (TSX disabled) Processor is only vulnerable when TSX is + enabled while this system was booted with TSX + disabled.Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 10 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +10
-0+ Unknown Running on virtual guest processor that is + affected but with no way to know if host + processor is mitigated or vulnerable. + ============================== =========================================== + +Default mitigations +------------------- +This new microcode serializes processor access during execution of RDRAND, +RDSEED ensures that the shared buffer is overwritten before it is released for +reuse.Errm. What has this to do with the default chosen by the kernel?
Thanks,
tglx
[MODERATED] Re: [PATCH 2/2] v3 more sampling fun 2
In reply to Thomas GleixnerQuoted 13 lines · speck for Thomas Gleixner
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 4
Quoted 10 lines · speck for mark gross
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0@@ -0,0 +1,145 @@ +.. SPDX-License-Identifier: GPL-2.0 + +SRBDS - Special Register Buffer Data Sampling +======================================I doubt that this builds w/o warnings. And right:
It does now.
Quoted 5 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:4: WARNING: Title underline too short.SRBDS - Special Register Buffer Data Sampling
======================================^
fixed.
Quoted 5 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:56: WARNING: Title underline too short.Attack scenarios
---------------^
fixed.
Quoted 3 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:76: WARNING: Unexpected indentation.Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst:77: WARNING: Block quote ends without a blank line; unexpected unindent.
fixed.
Quoted 6 lines
Sigh...
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 5
Quoted 1 line
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +1
-0+SRBDS is a hardware vulnerability that allows MDS techniques to infer valueslacks a link to the MDS documentation
will fix.
Quoted 8 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 9
Quoted 5 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+returned from special register accesses. Special register accesses are +accesses to off core registers. According to Intels evaluation, the special +register reads that have a security expectation of privacy are: +RDRAND, RDSEED and SGX EGETKEY.And what
Quoted 6 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst line 12
Quoted 3 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +3
-0+ +When RDRAND and RDSEED instructions are used the data is moved to the core +through the special register mechanism.This sentence is not providing any new information and can be packed in the above where you mention RDRAND, RDSEED already. EGETKEY is no instruction?
added " that is susceptible to MDS attacks."
Quoted 8 lines
Quoted 6 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+Affected processors +-------------------- +Core models (desktop, mobile, Xeon-E3) that implement RDRAND and/or RDSEED and +are vulnerable to MFBDS (Micro architectural Fill Buffer DataSampling) variant
s/to/to the/
done.
Quoted 28 lines
Quoted 27 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +26
-0+of MDS (Micro architectural Data Sampling) or to TAA (TSX Asynchronous Abort) +when TSX is enabled, + + ============= ============ ======== + common name Family_Model Stepping + ============= ============ ======== + Ivybridge 06_3AH All + + Haswell 06_3CH All + Haswell_L 06_45H All + Haswell_G 06_46H All + + Broadwell_G 06_47H All + Broadwell 06_3DH All + + Skylake_L 06_4EH All + Skylake 06_5EH All + + Kabylake_L 06_8EH <=A + Kabylake_L 06_8EH 0xB only if TSX is enabled + Kabylake_L 06_8EH 0xC only if TSX is enabled + + Kabylake 06_9EH <=B + Kabylake 06_9EH 0xC only if TSX is enabled + Kabylake 06_9EH 0xD only if TSX is enabled + ============= ============ ========
Quoted 3 lines
Quoted 1 line
+ ============= ============ ===========================
ditto at the top of the table.
I don't understand this feedback.
Quoted 12 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 10 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +9
-0+ +Related CVEs +------------ + +The following CVE entry is related to this SRBDS issue: + + ============== ===== =================================================== + CVE-2020--0543 + ============== ===== ===================================================Is the void filled at some point?
Working on it.
Quoted 7 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 5 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+Attack scenarios +--------------- +An unprivileged user can extract returned values from RDRAND and RDSEED +executed on another core or sibling thread using MDS techniques.Lacks EGETKEY again.
no, egetkey is not an instruction for use outside an SGX enclave.
Also, the mitigation only alows MSR control of the mitigation for RDRAND and RDSEED. EGETKEY is always mitigated.
Quoted 16 lines
Quoted 16 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +15
-0+ + +Mitigtion mechanism +------------------- +Intel will release microcode updates that modify the RDRAND, RDSEED, and +EGETKEY instructions to overwrite secret special register data in the shared +staging buffer before the secret data can be accessed by another logical +processor. + +During execution of the RDRAND, RDSEED, or EGETKEY instruction, off-core +accesses from other logical processors will be delayed until the special +register read is complete and the secret data in the shared staging buffer is +overwritten. + +This has three effects on performance:
Quoted 8 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 6 lines
+1 RDRAND, RDSEED, or EGETKEY instruction have higher latency. +2 Executing RDRAND at the same time on multiple logical processors will be
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +4
-0+ serialized, resulting in an overall reduction in the maximum RDRAND bandwidth. +3 Executing RDRAND, RDSEED or EGETKEY will delay memory accesses from other + logical processors that miss their core caches, with an impact similar to + legacy locked cache-line-split accesses.I seriously doubt that you ever looked at the build output of this.
I can see why you doubt I ever looked at the output. FWIW I used gitlab to view the *.rst formatting and I was focused on fighting the table below to the point I missed this section.
this list is fixed now.
Quoted 25 lines
Quoted 25 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +25
-0+ +Because of the performance impact of the mitigation, the microcode updates also +provide an opt-out mechanism (RNGDS_MITG_DIS) to disable the mitigation for +RDRAND and RDSEED instructions executed outside of Intel Software Guard +Extensions (Intel SGX) enclaves. On logical processors that disable the +mitigation using this opt-out mechanism, RDRAND and RDSEED do not take longer +to execute and do not impact performance of sibling logical processors memory +accesses. The opt-out mechanism does not affect Intel SGX enclaves (including +execution of RDRAND or RDSEED inside of an enclave, as well as EGETKEY +execution). + +IA32_MCU_OPT_CTRL MSR Definition +-------------------------------- +Along with the mitigation for this issue, Intel added a new thread-scope +IA32_MCU_OPT_CTRL MSR, (address 0x123). The presence of this MSR and +RNGDS_MITG_DIS (bit 0) is enumerated by CPUID.(EAX=07H,ECX=0).EDX[SRBDS_CTRL = +9]==1. This MSR is introduced through the microcode update. + +Setting IA32_MCU_OPT_CTRL[0] (RNGDS_MITG_DIS) to 1 for a logical processor +disables the mitigation for RDRAND and RDSEED executed outside of an Intel SGX +enclave on that logical processor. Opting out of the mitigation for a +particular logical processor does not affect the RDRAND and RDSEED mitigations +for other logical processors. + +Note that inside of an Intel SGX enclave, the mitigation is applied regardless
Quoted 8 lines
Quoted 6 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +5
-0+of the value of RNGDS_MITG_DS. + +Mitigation control on the kernel command line +--------------------------------------------- +The kernel command line allows for control over the SRBDS mitigationat boot
allows control (methinks)
ok.
Quoted 30 lines
Quoted 30 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +29
-0+time with the option "srbds=". The option for this is: + + ============= ============================================================ + off This option disables SRBS mitigation for RDRAND and RDSEED on + affected platforms. + ============= ============================================================ + +SRBS System Information +----------------------- +The Linux kernel provides vulnerability status information through sysfs. For +SRBDS this can be accessed by the following sysfs file: +/sys/devices/system/cpu/vulnerabilities/special_register_data_sampling + +The possible values contained in this file are: + + ============================== =========================================== + Not affected Processor not vulnerable + Vulnerable Processor vulnerable and mitigation disabled + Vulnerable: no microcode Processor vulnerable and microcode is missing + mitigation + Mitigated Processor is vulnerable and mitigation is in + effect. + Not affected (TSX disabled) Processor is only vulnerable when TSX is + enabled while this system was booted with TSX + disabled. + Unknown Running on virtual guest processor that is + affected but with no way to know if host + processor is mitigated or vulnerable. + ============================== ===========================================
Quoted 8 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Quoted 6 lines
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +6
-0+ +Default mitigations +------------------- +This new microcode serializes processor access during execution of RDRAND, +RDSEED ensures that the shared buffer is overwritten before it is released for +reuse.Errm. What has this to do with the default chosen by the kernel?
its a statement that if the kernel does nothing then the mitigation is in effect.
thanks for the feedback. Please respond about the comment I didn't understand.
--mark
Re: [PATCH 2/2] v3 more sampling fun 2
Mark,
Quoted 26 lines · speck for mark gross
Quoted 25 lines · speck for Thomas Gleixner
Quoted 23 lines · speck for mark gross
(quoted diff) +23
-0+ ============= ============ ======== + common name Family_Model Stepping + ============= ============ ======== + Ivybridge 06_3AH All + + Haswell 06_3CH All + Haswell_L 06_45H All + Haswell_G 06_46H All + + Broadwell_G 06_47H All + Broadwell 06_3DH All + + Skylake_L 06_4EH All + Skylake 06_5EH All + + Kabylake_L 06_8EH <=A + Kabylake_L 06_8EH 0xB only if TSX is enabled + Kabylake_L 06_8EH 0xC only if TSX is enabled + + Kabylake 06_9EH <=B + Kabylake 06_9EH 0xC only if TSX is enabled + Kabylake 06_9EH 0xD only if TSX is enabled + ============= ============ ========
Quoted 4 lines
Quoted 3 lines
Quoted 1 line
+ ============= ============ ===========================
ditto at the top of the table.
I don't understand this feedback.
The length of '===' must be at least as long as the text in the rows. At least that was a requirement in the past. Maybe it got relaxed, but even then it looks more consistent:
============= ============ ======== Kabylake 06_9EH 0xD only if TSX is enabled ============= ============ ========
vs.
============= ============ =========================== Kabylake 06_9EH 0xD only if TSX is enabled ============= ============ ===========================
Quoted 10 lines
Quoted 6 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+Attack scenarios +--------------- +An unprivileged user can extract returned values from RDRAND and RDSEED +executed on another core or sibling thread using MDS techniques.Lacks EGETKEY again.
no, egetkey is not an instruction for use outside an SGX enclave.
Also, the mitigation only alows MSR control of the mitigation for RDRAND and RDSEED. EGETKEY is always mitigated.
This is talking about attack scenarios not about mitigations.
Quoted 9 lines
Quoted 7 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+Default mitigations +------------------- +This new microcode serializes processor access during execution of RDRAND, +RDSEED ensures that the shared buffer is overwritten before it is released for +reuse.Errm. What has this to do with the default chosen by the kernel?
its a statement that if the kernel does nothing then the mitigation is in effect.
Then please write it in a way which makes it clear what the kernel does by default.
If updated microcode is available, the microcode mitigation which overwrites the shared buffer is enabled by default.
Or something to that effect.
Thanks,
tglx
[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
In reply to mark grossReview
Quoted 13 lines · speck for mark gross
(quoted diff) +13
-0+static void __init srbds_select_mitigation(void) +{ + u64 ia32_cap; + + if (!boot_cpu_has_bug(X86_BUG_SRBDS)) { + srbds_mitigation = SRBDS_NOT_AFFECTED; + return; + } + + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) { + srbds_mitigation = SRBDS_HYPERVISOR; + return; + }
These two ought to be reversing (and with a suitable adjustment to the docs in patch 2).
If you're running as a guest, you can't even trust the model number used to divine X86_BUG_SRBDS in the first place.
~Andrew
[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
Quoted 18 lines · speck for Andrew Cooper
Review
Quoted 13 lines · speck for mark gross
(quoted diff) +13
-0+static void __init srbds_select_mitigation(void) +{ + u64 ia32_cap; + + if (!boot_cpu_has_bug(X86_BUG_SRBDS)) { + srbds_mitigation = SRBDS_NOT_AFFECTED; + return; + } + + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) { + srbds_mitigation = SRBDS_HYPERVISOR; + return; + }These two ought to be reversing (and with a suitable adjustment to the docs in patch 2).
Quoted 3 lines
If you're running as a guest, you can't even trust the model number used to divine
X86_BUG_SRBDSin the first place.
I'll change this for the next version.
--mark
[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
Quoted 19 lines · speck for mark gross
Quoted 18 lines · speck for Andrew Cooper
Review
Quoted 13 lines · speck for mark gross
(quoted diff) +13
-0+static void __init srbds_select_mitigation(void) +{ + u64 ia32_cap; + + if (!boot_cpu_has_bug(X86_BUG_SRBDS)) { + srbds_mitigation = SRBDS_NOT_AFFECTED; + return; + } + + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) { + srbds_mitigation = SRBDS_HYPERVISOR; + return; + }These two ought to be reversing (and with a suitable adjustment to the docs in patch 2).
Quoted 5 lines
Quoted 3 lines
If you're running as a guest, you can't even trust the model number used to divine
X86_BUG_SRBDSin the first place.I'll change this for the next version.
Andrew: Are you really sure that you want that? It would mean that all guests running on any server level Xeon would report mitigation status as unknown. Actually all those server Xeons are not affected by SRBDS.
Technically that is entirely the right thing to do. But how much do hypervisors mess with the CPU model between different classes on processors in practice? I thought that commonly people put a bunch of Haswell/Broadwell/Skylakes into a "pool" and pretended they were all Haswell.
Do you want to consider the tradeoff between being absolutely accurate against all the support calls you will get because guests report "unknown" mitigation status?
-Tony
[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
Quoted 19 lines · Tony
Quoted 18 lines · speck for mark gross
Quoted 17 lines · speck for Andrew Cooper
Review
Quoted 13 lines · speck for mark gross
(quoted diff) +13
-0+static void __init srbds_select_mitigation(void) +{ + u64 ia32_cap; + + if (!boot_cpu_has_bug(X86_BUG_SRBDS)) { + srbds_mitigation = SRBDS_NOT_AFFECTED; + return; + } + + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) { + srbds_mitigation = SRBDS_HYPERVISOR; + return; + }These two ought to be reversing (and with a suitable adjustment to the docs in patch 2).
Quoted 5 lines
Quoted 4 lines
Quoted 3 lines
If you're running as a guest, you can't even trust the model number used to divine
X86_BUG_SRBDSin the first place.I'll change this for the next version.
Andrew: Are you really sure that you want that?
I thought I was...
Quoted 4 lines
It would
mean that all guests running on any server level Xeon would report mitigation status as unknown. Actually all those server Xeons are not affected by SRBDS.
There are Xeon E3's to be found, in server form factors, in clouds.
Quoted 5 lines
Technically that is entirely the right thing to do. But how much do hypervisors mess with the CPU model between different classes on processors in practice? I thought that commonly people put a bunch of Haswell/Broadwell/Skylakes into a "pool" and pretended they were all Haswell.
I actually made this point for KVM's benefit, which does (to the best of my knowledge) behave like that.
With Xen, a VM sees the real F/M/S of the processor it first started on, even if the reported features is way off from what a native version of that CPU would look like.
I can't speak to other hypervisors, but it would be a reasonable question to ask the keybase group.
Quoted 4 lines
Do you want to consider the tradeoff between being absolutely accurate against all the support calls you will get because guests report "unknown" mitigation status?
vs the warm, fuzzy, and sinking feeling where it says safe, but isn't actually?
Also, consider the case of a mix of E3's and E5's in a single migration pool, which is disappointingly common at least at the enterprise level.
I accept this is a complicated problem, but personally, I view providing potentially-wrong information worse than saying "ask the person who runs your hypervisor".
Particularly with this issue where it seems that no hypervisor is interested in offering the knob to guest kernels (so could at least infer based on SRBDS_CTRL), nor is there an ARCH_CAPS_$FOO_NO bit which could be filled in by a hypervisor on unaffected or mitigated hardware.
After all, the question of "are my random numbers known elsewhere?" also depends on "is my hypervisor intercepting the RDRAND/RDSEED instructions?" and possibly "has someone set the DETERMINISTIC_RNG bit behind my back?".
~Andrew
[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
Quoted 4 lines · speck for Andrew Cooper
Particularly with this issue where it seems that no hypervisor is interested in offering the knob to guest kernels (so could at least infer based on
SRBDS_CTRL), nor is there an ARCH_CAPS_$FOO_NO bit which could be filled in by a hypervisor on unaffected or mitigated hardware.
Maybe that is the solution? We allocated a s/w (VMM) bit in ARCH_CAPABILITIES for the TLB DoS issue for the nested hypervisor case. Should we do that again for this issue?
If you think so, then we can take this discussion to Keybase so that the other VMM vendors who hang out there can chime in.
[Note that it isn't my call to allocate a bit in ARCH_CAPABILITIES, but I can campaign for it if folks want it].
-Tony
[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
Quoted 9 lines · Tony
Quoted 4 lines · speck for Andrew Cooper
Particularly with this issue where it seems that no hypervisor is interested in offering the knob to guest kernels (so could at least infer based on
SRBDS_CTRL), nor is there an ARCH_CAPS_$FOO_NO bit which could be filled in by a hypervisor on unaffected or mitigated hardware.Maybe that is the solution? We allocated a s/w (VMM) bit in
ARCH_CAPABILITIESfor the TLB DoS issue for the nested hypervisor case. Should we do that again for this issue?
Bah, memory fail. I was actually thinking of bit 3 SKIP_L1DFL_VMENTRY that told a nested VMM it didn't need to flush L1D again for L1TF as an outer VMM was already handling that.
Quoted 7 lines
If you think so, then we can take this discussion to Keybase so that the other VMM vendors who hang out there can chime in.
[Note that it isn't my call to allocate a bit in
ARCH_CAPABILITIES, but I can campaign for it if folks want it].-Tony
[MODERATED] Re: [PATCH 1/2] v3 more sampling fun 1
Quoted 6 lines · Tony
Quoted 5 lines · Tony
Quoted 4 lines · speck for Andrew Cooper
Particularly with this issue where it seems that no hypervisor is interested in offering the knob to guest kernels (so could at least infer based on
SRBDS_CTRL), nor is there an ARCH_CAPS_$FOO_NO bit which could be filled in by a hypervisor on unaffected or mitigated hardware.
Quoted 12 lines
Quoted 3 lines
Maybe that is the solution? We allocated a s/w (VMM) bit in
ARCH_CAPABILITIESfor the TLB DoS issue for the nested hypervisor case. Should we do that again for this issue?Bah, memory fail. I was actually thinking of bit 3
SKIP_L1DFL_VMENTRYthat told a nested VMM it didn't need to flush L1D again for L1TF as an outer VMM was already handling that.Quoted 5 lines
If you think so, then we can take this discussion to Keybase so that the other VMM vendors who hang out there can chime in.
[Note that it isn't my call to allocate a bit in
ARCH_CAPABILITIES, but I can campaign for it if folks want it].
I think it would certainly be worth while asking.
~Andrew