While using the new_id interface, the user can unintentionally feed incorrect values if the driver static table has a matching entry. This is possible since only the device and vendor fields are mandatory and the rest are optional. As a result, store_new_id will fill in default values that are then passed on to the driver and can have unintended consequences. As an example, consider the ixgbe driver and the 82599EB network card : echo "8086 10fb" > /sys/bus/pci/drivers/ixgbe/new_id This will pass a driver_data value of 0 to the driver whereas the index 0 in ixgbe actually points to a different set of card operations. This change automatically selects the matching static entry if there is one for the newly created dynid. However, if the user intentionally wants a different set of values, she must provide all the 7 fields and the static entry will be ignored. In most cases, this use case seems unnecessary, however, this is a common libvirt/KVM/device assignment scenario where the user might want to bind a device back to the host driver. Signed-off-by: Bandan Das <bsd@xxxxxxxxxx> --- drivers/pci/pci-driver.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 25f0bc6..187e572 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -90,6 +90,24 @@ static void pci_free_dynids(struct pci_driver *drv) spin_unlock(&drv->dynids.lock); } +static const struct +pci_device_id *match_id_table_entry(struct device_driver *driver, + __u32 vendor, __u32 device) +{ + struct pci_driver *pdrv = to_pci_driver(driver); + const struct pci_device_id *ids = pdrv->id_table; + + if (ids) { + while (ids->vendor || ids->subvendor || ids->class_mask) { + if ((ids->vendor == vendor) && (ids->device == device)) + return ids; + ids++; + } + } + + return NULL; +} + /** * store_new_id - sysfs frontend to pci_add_dynid() * @driver: target device driver @@ -102,7 +120,8 @@ static ssize_t store_new_id(struct device_driver *driver, const char *buf, size_t count) { struct pci_driver *pdrv = to_pci_driver(driver); - const struct pci_device_id *ids = pdrv->id_table; + const struct pci_device_id *ids = pdrv->id_table, + *tids = NULL; __u32 vendor, device, subvendor=PCI_ANY_ID, subdevice=PCI_ANY_ID, class=0, class_mask=0; unsigned long driver_data=0; @@ -115,9 +134,24 @@ store_new_id(struct device_driver *driver, const char *buf, size_t count) if (fields < 2) return -EINVAL; - /* Only accept driver_data values that match an existing id_table - entry */ - if (ids) { + tids = match_id_table_entry(driver, vendor, device); + + if (tids && (fields != 7)) { + + subvendor = tids->subvendor; + subdevice = tids->subdevice; + class = tids->class; + class_mask = tids->class_mask; + driver_data = tids->driver_data; + + pr_warn("pci: Using driver (%s) static DeviceID table entry for vendor 0x%04x and device 0x%04x", + driver->name, vendor, device); + + } else if (ids) { + + /* Only accept driver_data values that match an existing + id_table entry */ + retval = -EINVAL; while (ids->vendor || ids->subvendor || ids->class_mask) { if (driver_data == ids->driver_data) { -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html