In order to traverse or add/remove ap_matrix_mdev objects in the matrix_dev->mdev_list, the matrix_dev->guests_lock mutex must be held. There are two functions that access the list without holding the mutex: vfio_ap_mdev_probe function ~~~~~~~~~~~~~~~~~~~~~~~~~~~ The vfio_ap_mdev_probe function uses the matrix_dev->mdevs_lock mutex to guard the add of a newly created ap_matrix_mdev object to the matrix_dev->mdev_list. This mutex does not protect list access; its purpose is to guard against concurrent access to fields contained in an ap_matrix_mdev object. This could lead to kernel memory corruption or use-after-free if another mdev is created or removed concurrently. The adding of an ap_matrix_mdev object to matrix_dev->mdev_list is now guarded by the matrix_dev->guests_lock which is the correct way to protect against concurrent mdev_list access. vfio_ap_mdev_for_queue function ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The status_show function that supports display of the status attribute of the devices in /sys/bus/ap/devices calls the vfio_ap_mdev_for_queue function which iterates the matrix_dev->mdev_list to find the object representing the queue device whose status is to be displayed. In order to traverse this list, the matrix_dev->guests_lock mutex must be held. To fix this, the guests_lock mutex is taken prior to taking the matrix_dev->mdevs_lock mutex in the status_show function. It is taken there rather than the vfio_ap_mdev_for_queue function - where it is needed - because it must be taken prior to the mdevs_lock mutex in order to adhere to the proper locking order and prevent a lockdep splat; also because the mdevs_lock is needed there to access fields within the matrix_mdev object in that function. See the vfio-ap-locking.rst in the linux kernel tree. Fixes: 2c1ee8983aa3 ("s390/vfio-ap: prepare for dynamic update of guest's APCB on queue probe/remove") Cc: stable@vger.kernel.org Signed-off-by: Anthony Krowiak --- drivers/s390/crypto/vfio_ap_ops.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index e382e5a1cb99..5c14ded4fd8e 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -803,9 +803,9 @@ static int vfio_ap_mdev_probe(struct mdev_device *mdev) matrix_mdev->req_trigger = NULL; matrix_mdev->cfg_chg_trigger = NULL; dev_set_drvdata(&mdev->dev, matrix_mdev); - mutex_lock(&matrix_dev->mdevs_lock); + mutex_lock(&matrix_dev->guests_lock); list_add(&matrix_mdev->node, &matrix_dev->mdev_list); - mutex_unlock(&matrix_dev->mdevs_lock); + mutex_unlock(&matrix_dev->guests_lock); return 0; err_put_vdev: @@ -2297,6 +2297,8 @@ static struct ap_matrix_mdev *vfio_ap_mdev_for_queue(struct vfio_ap_queue *q) unsigned long apid = AP_QID_CARD(q->apqn); unsigned long apqi = AP_QID_QUEUE(q->apqn); + lockdep_assert_held(&matrix_dev->guests_lock); + list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) { if (test_bit_inv(apid, matrix_mdev->matrix.apm) && test_bit_inv(apqi, matrix_mdev->matrix.aqm)) @@ -2316,6 +2318,7 @@ static ssize_t status_show(struct device *dev, struct ap_matrix_mdev *matrix_mdev; struct ap_device *apdev = to_ap_dev(dev); + mutex_lock(&matrix_dev->guests_lock); mutex_lock(&matrix_dev->mdevs_lock); q = dev_get_drvdata(&apdev->device); matrix_mdev = vfio_ap_mdev_for_queue(q); @@ -2343,6 +2346,7 @@ static ssize_t status_show(struct device *dev, } mutex_unlock(&matrix_dev->mdevs_lock); + mutex_unlock(&matrix_dev->guests_lock); return nchars; } -- 2.53.0