From dbad3ab97af39455c4e4f6967bdd47ef1ea67198 Mon Sep 17 00:00:00 2001 From: Max Gurtovoy Date: Thu, 13 May 2021 15:38:27 +0300 Subject: [PATCH 2/2] PCI: add matching checks for driver_override binding Allowing any driver in the system to be unconditionally bound to any PCI HW is dangerous. Connecting a driver to a physical HW device it was never intended to operate may trigger exploitable kernel bugs, or worse. It also allows userspace to load and run kernel code that otherwise would never be runnable on the system. Enable the option for drivers to specifically opt into driver_override feature. These drivers will expose a proper matching table that indicates what HW it can properly support. vfio-pci and pci stub drivers continues to support everything. The modalias tables to be populated with dedicated match table and userspace now becomes aware that vfio-pci can be loaded against any PCI device using driver_override (modalias for pci stub drivers can be added in the future, if needed). The dynamic new_id option is still might be in use for these drivers to preserve backward compatibility. For now, also preserve the old driver_override mechanism for non "driver_override" modules. Signed-off-by: Max Gurtovoy --- Documentation/ABI/testing/sysfs-bus-pci | 5 ++++- drivers/pci/pci-driver.c | 26 +++++++++++++++++++++---- drivers/pci/pci-pf-stub.c | 1 + drivers/pci/pci-stub.c | 9 ++++++++- drivers/vfio/pci/vfio_pci.c | 9 ++++++++- 5 files changed, 43 insertions(+), 7 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci index ef00fada2efb..ffb31fda597d 100644 --- a/Documentation/ABI/testing/sysfs-bus-pci +++ b/Documentation/ABI/testing/sysfs-bus-pci @@ -289,7 +289,10 @@ Description: will not bind to any driver. This also allows devices to opt-out of driver binding using a driver_override name such as "none". Only a single driver may be specified in the override, - there is no support for parsing delimiters. + there is no support for parsing delimiters. For static IDs + matching, this binding mechanism is allowed if the matching + driver has specifically opt into this feature (by setting a + suitable flag in the "flags" field of pci_device_id structure). What: /sys/bus/pci/devices/.../numa_node Date: Oct 2014 diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index ec44a79e951a..ba4494499931 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -136,7 +136,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, struct pci_dev *dev) { struct pci_dynid *dynid; - const struct pci_device_id *found_id = NULL; + const struct pci_device_id *found_id = NULL, *ids; /* When driver_override is set, only bind to the matching driver */ if (dev->driver_override && strcmp(dev->driver_override, drv->name)) @@ -152,10 +152,28 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, } spin_unlock(&drv->dynids.lock); - if (!found_id) - found_id = pci_match_id(drv->id_table, dev); + if (found_id) + return found_id; - /* driver_override will always match, send a dummy id */ + ids = drv->id_table; + while ((found_id = pci_match_id(ids, dev))) { + /* + * if we found id in the static table, we must fulfill the + * matching flags (i.e. if PCI_ID_F_DRIVER_OVERRIDE flag is + * set, driver_override should be provided). + */ + bool is_driver_override = + (found_id->flags & PCI_ID_F_DRIVER_OVERRIDE) != 0; + if (is_driver_override && !dev->driver_override) + ids = found_id + 1; /* continue searching */ + else + break; /* found a match ! */ + } + + /* + * if no static match, driver_override will always match, send a dummy + * id. + */ if (!found_id && dev->driver_override) found_id = &pci_device_id_any; diff --git a/drivers/pci/pci-pf-stub.c b/drivers/pci/pci-pf-stub.c index 45855a5e9fca..49544ba9a7af 100644 --- a/drivers/pci/pci-pf-stub.c +++ b/drivers/pci/pci-pf-stub.c @@ -19,6 +19,7 @@ */ static const struct pci_device_id pci_pf_stub_whitelist[] = { { PCI_VDEVICE(AMAZON, 0x0053) }, + { PCI_DEVICE_FLAGS(PCI_ANY_ID, PCI_ANY_ID, PCI_ID_F_STUB_DRIVER_OVERRIDE) }, /* match all by default (override) */ /* required last entry */ { 0 } }; diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c index e408099fea52..a67d136ad1e2 100644 --- a/drivers/pci/pci-stub.c +++ b/drivers/pci/pci-stub.c @@ -26,6 +26,13 @@ MODULE_PARM_DESC(ids, "Initial PCI IDs to add to the stub driver, format is " "\"vendor:device[:subvendor[:subdevice[:class[:class_mask]]]]\"" " and multiple comma separated entries can be specified"); +static const struct pci_device_id stub_pci_table[] = { + { PCI_DEVICE_FLAGS(PCI_ANY_ID, PCI_ANY_ID, PCI_ID_F_STUB_DRIVER_OVERRIDE) }, /* match all by default */ + { 0, } +}; + +MODULE_DEVICE_TABLE(pci, stub_pci_table); + static int pci_stub_probe(struct pci_dev *dev, const struct pci_device_id *id) { pci_info(dev, "claimed by stub\n"); @@ -34,7 +41,7 @@ static int pci_stub_probe(struct pci_dev *dev, const struct pci_device_id *id) static struct pci_driver stub_driver = { .name = "pci-stub", - .id_table = NULL, /* only dynamic id's */ + .id_table = stub_pci_table, .probe = pci_stub_probe, }; diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 850ea3a94e28..9beb4b841945 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -166,9 +166,16 @@ static int vfio_pci_sriov_configure(struct pci_dev *pdev, int nr_virtfn) return vfio_pci_core_sriov_configure(pdev, nr_virtfn); } +static const struct pci_device_id vfio_pci_table[] = { + { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_ANY_ID, PCI_ANY_ID) }, /* match all by default */ + { 0, } +}; + +MODULE_DEVICE_TABLE(pci, vfio_pci_table); + static struct pci_driver vfio_pci_driver = { .name = "vfio-pci", - .id_table = NULL, /* only dynamic ids */ + .id_table = vfio_pci_table, .probe = vfio_pci_probe, .remove = vfio_pci_remove, .sriov_configure = vfio_pci_sriov_configure, -- 2.18.1