linux-kernel - Re: [PATCH] s390/vfio-ap: lock mdev object when handling mdev remove request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b763cd33-fb89-498a-841d-1a5423b7ef9b@linux.ibm.com>
Date: Wed, 12 Mar 2025 11:19:05 -0400
From: Matthew Rosato <mjrosato@...ux.ibm.com>
To: Anthony Krowiak <akrowiak@...ux.ibm.com>, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc: jjherne@...ux.ibm.com, pasic@...ux.ibm.com, hca@...ux.ibm.com,
        gor@...ux.ibm.com, borntraeger@...ux.ibm.com,
        alex.williamson@...hat.com, clg@...hat.com, stable@...r.kernel.org
Subject: Re: [PATCH] s390/vfio-ap: lock mdev object when handling mdev remove
 request

On 2/21/25 10:32 AM, Anthony Krowiak wrote:
> The vfio_ap_mdev_request function in drivers/s390/crypto/vfio_ap_ops.c
> accesses fields of an ap_matrix_mdev object without ensuring that the
> object is accessed by only one thread at a time. This patch adds the lock
> necessary to secure access to the ap_matrix_mdev object.
> 
> Fixes: 2e3d8d71e285 ("s390/vfio-ap: wire in the vfio_device_ops request callback")
> Signed-off-by: Anthony Krowiak <akrowiak@...ux.ibm.com>
> Cc: <stable@...r.kernel.org>

The new code itself seems sane.

But besides this area of code, there are 2 other paths that touch matrix_mdev->req_trigger:

the one via vfio_ap_set_request_irq will already hold the lock via vfio_ap_mdev_ioctl (OK).

However the other one in vfio_ap_mdev_probe acquires mdevs_lock a few lines -after- initializing req_trigger and cfg_chg_trigger to NULL.  Should the lock also be held there since we would have already registered the vfio device above?  We might be protected by circumstance because we are in .probe() but I'm not sure, and we bother getting the lock already to protect mdev_list...