While ccw_io_helper() seems like intended to be exclusive in a sense that it is supposed to facilitate I/O for at most one thread at any given time, there is actually nothing ensuring that threads won't pile up at vcdev->wait_q. If they all threads get woken up and see the status that belongs to some other request as their own. This can lead to bugs. For an example see : https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1788432 This normally does not cause problems, as these are usually infrequent operations that happen in a well defined sequence and normally do not fail. But occasionally sysfs attributes are directly dependent on pieces of virio config and trigger a get on each read. This gives us at least one method to trigger races. Signed-off-by: Halil Pasic <pasic@xxxxxxxxxxxxx> Reported-by: Colin Ian King <colin.king@xxxxxxxxxxxxx> --- This is a big hammer -- mutex on virtio_ccw device level would more than suffice. But I don't think it hurts, and maybe there is a better way e.g. one using some common ccw/cio mechanisms to address this. That's why this is an RFC. --- drivers/s390/virtio/virtio_ccw.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c index a5e8530a3391..36252f344660 100644 --- a/drivers/s390/virtio/virtio_ccw.c +++ b/drivers/s390/virtio/virtio_ccw.c @@ -289,6 +289,8 @@ static int doing_io(struct virtio_ccw_device *vcdev, __u32 flag) return ret; } +DEFINE_MUTEX(vcio_mtx); + static int ccw_io_helper(struct virtio_ccw_device *vcdev, struct ccw1 *ccw, __u32 intparm) { @@ -296,6 +298,7 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev, unsigned long flags; int flag = intparm & VIRTIO_CCW_INTPARM_MASK; + mutex_lock(&vcio_mtx); do { spin_lock_irqsave(get_ccwdev_lock(vcdev->cdev), flags); ret = ccw_device_start(vcdev->cdev, ccw, intparm, 0, 0); @@ -308,7 +311,9 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev, cpu_relax(); } while (ret == -EBUSY); wait_event(vcdev->wait_q, doing_io(vcdev, flag) == 0); - return ret ? ret : vcdev->err; + ret = ret ? ret : vcdev->err; + mutex_unlock(&vcio_mtx); + return ret; } static void virtio_ccw_drop_indicator(struct virtio_ccw_device *vcdev, -- 2.16.4
