diff options
| author | Bandan Das <bsd@redhat.com> | 2014-04-01 21:32:59 -0400 |
|---|---|---|
| committer | Bjorn Helgaas <bhelgaas@google.com> | 2014-04-29 19:36:44 -0400 |
| commit | 8895d3bcb8ba960b1b83f95d772b641352ea8e51 (patch) | |
| tree | fe6a3d11c5e029fc59cef437afede23a63d91d60 | |
| parent | 7c82126a94e69bbbac586f0249e7ef11e681246c (diff) | |
PCI: Fail new_id for vendor/device values already built into driver
While using the sysfs 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 pci_device_id with driver_data = 0 to ixgbe_probe(), which
uses that zero to index a table of card operations. The zeroth entry of
the table does *not* correspond to the 82599 operations.
This change returns an error if the user attempts to add a dynid for a
vendor/device combination for which a static entry already exists.
However, if the user intentionally wants a different set of values, she
must provide all the 7 fields and that will be accepted.
[bhelgaas: drop KVM text since the problem isn't KVM-specific]
Signed-off-by: Bandan Das <bsd@redhat.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
| -rw-r--r-- | drivers/pci/pci-driver.c | 22 |
1 files changed, 21 insertions, 1 deletions
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index d911e0c1f359..650ce8935fd7 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c | |||
| @@ -107,7 +107,7 @@ store_new_id(struct device_driver *driver, const char *buf, size_t count) | |||
| 107 | subdevice=PCI_ANY_ID, class=0, class_mask=0; | 107 | subdevice=PCI_ANY_ID, class=0, class_mask=0; |
| 108 | unsigned long driver_data=0; | 108 | unsigned long driver_data=0; |
| 109 | int fields=0; | 109 | int fields=0; |
| 110 | int retval; | 110 | int retval = 0; |
| 111 | 111 | ||
| 112 | fields = sscanf(buf, "%x %x %x %x %x %x %lx", | 112 | fields = sscanf(buf, "%x %x %x %x %x %x %lx", |
| 113 | &vendor, &device, &subvendor, &subdevice, | 113 | &vendor, &device, &subvendor, &subdevice, |
| @@ -115,6 +115,26 @@ store_new_id(struct device_driver *driver, const char *buf, size_t count) | |||
| 115 | if (fields < 2) | 115 | if (fields < 2) |
| 116 | return -EINVAL; | 116 | return -EINVAL; |
| 117 | 117 | ||
| 118 | if (fields != 7) { | ||
| 119 | struct pci_dev *pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); | ||
| 120 | if (!pdev) | ||
| 121 | return -ENOMEM; | ||
| 122 | |||
| 123 | pdev->vendor = vendor; | ||
| 124 | pdev->device = device; | ||
| 125 | pdev->subsystem_vendor = subvendor; | ||
| 126 | pdev->subsystem_device = subdevice; | ||
| 127 | pdev->class = class; | ||
| 128 | |||
| 129 | if (pci_match_id(pdrv->id_table, pdev)) | ||
| 130 | retval = -EEXIST; | ||
| 131 | |||
| 132 | kfree(pdev); | ||
| 133 | |||
| 134 | if (retval) | ||
| 135 | return retval; | ||
| 136 | } | ||
| 137 | |||
| 118 | /* Only accept driver_data values that match an existing id_table | 138 | /* Only accept driver_data values that match an existing id_table |
| 119 | entry */ | 139 | entry */ |
| 120 | if (ids) { | 140 | if (ids) { |
