openwrt/flashrom
1
0
Fork 0
mirror of https://review.coreboot.org/flashrom.git synced 2025-04-27 23:22:37 +02:00
Code Issues Releases Wiki Activity

rpci: Use pci_dev struct pointer to avoid API breaks

Browse Source 
The pci_dev structure is never meant to be used as is, but always as a
pointer. By using the struct itself in undo_pci_write_data, we are risking
data corruption, or buffer overflows if the structure size changes.

This is especially apparent on my system where flashrom segfaults
because I compile it with pciutils 3.3.0 and I run it on a system
with pciutils 3.5.2. The struture size is different and causes a
struct with the wrong size to be sent to the library, with invalid
internal field values.

This has been discovered and discussed in Change ID 18925 [1]

[1] https://review.coreboot.org/#/c/18925/

Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298
Signed-off-by: Youness Alaoui <kakaroto@kakaroto.homelinux.net>
Reviewed-on: https://review.coreboot.org/20784
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
This commit is contained in:
Youness Alaoui 2017-07-26 18:03:36 -04:00 committed by Nico Huber
parent 67d7179292
commit a54ceb1dbe
3 changed files with 25 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
chipset_enable.c
View File

@ -843,6 +843,7 @@ static int enable_flash_pch100(struct pci_dev *const dev, const char *const name
* straints (e.g. on PCI domains, extended PCIe config space).
*/
struct pci_access *const pci_acc = pci_alloc();
struct pci_access *const saved_pacc = pacc;
if (!pci_acc) {
msg_perr("Can't allocate PCI accessor.\n");
return ret;
@ -857,6 +858,9 @@ static int enable_flash_pch100(struct pci_dev *const dev, const char *const name
return ret;
}
/* Modify pacc so the rpci_write can register the undo callback with a
* device using the correct pci_access */
pacc = pci_acc;
enable_flash_ich_report_gcs(spi_dev, pch_generation, NULL);
const int ret_bc = enable_flash_ich_bios_cntl_config_space(spi_dev, pch_generation, 0xdc);
@ -880,6 +884,7 @@ static int enable_flash_pch100(struct pci_dev *const dev, const char *const name
_freepci_ret:
pci_free_dev(spi_dev);
pacc = saved_pacc;
return ret;
}

24
pcidev.c
View File

@ -160,6 +160,7 @@ static int pcidev_shutdown(void *data)
return 1;
}
pci_cleanup(pacc);
pacc = NULL;
return 0;
}
@ -259,7 +260,7 @@ enum pci_write_type {
};
struct undo_pci_write_data {
struct pci_dev dev;
struct pci_dev *dev;
int reg;
enum pci_write_type type;
union {
@ -272,22 +273,23 @@ struct undo_pci_write_data {
int undo_pci_write(void *p)
{
struct undo_pci_write_data *data = p;
if (pacc == NULL) {
msg_perr("%s: Tried to undo PCI writes without a valid PCI context!\n"
"Please report a bug at flashrom@flashrom.org\n", __func__);
if (pacc == NULL || data->dev == NULL) {
msg_perr("%s: Tried to undo PCI writes without a valid PCI %s!\n"
"Please report a bug at flashrom@flashrom.org\n",
__func__, data->dev == NULL ? "device" : "context");
return 1;
}
msg_pdbg("Restoring PCI config space for %02x:%02x:%01x reg 0x%02x\n",
data->dev.bus, data->dev.dev, data->dev.func, data->reg);
data->dev->bus, data->dev->dev, data->dev->func, data->reg);
switch (data->type) {
case pci_write_type_byte:
pci_write_byte(&data->dev, data->reg, data->bytedata);
pci_write_byte(data->dev, data->reg, data->bytedata);
break;
case pci_write_type_word:
pci_write_word(&data->dev, data->reg, data->worddata);
pci_write_word(data->dev, data->reg, data->worddata);
break;
case pci_write_type_long:
pci_write_long(&data->dev, data->reg, data->longdata);
pci_write_long(data->dev, data->reg, data->longdata);
break;
}
/* p was allocated in register_undo_pci_write. */
@ -303,7 +305,11 @@ int undo_pci_write(void *p)
msg_gerr("Out of memory!\n"); \
exit(1); \
} \
undo_pci_write_data->dev = *a; \
if (pacc) \
undo_pci_write_data->dev = pci_get_dev(pacc, \
a->domain, a->bus, a->dev, a->func); \
else \
undo_pci_write_data->dev = NULL; \
undo_pci_write_data->reg = b; \
undo_pci_write_data->type = pci_write_type_##c; \
undo_pci_write_data->c##data = pci_read_##c(dev, reg); \

5
programmer.h
View File

@ -195,6 +195,11 @@ uintptr_t pcidev_readbar(struct pci_dev *dev, int bar);
struct pci_dev *pcidev_init(const struct dev_entry *devs, int bar);
/* rpci_write_* are reversible writes. The original PCI config space register
* contents will be restored on shutdown.
* To clone the pci_dev instances internally, the `pacc` global
* variable has to reference a pci_access method that is compatible
* with the given pci_dev handle. The referenced pci_access (not
* the variable) has to stay valid until the shutdown handlers are
* finished.
*/
int rpci_write_byte(struct pci_dev *dev, int reg, uint8_t data);
int rpci_write_word(struct pci_dev *dev, int reg, uint16_t data);