Skip to content

Commit 81d6f7c

Browse files
author
Danilo Krummrich
committed
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: d38a87d ("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>
1 parent ac4d8bb commit 81d6f7c

3 files changed

Lines changed: 23 additions & 36 deletions

File tree

drivers/s390/crypto/ap_bus.c

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -859,25 +859,24 @@ static int __ap_queue_devices_with_id_unregister(struct device *dev, void *data)
859859

860860
static int __ap_revise_reserved(struct device *dev, void *dummy)
861861
{
862-
int rc, card, queue, devres, drvres;
862+
int rc, card, queue, devres, drvres, ovrd;
863863

864864
if (is_queue_dev(dev)) {
865865
struct ap_driver *ap_drv = to_ap_drv(dev->driver);
866866
struct ap_queue *aq = to_ap_queue(dev);
867-
struct ap_device *ap_dev = &aq->ap_dev;
868867

869868
card = AP_QID_CARD(aq->qid);
870869
queue = AP_QID_QUEUE(aq->qid);
871870

872-
if (ap_dev->driver_override) {
873-
if (strcmp(ap_dev->driver_override,
874-
ap_drv->driver.name)) {
875-
pr_debug("reprobing queue=%02x.%04x\n", card, queue);
876-
rc = device_reprobe(dev);
877-
if (rc) {
878-
AP_DBF_WARN("%s reprobing queue=%02x.%04x failed\n",
879-
__func__, card, queue);
880-
}
871+
ovrd = device_match_driver_override(dev, &ap_drv->driver);
872+
if (ovrd > 0) {
873+
/* override set and matches, nothing to do */
874+
} else if (ovrd == 0) {
875+
pr_debug("reprobing queue=%02x.%04x\n", card, queue);
876+
rc = device_reprobe(dev);
877+
if (rc) {
878+
AP_DBF_WARN("%s reprobing queue=%02x.%04x failed\n",
879+
__func__, card, queue);
881880
}
882881
} else {
883882
mutex_lock(&ap_attr_mutex);
@@ -928,7 +927,7 @@ int ap_owned_by_def_drv(int card, int queue)
928927
if (aq) {
929928
const struct device_driver *drv = aq->ap_dev.device.driver;
930929
const struct ap_driver *ap_drv = to_ap_drv(drv);
931-
bool override = !!aq->ap_dev.driver_override;
930+
bool override = device_has_driver_override(&aq->ap_dev.device);
932931

933932
if (override && drv && ap_drv->flags & AP_DRIVER_FLAG_DEFAULT)
934933
rc = 1;
@@ -977,7 +976,7 @@ static int ap_device_probe(struct device *dev)
977976
{
978977
struct ap_device *ap_dev = to_ap_dev(dev);
979978
struct ap_driver *ap_drv = to_ap_drv(dev->driver);
980-
int card, queue, devres, drvres, rc = -ENODEV;
979+
int card, queue, devres, drvres, rc = -ENODEV, ovrd;
981980

982981
if (!get_device(dev))
983982
return rc;
@@ -991,10 +990,11 @@ static int ap_device_probe(struct device *dev)
991990
*/
992991
card = AP_QID_CARD(to_ap_queue(dev)->qid);
993992
queue = AP_QID_QUEUE(to_ap_queue(dev)->qid);
994-
if (ap_dev->driver_override) {
995-
if (strcmp(ap_dev->driver_override,
996-
ap_drv->driver.name))
997-
goto out;
993+
ovrd = device_match_driver_override(dev, &ap_drv->driver);
994+
if (ovrd > 0) {
995+
/* override set and matches, nothing to do */
996+
} else if (ovrd == 0) {
997+
goto out;
998998
} else {
999999
mutex_lock(&ap_attr_mutex);
10001000
devres = test_bit_inv(card, ap_perms.apm) &&

drivers/s390/crypto/ap_bus.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@ void ap_driver_unregister(struct ap_driver *);
166166
struct ap_device {
167167
struct device device;
168168
int device_type; /* AP device type. */
169-
const char *driver_override;
170169
};
171170

172171
#define to_ap_dev(x) container_of((x), struct ap_device, device)

drivers/s390/crypto/ap_queue.c

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -734,26 +734,14 @@ static ssize_t driver_override_show(struct device *dev,
734734
struct device_attribute *attr,
735735
char *buf)
736736
{
737-
struct ap_queue *aq = to_ap_queue(dev);
738-
struct ap_device *ap_dev = &aq->ap_dev;
739-
int rc;
740-
741-
device_lock(dev);
742-
if (ap_dev->driver_override)
743-
rc = sysfs_emit(buf, "%s\n", ap_dev->driver_override);
744-
else
745-
rc = sysfs_emit(buf, "\n");
746-
device_unlock(dev);
747-
748-
return rc;
737+
guard(spinlock)(&dev->driver_override.lock);
738+
return sysfs_emit(buf, "%s\n", dev->driver_override.name ?: "");
749739
}
750740

751741
static ssize_t driver_override_store(struct device *dev,
752742
struct device_attribute *attr,
753743
const char *buf, size_t count)
754744
{
755-
struct ap_queue *aq = to_ap_queue(dev);
756-
struct ap_device *ap_dev = &aq->ap_dev;
757745
int rc = -EINVAL;
758746
bool old_value;
759747

@@ -764,13 +752,13 @@ static ssize_t driver_override_store(struct device *dev,
764752
if (ap_apmask_aqmask_in_use)
765753
goto out;
766754

767-
old_value = ap_dev->driver_override ? true : false;
768-
rc = driver_set_override(dev, &ap_dev->driver_override, buf, count);
755+
old_value = device_has_driver_override(dev);
756+
rc = __device_set_driver_override(dev, buf, count);
769757
if (rc)
770758
goto out;
771-
if (old_value && !ap_dev->driver_override)
759+
if (old_value && !device_has_driver_override(dev))
772760
--ap_driver_override_ctr;
773-
else if (!old_value && ap_dev->driver_override)
761+
else if (!old_value && device_has_driver_override(dev))
774762
++ap_driver_override_ctr;
775763

776764
rc = count;

0 commit comments

Comments
 (0)