s390/ap: use generic driver_override infrastructure

When the AP masks are updated via apmask_store() or aqmask_store(),
ap_bus_revise_bindings() is called after ap_attr_mutex has been
released.

This calls __ap_revise_reserved(), which accesses the driver_override
field without holding any lock, racing against a concurrent
driver_override_store() that may free the old string, resulting in a
potential UAF.

Fix this by using the driver-core driver_override infrastructure, which
protects all accesses with an internal spinlock.

Note that unlike most other buses, the AP bus does not check
driver_override in its match() callback; the override is checked in
ap_device_probe() and __ap_revise_reserved() instead.

Also note that we do not enable the driver_override feature of struct
bus_type, as AP - in contrast to most other buses - passes "" to
sysfs_emit() when the driver_override pointer is NULL. Thus, printing
"\n" instead of "(null)\n".

Additionally, AP has a custom counter that is modified in the
corresponding custom driver_override_store().

Fixes: d38a87d7c064 ("s390/ap: Support driver_override for AP queue devices")
Tested-by: Holger Dengler <dengler@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
Reviewed-by: Harald Freudenberger <freude@linux.ibm.com>
Link: https://patch.msgid.link/20260324005919.2408620-11-dakr@kernel.org
Signed-off-by: Danilo Krummrich <dakr@kernel.org>

diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
index d652df96a50784362026e0695d075e114e385bc7..f24e27add721d1a97da3e1c3d4bf30f63575e8a5 100644 (file)
--- a/drivers/s390/crypto/ap_bus.c
+++ b/drivers/s390/crypto/ap_bus.c
@@ -859,25 +859,24 @@ static int __ap_queue_devices_with_id_unregister(struct device *dev, void *data)
 
 static int __ap_revise_reserved(struct device *dev, void *dummy)
 {
-       int rc, card, queue, devres, drvres;
+       int rc, card, queue, devres, drvres, ovrd;
 
        if (is_queue_dev(dev)) {
                struct ap_driver *ap_drv = to_ap_drv(dev->driver);
                struct ap_queue *aq = to_ap_queue(dev);
-               struct ap_device *ap_dev = &aq->ap_dev;
 
                card = AP_QID_CARD(aq->qid);
                queue = AP_QID_QUEUE(aq->qid);
 
-               if (ap_dev->driver_override) {
-                       if (strcmp(ap_dev->driver_override,
-                                  ap_drv->driver.name)) {
-                               pr_debug("reprobing queue=%02x.%04x\n", card, queue);
-                               rc = device_reprobe(dev);
-                               if (rc) {
-                                       AP_DBF_WARN("%s reprobing queue=%02x.%04x failed\n",
-                                                   __func__, card, queue);
-                               }
+               ovrd = device_match_driver_override(dev, &ap_drv->driver);
+               if (ovrd > 0) {
+                       /* override set and matches, nothing to do */
+               } else if (ovrd == 0) {
+                       pr_debug("reprobing queue=%02x.%04x\n", card, queue);
+                       rc = device_reprobe(dev);
+                       if (rc) {
+                               AP_DBF_WARN("%s reprobing queue=%02x.%04x failed\n",
+                                           __func__, card, queue);
                        }
                } else {
                        mutex_lock(&ap_attr_mutex);
@@ -928,7 +927,7 @@ int ap_owned_by_def_drv(int card, int queue)
        if (aq) {
                const struct device_driver *drv = aq->ap_dev.device.driver;
                const struct ap_driver *ap_drv = to_ap_drv(drv);
-               bool override = !!aq->ap_dev.driver_override;
+               bool override = device_has_driver_override(&aq->ap_dev.device);
 
                if (override && drv && ap_drv->flags & AP_DRIVER_FLAG_DEFAULT)
                        rc = 1;
@@ -977,7 +976,7 @@ static int ap_device_probe(struct device *dev)
 {
        struct ap_device *ap_dev = to_ap_dev(dev);
        struct ap_driver *ap_drv = to_ap_drv(dev->driver);
-       int card, queue, devres, drvres, rc = -ENODEV;
+       int card, queue, devres, drvres, rc = -ENODEV, ovrd;
 
        if (!get_device(dev))
                return rc;
@@ -991,10 +990,11 @@ static int ap_device_probe(struct device *dev)
                 */
                card = AP_QID_CARD(to_ap_queue(dev)->qid);
                queue = AP_QID_QUEUE(to_ap_queue(dev)->qid);
-               if (ap_dev->driver_override) {
-                       if (strcmp(ap_dev->driver_override,
-                                  ap_drv->driver.name))
-                               goto out;
+               ovrd = device_match_driver_override(dev, &ap_drv->driver);
+               if (ovrd > 0) {
+                       /* override set and matches, nothing to do */
+               } else if (ovrd == 0) {
+                       goto out;
                } else {
                        mutex_lock(&ap_attr_mutex);
                        devres = test_bit_inv(card, ap_perms.apm) &&

diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h
index 51e08f27bd75e8541923740cd7bc1c34cbd6a735..04ea256ecf9196588d9b944530168263d6cb65ba 100644 (file)
--- a/drivers/s390/crypto/ap_bus.h
+++ b/drivers/s390/crypto/ap_bus.h
@@ -166,7 +166,6 @@ void ap_driver_unregister(struct ap_driver *);
 struct ap_device {
        struct device device;
        int device_type;                /* AP device type. */
-       const char *driver_override;
 };
 
 #define to_ap_dev(x) container_of((x), struct ap_device, device)

diff --git a/drivers/s390/crypto/ap_queue.c b/drivers/s390/crypto/ap_queue.c
index 3fe2e41c5c6b1899541de7b8b624a6d344613fa2..ca9819e6f7e7617cb3d34accc0d3dc41406170c8 100644 (file)
--- a/drivers/s390/crypto/ap_queue.c
+++ b/drivers/s390/crypto/ap_queue.c
@@ -734,26 +734,14 @@ static ssize_t driver_override_show(struct device *dev,
                                    struct device_attribute *attr,
                                    char *buf)
 {
-       struct ap_queue *aq = to_ap_queue(dev);
-       struct ap_device *ap_dev = &aq->ap_dev;
-       int rc;
-
-       device_lock(dev);
-       if (ap_dev->driver_override)
-               rc = sysfs_emit(buf, "%s\n", ap_dev->driver_override);
-       else
-               rc = sysfs_emit(buf, "\n");
-       device_unlock(dev);
-
-       return rc;
+       guard(spinlock)(&dev->driver_override.lock);
+       return sysfs_emit(buf, "%s\n", dev->driver_override.name ?: "");
 }
 
 static ssize_t driver_override_store(struct device *dev,
                                     struct device_attribute *attr,
                                     const char *buf, size_t count)
 {
-       struct ap_queue *aq = to_ap_queue(dev);
-       struct ap_device *ap_dev = &aq->ap_dev;
        int rc = -EINVAL;
        bool old_value;
 
@@ -764,13 +752,13 @@ static ssize_t driver_override_store(struct device *dev,
        if (ap_apmask_aqmask_in_use)
                goto out;
 
-       old_value = ap_dev->driver_override ? true : false;
-       rc = driver_set_override(dev, &ap_dev->driver_override, buf, count);
+       old_value = device_has_driver_override(dev);
+       rc = __device_set_driver_override(dev, buf, count);
        if (rc)
                goto out;
-       if (old_value && !ap_dev->driver_override)
+       if (old_value && !device_has_driver_override(dev))
                --ap_driver_override_ctr;
-       else if (!old_value && ap_dev->driver_override)
+       else if (!old_value && device_has_driver_override(dev))
                ++ap_driver_override_ctr;
 
        rc = count;