AMD errata 793 (CVE-2013-6885) needs a workaround in Linux?

I just got this assigned to amd64-microcode in Debian, but it is something that needs to be worked around by the EFI/BIOS and/or the kernel.

Since we all know just how well it pans out to depend on BIOS/EFI updates for *anything*, I'm raising the issue here. IMHO looks like it would be worthwhile to either set the relevant MSR in the kernel if the BIOS didn't do it, or at least warn the user of the need for a BIOS/EFI update...

It is the usual hangs-core type of CPU errata (therefore, the best type since it won't cause silent data corruption). gcc-produced code managed to trigger it (in DragonFly BSD).

A quick search under arch/x86 did not locate any existing workaround for this issue.

Date: Wed, 27 Nov 2013 21:23:37 -0500 (EST)
From: [email protected]
To: [email protected]
Cc: [email protected]
Subject: CVE-2013-6885 AMD Publ. 51810 Errata 793 system hang

The person who requested CVE-2013-6885 asked that we send the CVE assignment here because various open-source software will probably be adding code to prevent this denial of service attack.

http://support.amd.com/TechDocs/51810_16h_00h-0Fh_Rev_Guide.pdf
http://lists.dragonflybsd.org/pipermail/kernel/2011-December/046594.html
http://www.zdnet.com/blog/hardware/amd-owns-up-to-cpu-bug/18924

793 Specific Combination of Writes to Write Combined Memory Types and Locked Instructions May Cause Core Hang

Under a highly specific and detailed set of internal timing conditions, a locked instruction may trigger a timing sequence whereby the write to a write combined memory type is not flushed, causing the locked instruction to stall indefinitely.

Potential Effect on System
Processor core hang.

Suggested Workaround
BIOS should set MSRC001_1020[15] = 1b.

No fix planned

- --
CVE assignment team, MITRE CVE Numbering Authority M/S M300
202 Burlington Road, Bedford, MA 01730 USA

  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

I think this is a different issue than the dragonfly issue. In any case, if AMD delivers all BIOS with this workaround enabled, we don't need to do anything. And I asked them about it so we'll have an answer soon, I hope.

In any case, I'm on it.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Seriously, though, if this MSR can be set at runtime without problems (which covers 98% of all workarounds, but not 100%) then it seems like a no-brainer to just do it as long as the offending CPUs can be identified by the kernel.

-hpa

Yeah, ok, let me cook it up.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

From: Borislav Petkov <[email protected]>

This adds the workaround for erratum 793 as a precaution in case not every BIOS implements it. This addresses CVE-2013-6885.

---
 arch/x86/kernel/cpu/amd.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

index 4a48e8bbd857..e4d6f8c91f51 100644
@@ -507,6 +507,17 @@ static void early_init_amd(struct cpuinfo_x86 *c)
 			set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
 	}
 #endif
+
+#define MSR_AMD_LS_CFG	0xc0011020
+	/* F16h, models 00h-0fh, erratum 793 */
+	if (c->x86 == 0x16 && c->x86_model <= 0xf) {
+		u64 val;
+
+		rdmsrl(MSR_AMD_LS_CFG, val);
+		if (!(val & BIT(15)))
+			wrmsrl(MSR_AMD_LS_CFG, val | BIT(15));
+	}
+
 }
 
 static const int amd_erratum_383[];

1.8.5.2.192.g7794a68

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Yeah, can we agree on some kind of a rule for msr-index.h? Something like the rule for pci.h, for example?

My only wish is to have all the MSRs at one place so one can search them fast. This has been the only thing that I wanted to have in the past, from fiddling with MSRs for so long.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

That is what msr-index.h is supposed to be.

-hpa

From: Borislav Petkov <[email protected]>

This adds the workaround for erratum 793 as a precaution in case not every BIOS implements it. This addresses CVE-2013-6885.

---
 arch/x86/include/uapi/asm/msr-index.h |  1 +
 arch/x86/kernel/cpu/amd.c             | 10 ++++++++++
 2 files changed, 11 insertions(+)

index 37813b5ddc37..59cea185ad1d 100644
@@ -184,6 +184,7 @@
 #define MSR_AMD64_PATCH_LOADER		0xc0010020
 #define MSR_AMD64_OSVW_ID_LENGTH	0xc0010140
 #define MSR_AMD64_OSVW_STATUS		0xc0010141
+#define MSR_AMD64_LS_CFG		0xc0011020
 #define MSR_AMD64_DC_CFG		0xc0011022
 #define MSR_AMD64_BU_CFG2		0xc001102a
 #define MSR_AMD64_IBSFETCHCTL		0xc0011030

index 4a48e8bbd857..a4bec9cd293a 100644
@@ -507,6 +507,16 @@ static void early_init_amd(struct cpuinfo_x86 *c)
 			set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
 	}
 #endif
+
+	/* F16h erratum 793, CVE-2013-6885 */
+	if (c->x86 == 0x16 && c->x86_model <= 0xf) {
+		u64 val;
+
+		rdmsrl(MSR_AMD64_LS_CFG, val);
+		if (!(val & BIT(15)))
+			wrmsrl(MSR_AMD_LS_CFG, val | BIT(15));
+	}
+
 }
 
 static const int amd_erratum_383[];

1.8.5.2.192.g7794a68

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Finnish!

Well, Boris the dumbass forgot to amend, changes were still in the index but not committed. This is what I get when I do a couple of things at the same time. :(

---
From: Borislav Petkov <[email protected]>

This adds the workaround for erratum 793 as a precaution in case not every BIOS implements it. This addresses CVE-2013-6885.

---
 arch/x86/include/uapi/asm/msr-index.h |  1 +
 arch/x86/kernel/cpu/amd.c             | 10 ++++++++++
 2 files changed, 11 insertions(+)

index 37813b5ddc37..59cea185ad1d 100644
@@ -184,6 +184,7 @@
 #define MSR_AMD64_PATCH_LOADER		0xc0010020
 #define MSR_AMD64_OSVW_ID_LENGTH	0xc0010140
 #define MSR_AMD64_OSVW_STATUS		0xc0010141
+#define MSR_AMD64_LS_CFG		0xc0011020
 #define MSR_AMD64_DC_CFG		0xc0011022
 #define MSR_AMD64_BU_CFG2		0xc001102a
 #define MSR_AMD64_IBSFETCHCTL		0xc0011030

index 4a48e8bbd857..6ac3a67ad471 100644
@@ -507,6 +507,16 @@ static void early_init_amd(struct cpuinfo_x86 *c)
 			set_cpu_cap(c, X86_FEATURE_EXTD_APICID);
 	}
 #endif
+
+	/* F16h erratum 793, CVE-2013-6885 */
+	if (c->x86 == 0x16 && c->x86_model <= 0xf) {
+		u64 val;
+
+		rdmsrl(MSR_AMD64_LS_CFG, val);
+		if (!(val & BIT(15)))
+			wrmsrl(MSR_AMD64_LS_CFG, val | BIT(15));
+	}
+
 }
 
 static const int amd_erratum_383[];

1.8.5.2.192.g7794a68


-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

I'd suggest the introduction of a standard set of methods operating on MSRs:

	msr_read()
	msr_write()
	msr_set_bit()
	msr_clear_bit()
	msr_set_mask()
	msr_clear_mask()

etc.

msr_read() would essentially map to rdmsr_safe(). Each method has a return value that can be checked for failure.

Note that the naming of 'msr_set_bit()' and 'msr_clear_bit()' mirrors that of bitops, and set_mask/clear_mask is named along a similar pattern, so that it's more immediately obvious what's going on.

With such methods in place we could use them in most new code, and would use 'raw, unsafe' rdmsr()/wrmsr() only in very specific, justified cases.

Thanks,

Ingo

I'd prefer the _bit() variant because it is easy to use in all those set-chicken-bit cases.

I'm not sure we want to use the _safe() variants by default as it would generate the exception tables even in cases where they're clearly not needed.

Yes, I completely agree - this is something I will do after the merge window.

The question about the need for the _mask() variants will be best answered after going over the sources and checking whether there actually is a need for setting more than one bit in an MSR.

Thanks.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

It would be particularly silly if what you end up with is in effect to wrap msr_read/write() in a BUG_ON(), which is the effect of the current (trapping) form. There is something to be said for hard errors.

-hpa

I don't think those new methods should be inline functions - thus there will be only one exception entry for each.

Right, the fact that most of our MSR accesses today are crash-on-failure, which happens to trigger crashes on a regular schedule, where most of the crashes are 'harmless' situation except that they crash the systems for good.

So I think defaulting to soft failures is the right approach.

Thanks,

Ingo

Well, as I said, as long as people don't start doing:

	BUG_ON(msr_read(...))

... 'cuz that's just silly. Also, on 64 bits the code generated by going from 2x32 bits to 1x64 and sometimes back can add up (on 32 bits there is no difference.)

-hpa

Before you go and do that, just ask internally whether those workarounds are being delivered with AGESA instead. We don't want to have a fix in the kernel for *every* erratum if it can be helped.

We're making an exception for this one because it can freeze the core just by executing a locked insn which can be done even in userspace.

For future AMD patches, make sure to almost always use tip master, i.e.

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git#master

to base your work off of.

Thanks, much appreciated. I'll add your Tested-by.

Yeah, I don't. So I'm relying on you for future patches :-)

Thanks.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Ok, So looks like workaround for E776 went in early, so it's probably safe to assume all BIOSes will have the fix. The workaround for E792 was checked in this month and *will* be picked up by BIOS later..

We might want to still have a software fix for this just in case someone uses older BIOSes..

I shall spin a patch on top of tip and send it out soon

Thanks,
-Aravind.

Are you saying there are F16h BIOSes out there which might not have the fix?

Also, for PCI config space fixes, take a look at
arch/x86/kernel/quirks.c

Also, code in arch/x86/kernel/amd_nb.c could come in handy, especially if you have multiple F16h sockets configurations.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

There is no "just in case" when it comes to someone using outdated firmware. It is a *given*, except for hardware that is only used in HPC and multiple (> 2) socket servers/workstations (which might be the case here, I don't know what processors we're talking about).

The vast majority of PC users will never update their firmware, unless the update is automatically offered to them, and sometimes not even in that case.

  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Yes, the real question is: did the broken BIOS ship in production systems? We don't care about engineering samples/beta boxes, but once it is sold as a product, it is real.

-hpa

We also cannot carry *every* erratum workaround in the kernel just because people don't update firmware. Firmware is becoming ubiquitous, sadly, and because of that, admins should provision for firmware upgrades too.

Besides, *even* if we put *all* errata fixes in the kernel, you'd need to update it anyway and reboot. In this case, you can just as well update your firmware instead, which involves that same reboot.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Actually I by and large disagree with that. There is a limit, of course, but when it comes to flipping an MSR in init code, the bar is pretty darn low. We have quirks for all kind of hardware, and this is just another example.

What *is* important, though, is that the workaround is well commented so that when someone comes and wonders "WTF is this, and what constraints does it have on it" they can get back to the primary sources (errata documents, mailing list discussions, CVEs, etc.) without undue effort.

The effort of a kernel update is much lower, especially since the kernel is generally automatically updated. It would be awesome if that was done for firmware, but in the absence of central distribution, arbitrary EOL sunsets, and a standard OS-driven firmware installer, it just isn't going to happen widely. Yes, that is a problem.

-hpa

No, you don't. :-)

You would much prefer to have the workaround done in the BIOS and only if there's a coverage hole, only then to do it in the kernel.

I'm not saying we shouldn't do it in the kernel per se - I'm saying we should do it only when really necessary. And it doesn't hurt to talk about it first before inviting in all fixes for all errata for all families of all vendors. No, you don't want that, believe me. :-)

Does that even matter? I think what matters is whether we reboot or not, i.e. HA crap. If we have to reboot, we might just as well flash the BIOS
- it takes almost as long.

Oh, the brave new world of UEFI wants to address that - it is supposed to flash the firmware from the OS. OEM vendors have their home grown solutions already, as I'm sure you know.

That definitely is a problem.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Right, so the chip goes into multiple platforms (server,desktop..) and this Erratum affects all platforms the chip fits in.. Also, the problem is that we can't be certain how many systems in the field carry BIOSes with the fix for this. Therefore, it is better to insulate ourselves than trust BIOS to do the right thing..

True, but isn't the case that it's more *likely* for sys admins to resort to updating kernels than program new BIOS? The alternative would be to update microcode, but there is no microcode patch file for this family yet on linux-firmware..

I've got a patch worked up for this anyway; Sending it as separate mail..

Thanks,
Aravind.

Hence the question: did an unfixed BIOS ever make it into a shipping production system? Because if it did, we do have a coverage hole.

-hpa

Right, and the answer to your question is: there is a *chance* it did.. From internal discussions, I could gather that the fix went into BIOS code earlier for desktop/mobile versions than it did for server versions;

So there is a chance production systems (desktop ones) might never see the issue, but server ones will, as BIOSes spun only from now onwards will have the workaround..

-Aravind.

This is exactly what I mean: I would like to have this question answered before any erratum fix goes into the kernel.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

No because people come and start bitching about their dmesg containing a warning and whether their hardware is b0rked without even reading the actual words.

Well, if the problem happens, the machine will lock. If it doesn't lock => no problem and we apply the workaround => no problem at all. :-)

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Printing a warning is appropriate if we can't actually fix the problem in the OS. If we actually make the problem go away then we have just done our job and we can be done with it.

-hpa

And we do make it go away so no warning and a gold star for us!

:-)

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Have you checked your dmesg recently? Normal people don't read it... it is just too much of it.

I disagree. Older kernel versions still may have problem, etc.

We normally do print warnings for problems we work around. We want vendors to fix their hardware, too...

ACPI BIOS Warning (bug): 32/64X FACS address mismatch in FADT - 0xBDB5FF40/0x00000000BDB64F40, using 32 (20131115/tbfadt-522) [Firmware Bug]: ACPI: BIOS _OSI(Linux) query ignored

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

You say people don't read their dmesg...
... because there is too much ...
... so let's add more?

What am I missing here?

-hpa

I'd say that proposed "your bios has a bug" is way more useful than stuff such as:

pci_bus 0000:00: root bus resource [bus 00-ff]
pci_bus 0000:00: root bus resource [io 0x0000-0x0cf7] pci_bus 0000:00: root bus resource [io 0x0d00-0xffff] pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000bffff] pci_bus 0000:00: root bus resource [mem 0xc0000000-0xffffffff] pci 0000:00:00.0: [8086:2e30] type 00 class 0x060000 pci 0000:00:01.0: [8086:2e31] type 01 class 0x060400 pci 0000:00:01.0: PME# supported from D0 D3hot D3cold pci 0000:00:01.0: System wakeup disabled by ACPI
pci 0000:00:02.0: [8086:2e32] type 00 class 0x030000 pci 0000:00:02.0: reg 0x10: [mem 0xd0000000-0xd03fffff 64bit] pci 0000:00:02.0: reg 0x18: [mem 0xc0000000-0xcfffffff 64bit pref] pci 0000:00:02.0: reg 0x20: [io 0xf140-0xf147]
pci 0000:00:02.1: [8086:2e33] type 00 class 0x038000 pci 0000:00:02.1: reg 0x10: [mem 0xd0400000-0xd04fffff 64bit] pci 0000:00:1b.0: [8086:27d8] type 00 class 0x040300 pci 0000:00:1b.0: reg 0x10: [mem 0xd0600000-0xd0603fff 64bit] pci 0000:00:1b.0: PME# supported from D0 D3hot D3cold pci 0000:00:1c.0: [8086:27d0] type 01 class 0x060400 pci 0000:00:1c.0: PME# supported from D0 D3hot D3cold pci 0000:00:1c.0: System wakeup disabled by ACPI
pci 0000:00:1c.1: [8086:27d2] type 01 class 0x060400 pci 0000:00:1c.1: PME# supported from D0 D3hot D3cold pci 0000:00:1c.1: System wakeup disabled by ACPI
pci 0000:00:1d.0: [8086:27c8] type 00 class 0x0c0300 pci 0000:00:1d.0: reg 0x20: [io 0xf080-0xf09f]
pci 0000:00:1d.0: System wakeup disabled by ACPI

So yes. Lets add useful stuff.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

You can always unconditionally print "your bios has a bug". Just like that. Without even looking.

So no, I don't think adding a warning for this one case makes sense.

If you want to add a warning yourself, you can put it in rc.local using userspace goodies like msr-tools. With them you can toggle that bit ad absurdum - no need for the kernel except msr.ko.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Yeah, and we have linux-firmware testing toolkits, to discourage buggy bioses. In this case it is "your bios has rare and dangerous bug, see CVS-1234".

?? By the time userspace is booted, you have overwritten the MSR, and information, if the BIOS is buggy, was lost.

So linux-firmware testing toolkit actually has no chance to report this one.

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Bullshit - if you don't apply the workaround, the bit in the MSR remains unset.

Drop this senseless conversation and go play with something else. At least I am.

Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

If you do not apply the workaround, you have the buggy system that may or may not boot. There's even CVE for it.

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html