When a driver is probed through __driver_attach(), the bus' match()
callback is called without the device lock held, thus accessing the
driver_override field without a lock, which can cause a UAF.

Fix this by using the driver-core driver_override infrastructure taking
care of proper locking internally.

Note that calling match() from __driver_attach() without the device lock
held is intentional. [1]

Link: 
https://lore.kernel.org/driver-core/[email protected]/ [1]
Reported-by: Gui-Dong Han <[email protected]>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220789
Fixes: 2959ab247061 ("cdx: add the cdx bus driver")
Signed-off-by: Danilo Krummrich <[email protected]>
---
 drivers/cdx/cdx.c           | 40 +++++--------------------------------
 include/linux/cdx/cdx_bus.h |  4 ----
 2 files changed, 5 insertions(+), 39 deletions(-)

diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
index 9196dc50a48d..d3d230247262 100644
--- a/drivers/cdx/cdx.c
+++ b/drivers/cdx/cdx.c
@@ -156,8 +156,6 @@ static int cdx_unregister_device(struct device *dev,
        } else {
                cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
                debugfs_remove_recursive(cdx_dev->debugfs_dir);
-               kfree(cdx_dev->driver_override);
-               cdx_dev->driver_override = NULL;
        }
 
        /*
@@ -268,6 +266,7 @@ static int cdx_bus_match(struct device *dev, const struct 
device_driver *drv)
        const struct cdx_driver *cdx_drv = to_cdx_driver(drv);
        const struct cdx_device_id *found_id = NULL;
        const struct cdx_device_id *ids;
+       int ret;
 
        if (cdx_dev->is_bus)
                return false;
@@ -275,7 +274,8 @@ static int cdx_bus_match(struct device *dev, const struct 
device_driver *drv)
        ids = cdx_drv->match_id_table;
 
        /* When driver_override is set, only bind to the matching driver */
-       if (cdx_dev->driver_override && strcmp(cdx_dev->driver_override, 
drv->name))
+       ret = device_match_driver_override(dev, drv);
+       if (ret == 0)
                return false;
 
        found_id = cdx_match_id(ids, cdx_dev);
@@ -289,7 +289,7 @@ static int cdx_bus_match(struct device *dev, const struct 
device_driver *drv)
                 */
                if (!found_id->override_only)
                        return true;
-               if (cdx_dev->driver_override)
+               if (ret > 0)
                        return true;
 
                ids = found_id + 1;
@@ -453,36 +453,6 @@ static ssize_t modalias_show(struct device *dev, struct 
device_attribute *attr,
 }
 static DEVICE_ATTR_RO(modalias);
 
-static ssize_t driver_override_store(struct device *dev,
-                                    struct device_attribute *attr,
-                                    const char *buf, size_t count)
-{
-       struct cdx_device *cdx_dev = to_cdx_device(dev);
-       int ret;
-
-       if (WARN_ON(dev->bus != &cdx_bus_type))
-               return -EINVAL;
-
-       ret = driver_set_override(dev, &cdx_dev->driver_override, buf, count);
-       if (ret)
-               return ret;
-
-       return count;
-}
-
-static ssize_t driver_override_show(struct device *dev,
-                                   struct device_attribute *attr, char *buf)
-{
-       struct cdx_device *cdx_dev = to_cdx_device(dev);
-       ssize_t len;
-
-       device_lock(dev);
-       len = sysfs_emit(buf, "%s\n", cdx_dev->driver_override);
-       device_unlock(dev);
-       return len;
-}
-static DEVICE_ATTR_RW(driver_override);
-
 static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
                            const char *buf, size_t count)
 {
@@ -552,7 +522,6 @@ static struct attribute *cdx_dev_attrs[] = {
        &dev_attr_class.attr,
        &dev_attr_revision.attr,
        &dev_attr_modalias.attr,
-       &dev_attr_driver_override.attr,
        NULL,
 };
 
@@ -646,6 +615,7 @@ ATTRIBUTE_GROUPS(cdx_bus);
 
 const struct bus_type cdx_bus_type = {
        .name           = "cdx",
+       .driver_override = true,
        .match          = cdx_bus_match,
        .probe          = cdx_probe,
        .remove         = cdx_remove,
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
index b1ba97f6c9ad..f54770f110bc 100644
--- a/include/linux/cdx/cdx_bus.h
+++ b/include/linux/cdx/cdx_bus.h
@@ -137,9 +137,6 @@ struct cdx_controller {
  * @enabled: is this bus enabled
  * @msi_dev_id: MSI Device ID associated with CDX device
  * @num_msi: Number of MSI's supported by the device
- * @driver_override: driver name to force a match; do not set directly,
- *                   because core frees it; use driver_set_override() to
- *                   set or clear it.
  * @irqchip_lock: lock to synchronize irq/msi configuration
  * @msi_write_pending: MSI write pending for this device
  */
@@ -165,7 +162,6 @@ struct cdx_device {
        bool enabled;
        u32 msi_dev_id;
        u32 num_msi;
-       const char *driver_override;
        struct mutex irqchip_lock;
        bool msi_write_pending;
 };
-- 
2.53.0


Reply via email to