linux-kernel - [PATCH] PCI: rework new_id interface for known vendor/device values

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <jpgsipz3sbs.fsf@nelium.bos.redhat.com>
Date:	Mon, 31 Mar 2014 00:28:07 -0400
From:	Bandan Das <bsd@...hat.com>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
Cc:	Alex Williamson <alex.williamson@...hat.com>,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] PCI: rework new_id interface for known vendor/device values


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@...hat.com>
---
 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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/