Re: [PATCH v3 3/9] s390/vfio-ap: Fix missing lock required to access list of ap_matrix_mdev objects

[Matthew Rosato]

On 8/12/26 7:51 AM, Anthony Krowiak wrote:
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Anthony Krowiak <akrowiak@xxxxxxxxxxxxx>

Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>