[PATCH] firmware: coreboot: Fix a missing-check bug

From: Wenwen Wang
Date: Thu Oct 18 2018 - 11:37:20 EST


In coreboot_table_init(), a for loop is used to copy the entries of the
coreboot table. For each entry, the header of the entry, which is a
structure coreboot_table_entry and includes the size of the entry, is
firstly copied from the IO region 'ptr_entry' to 'entry' through the first
memcpy_fromio(). Then the 'entry.size' is used to allocate the
coreboot_device 'device' through kzalloc(). After 'device' is allocated,
the whole entry, including the header, is then copied to 'device->entry'
through the second memcpy_fromio(). Obviously, the header of the entry is
copied twice here. More importantly, no check is enforced after the second
copy to make sure the two copies obtain the same values. Given that the IO
region can also be accessed by the device, it is possible that
'device->entry.size' is different from 'entry.size' after the second copy,
especially when the device race to modify the size value between these two
copies. This can cause undefined behavior of the kernel and introduce
potential security risk, because 'device->entry.size' is inconsistent with
the actual size of the entry.

This patch rewrites the header of each entry after the second copy, using
the value acquired in the first copy. Through this way, the above issue can
be avoided.

Signed-off-by: Wenwen Wang <wang6495@xxxxxxx>
---
drivers/firmware/google/coreboot_table.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index 19db570..20fcd54 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -128,6 +128,7 @@ int coreboot_table_init(struct device *dev, void __iomem *ptr)
device->dev.bus = &coreboot_bus_type;
device->dev.release = coreboot_device_release;
memcpy_fromio(&device->entry, ptr_entry, entry.size);
+ device->entry = entry;

ret = device_register(&device->dev);
if (ret) {
--
2.7.4