Alexander Couzens has posted comments on this change. ( https://review.coreboot.org/19498 )
Change subject: acpi: fix FADT header version for ChromeOS devices
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/19498
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I689d2f848f4b8e5750742ea07f31162ee36ff64d
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: No
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/19498 )
Change subject: acpi: fix FADT header version for ChromeOS devices
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/19498
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I689d2f848f4b8e5750742ea07f31162ee36ff64d
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: No
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/19242 )
Change subject: Documentation: Add technote/design doc for mitigating ReBAR issue
......................................................................
Documentation: Add technote/design doc for mitigating ReBAR issue
Change-Id: Icba9d7910dfd46f32a2c46b6fd064a9cc8e3beac
Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org>
Reviewed-on: https://review.coreboot.org/19242
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
---
A Documentation/technotes/2017-02-dealing-with-untrusted-input-in-smm.md
1 file changed, 136 insertions(+), 0 deletions(-)
Approvals:
Stefan Reinauer: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/Documentation/technotes/2017-02-dealing-with-untrusted-input-in-smm.md b/Documentation/technotes/2017-02-dealing-with-untrusted-input-in-smm.md
new file mode 100644
index 0000000..ce1a097
--- /dev/null
+++ b/Documentation/technotes/2017-02-dealing-with-untrusted-input-in-smm.md
@@ -0,0 +1,136 @@
+Dealing with Untrusted Input in SMM
+===================================
+
+Objective
+---------
+Intel Security recently held a talk and published
+[slides](http://www.intelsecurity.com/advanced-threat-research/content/data/REConBrussels2017_BARing_the_system.pdf)
+on a vulnerability in SMM handlers on x86 systems. They provide examples
+on how both UEFI and coreboot are affected.
+
+Background
+----------
+SMM, the System Management Mode, is a CPU mode that is configured by
+firmware and survives the system’s initialization phase. On certain
+events that mode can be triggered and executes code, suspending the
+current processing that is going on the CPU, no matter whether it’s
+in kernel or user space.
+
+In SMM, the CPU has access to memory dedicated to that mode (SMRAM) that
+is normally inaccessible, and typically some restrictions are lifted as
+well (eg. in some configurations, certain flash write protection registers
+are writable in SMM only). This makes SMM a target for attacks which
+seek to elevate a ring0 (kernel) exploit to something permanent.
+
+Overview
+--------
+Intel Security showed several places in coreboot’s SMM handler (Slides
+32+) that could be manipulated into writing data at user-chosen addresses
+(SMRAM or otherwise), by modifying the BAR (Base Address Register) on
+certain devices. By picking the right addresses and the right events
+(and with them, mutators on the data at these addresses), it might
+be possible to change the SMM handler itself to call into regular RAM
+(where other code resides that then can work with elevated privileges).
+
+Their proposed mitigations (Slide 37) revolve around making sure
+that the BAR entries are reasonable, and point to a device instead of
+regular memory or SMRAM. They’re not very detailed on how this could
+be implemented, which is what this document discusses.
+
+Detailed Design
+---------------
+The attack works because the SMM handler trusts the results of the
+`pci_read_config32(dev, reg)` function, even though the value read by that
+function can be modified in kernel mode.
+
+In the general case it’s not possible to keep the cached value from
+system initialization because there are legitimate modifications the
+kernel can do to these values, so the only remedy is to make sure that
+the value isn’t totally off.
+
+For applications where hardware changes are limited by design (eg. no
+user-modifiable PCIe slots) and where the running kernel is known,
+such as Chromebooks, further efforts include caching the BAR settings
+at initialization time and comparing later accesses to that.
+
+What "totally off" means is chipset specific because it requires
+knowledge of the memory map as seen by the memory controller: which
+addresses are routed to devices, which are handled by the memory
+controller itself?
+The proposal is that in SMM, the `pci_read_config` functions (which
+aren’t timing critical) _always_ validate the value read from a given
+set of registers (the BARs) and fail hard (ie. cold reset, potentially
+after logging the event) if they’re invalid (because that points to
+a severe kernel bug or an attack).
+The actual validation is done by a function implemented by the chipset code.
+
+Another validation that can be done is to make sure that the BAR has the
+appropriate bits set so it is enabled and points to memory (instead of
+IO space).
+
+In terms of implementation, this might look somewhat as follows. There
+are a bunch of blanks to fill in, in particular how to handle the actual
+config space access and there will be more registers that need to be
+checked for correctness, both official BARs (0-4) and per-chipset
+registers that need to be blacklisted in another chipset specific
+function:
+
+```c
+static inline __attribute__((always_inline))
+uint32_t pci_read_config32[d](pci_devfn_t dev, unsigned int where)
+{
+ uint32_t val = real_pci_read_config32(dev, where);
+ if (IS_ENABLED(__SMM__) && (where == PCI_BASE_ADDRESS_0) &&
+ is_mmio_ptr(dev, where) && !is_address_in_mmio(val)) {
+ cold_reset();
+ }
+ return val;
+}
+```
+
+`is_address_in_mmio(addr)` would be a newly introduced function to be
+implemented by chipset drivers that returns true if the passed address
+points into whatever is considered valid MMIO space.
+`is_mmio_ptr(dev, where)` returns true for PCI config space registers that
+point to BARs (allowing custom overrides because sometimes additional
+registers are used to point to addresses).
+
+For this function what is considered a legal address needs to be
+documented, in accordance with the chipset design. (For example: AMD
+K8 has a bunch of registers that define strictly which addresses are
+"MMIO")
+
+### Fully insured (aka “paranoid”) mode
+For systems with more control over the hardware and kernel (such as
+Chromebooks), it may be possible to set up the BARs in a way that the
+kernel isn’t compelled to rewrite them, and store these values for
+later comparison.
+
+This avoids attacks such as setting the BAR to point to another device’s
+MMIO region which the above method can’t catch. Such a configuration
+would be “illegal”, but depending on the evaluation order of BARs
+in the chipset, this might effectively only disable the device used for
+the attack, while still fooling the SMM handler.
+
+Since this method isn’t generalizable, it has to be an optional
+compile-time feature.
+
+Caveats
+-------
+This capability might need to be hidden behind a Kconfig flag
+because we won’t be able to provide functional implementations of
+`is_address_in_mmio()` for every chipset supported by coreboot from the
+start.
+
+Security Considerations
+-----------------------
+The actual exploitability of the issue is unknown, but fixing it serves
+as defense in depth, similar to the
+[Memory Sinkhole mitigation](https://review.coreboot.org/#/c/11519/) for
+older Intel chipsets.
+
+Testing Plan
+------------
+Manual testing can be conducted easily by creating a small payload that
+provokes the reaction. It should test all conditions that enable the
+address test (ie. the different BAR offsets if used by SMM handlers).
--
To view, visit https://review.coreboot.org/19242
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Icba9d7910dfd46f32a2c46b6fd064a9cc8e3beac
Gerrit-PatchSet: 2
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins)
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/19446 )
Change subject: purism/librem13: Enable support for M.2 NVMe
......................................................................
purism/librem13: Enable support for M.2 NVMe
Enable/Disable the PCIe ports to match factory BIOS. The port #6
is used for PCIe on the M.2 connector which allows for NVMe SSDs
to function.
Change-Id: I8058cbad3da651144545d588c0ae78c5f5e598ac
Signed-off-by: Youness Alaoui <youness.alaoui(a)puri.sm>
Reviewed-on: https://review.coreboot.org/19446
Tested-by: build bot (Jenkins)
Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/mainboard/purism/librem13/devicetree.cb
1 file changed, 3 insertions(+), 3 deletions(-)
Approvals:
Arthur Heymans: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/mainboard/purism/librem13/devicetree.cb b/src/mainboard/purism/librem13/devicetree.cb
index af6641a..ba38070 100644
--- a/src/mainboard/purism/librem13/devicetree.cb
+++ b/src/mainboard/purism/librem13/devicetree.cb
@@ -50,11 +50,11 @@
device pci 19.0 off end # GbE
device pci 1b.0 on end # High Definition Audio
device pci 1c.0 on end # PCIe Port #1
- device pci 1c.1 on end # PCIe Port #2
+ device pci 1c.1 off end # PCIe Port #2
device pci 1c.2 on end # PCIe Port #3 - LAN
device pci 1c.3 on end # PCIe Port #4 - WiFi
- device pci 1c.4 off end # PCIe Port #5
- device pci 1c.5 off end # PCIe Port #6
+ device pci 1c.4 on end # PCIe Port #5
+ device pci 1c.5 on end # PCIe Port #6 - M.2 NVMe
device pci 1d.0 off end # USB2 EHCI
device pci 1e.0 off end # PCI bridge
device pci 1f.0 on
--
To view, visit https://review.coreboot.org/19446
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8058cbad3da651144545d588c0ae78c5f5e598ac
Gerrit-PatchSet: 4
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins)
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/19445 )
Change subject: util/inteltool: Add support for Wildcat Point-LP Premium
......................................................................
util/inteltool: Add support for Wildcat Point-LP Premium
The Wildcat Point-LP Premium is handled the same as the Wildcat Point-LP,
but it wasn't supported by inteltool.
Change-Id: I694514e1963f074582a3f5f81d63c20e7fa49189
Signed-off-by: Youness Alaoui <youness.alaoui(a)puri.sm>
Reviewed-on: https://review.coreboot.org/19445
Tested-by: build bot (Jenkins)
Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net>
Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M util/inteltool/gpio.c
M util/inteltool/inteltool.h
M util/inteltool/powermgt.c
M util/inteltool/rootcmplx.c
M util/inteltool/spi.c
5 files changed, 6 insertions(+), 0 deletions(-)
Approvals:
Arthur Heymans: Looks good to me, approved
Paul Menzel: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
diff --git a/util/inteltool/gpio.c b/util/inteltool/gpio.c
index db0e3a4..5fd160b 100644
--- a/util/inteltool/gpio.c
+++ b/util/inteltool/gpio.c
@@ -852,6 +852,7 @@
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_FULL:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_BASE:
+ case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP:
gpiobase = pci_read_word(sb, 0x48) & 0xfffc;
gpio_registers = lynxpoint_lp_gpio_registers;
@@ -1048,6 +1049,7 @@
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_FULL:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_BASE:
+ case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP:
for (i = 0; i < 95; i++) {
io_register_t tmp_gpio;
diff --git a/util/inteltool/inteltool.h b/util/inteltool/inteltool.h
index cd981d1..3e534b5 100644
--- a/util/inteltool/inteltool.h
+++ b/util/inteltool/inteltool.h
@@ -134,6 +134,7 @@
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_FULL 0x9c41
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_PREM 0x9c43
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_BASE 0x9c45
+#define PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_PREM 0x9cc3
#define PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP 0x9cc5
#define PCI_DEVICE_ID_INTEL_82810 0x7120
#define PCI_DEVICE_ID_INTEL_82810_DC 0x7122
diff --git a/util/inteltool/powermgt.c b/util/inteltool/powermgt.c
index 7f04308..5507985 100644
--- a/util/inteltool/powermgt.c
+++ b/util/inteltool/powermgt.c
@@ -701,6 +701,7 @@
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_FULL:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_BASE:
+ case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP:
case PCI_DEVICE_ID_INTEL_BAYTRAIL_LPC:
pmbase = pci_read_word(sb, 0x40) & 0xff80;
diff --git a/util/inteltool/rootcmplx.c b/util/inteltool/rootcmplx.c
index 337f981..2ad3410 100644
--- a/util/inteltool/rootcmplx.c
+++ b/util/inteltool/rootcmplx.c
@@ -95,6 +95,7 @@
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_FULL:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_BASE:
+ case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP:
rcba_phys = pci_read_long(sb, 0xf0) & 0xfffffffe;
break;
diff --git a/util/inteltool/spi.c b/util/inteltool/spi.c
index 154b3c9..cda8667 100644
--- a/util/inteltool/spi.c
+++ b/util/inteltool/spi.c
@@ -241,6 +241,7 @@
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_FULL:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_BASE:
+ case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP_PREM:
case PCI_DEVICE_ID_INTEL_WILDCATPOINT_LP:
spibaroffset = ICH9_SPIBAR;
rcba_phys = pci_read_long(sb, 0xf0) & 0xfffffffe;
--
To view, visit https://review.coreboot.org/19445
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I694514e1963f074582a3f5f81d63c20e7fa49189
Gerrit-PatchSet: 4
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins)