* [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()
@ 2026-08-06 14:03 Anthony Krowiak
2026-08-06 14:18 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Anthony Krowiak @ 2026-08-06 14:03 UTC (permalink / raw)
To: linux-s390, linux-kernel, kvm
Cc: jjherne, borntraeger, mjrosato, pasic, alex, kwankhede, hca, gor,
agordeev, stable
In vfio_ap_mdev_set_kvm(), kvm->arch.crypto.pqap_hook is set to
&matrix_mdev->pqap_hook before the update locks are acquired and the
mdev list is checked for a conflicting assignment. If another mdev is
already attached to the same KVM instance, the function returns -EPERM
without restoring the hook pointer, leaving kvm->arch.crypto.pqap_hook
pointing at the failing matrix_mdev instead of the mdev that legitimately
owns the KVM.
Since matrix_mdev->kvm is never set on this error path,
vfio_ap_mdev_unset_kvm() will not clean up the hook when matrix_mdev
is later closed. If matrix_mdev is subsequently freed, any PQAP
instruction executed by the guest will dereference the stale pointer
through pqap_hook_rwsem, resulting in a use-after-free.
Since kvm->arch.crypto.pqap_hook is only set in the vfio_ap_mdev_set_kvm()
function and is cleared in the vfio_ap_mdev_unset_kvm() function, a check
for 'kvm->arch.crypto.pqap_hook != NULL' is all that is needed to determine
whether it belongs to another mdev. This will alleviate the need to iterate
the matrix_dev->mdev_list list to see if the kvm object is assigned to
another mdev.This was introduced in v3 to alleviate the need to take the
mdevs_lock while iterating the list; however, this did not prevent a
potential race condition.
The pqap_hook_rwsem(write) is now performed inside
get_update_locks_for_kvm(), which is updated to acquire
pqap_hook_rwsem(write) between kvm->lock and mdevs_lock. This ordering
is consistent with the PQAP intercept path, which acquires pqap_hook_rwsem
in read mode while srcu is held under vcpu->mutex, establishing the
dependency: kvm->lock -> vcpu->mutex -> srcu -> pqap_hook_rwsem(read).
The pqap_hook_rwsem is now released inside the
release_update_locks_for_kvm(), which is updated to release
pqap_hook_rwsem(write) between mdevs_lock and kvm->lock.
Additionally, kvm_put_kvm() in vfio_ap_mdev_unset_kvm() is moved
after release_update_locks_for_kvm(). Previously it was called while
kvm->lock was held; if it were ever the last reference, kvm_destroy_vm()
would run under kvm->lock, which would deadlock.
Fixes: 86956e70761b3 ("s390/vfio-ap: replace open coded locks for VFIO_GROUP_NOTIFY_SET_KVM notification")
Cc: stable@vger.kernel.org
Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
drivers/s390/crypto/vfio_ap_ops.c | 45 ++++++++++++++-----------------
1 file changed, 20 insertions(+), 25 deletions(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 44b3a1dcc1b3..99a0efd999ef 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -48,15 +48,19 @@ static void vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q);
* 1. matrix_dev->guests_lock: required to use the KVM pointer to update a KVM
* guest's APCB.
* 2. kvm->lock: required to update a guest's APCB
- * 3. matrix_dev->mdevs_lock: required to access data stored in a matrix_mdev
+ * 3. kvm->arch.crypto.pqap_hook_rwsem: required to update pqap_hook and
+ * serialize against PQAP intercepts
+ * 4. matrix_dev->mdevs_lock: required to access data stored in a matrix_mdev
*
- * Note: If @kvm is NULL, the KVM lock will not be taken.
+ * Note: If @kvm is NULL, the KVM lock and pqap_hook_rwsem will not be taken.
*/
static inline void get_update_locks_for_kvm(struct kvm *kvm)
{
mutex_lock(&matrix_dev->guests_lock);
- if (kvm)
+ if (kvm) {
mutex_lock(&kvm->lock);
+ down_write(&kvm->arch.crypto.pqap_hook_rwsem);
+ }
mutex_lock(&matrix_dev->mdevs_lock);
}
@@ -68,16 +72,19 @@ static inline void get_update_locks_for_kvm(struct kvm *kvm)
*
* The proper unlocking order is:
* 1. matrix_dev->mdevs_lock
- * 2. kvm->lock
- * 3. matrix_dev->guests_lock
+ * 2. kvm->arch.crypto.pqap_hook_rwsem
+ * 3. kvm->lock
+ * 4. matrix_dev->guests_lock
*
- * Note: If @kvm is NULL, the KVM lock will not be released.
+ * Note: If @kvm is NULL, the KVM lock and pqap_hook_rwsem will not be released.
*/
static inline void release_update_locks_for_kvm(struct kvm *kvm)
{
mutex_unlock(&matrix_dev->mdevs_lock);
- if (kvm)
+ if (kvm) {
+ up_write(&kvm->arch.crypto.pqap_hook_rwsem);
mutex_unlock(&kvm->lock);
+ }
mutex_unlock(&matrix_dev->guests_lock);
}
@@ -1821,26 +1828,17 @@ static const struct attribute_group *vfio_ap_mdev_attr_groups[] = {
static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
struct kvm *kvm)
{
- struct ap_matrix_mdev *m;
-
if (kvm->arch.crypto.crycbd) {
- down_write(&kvm->arch.crypto.pqap_hook_rwsem);
- kvm->arch.crypto.pqap_hook = &matrix_mdev->pqap_hook;
- up_write(&kvm->arch.crypto.pqap_hook_rwsem);
-
get_update_locks_for_kvm(kvm);
-
- list_for_each_entry(m, &matrix_dev->mdev_list, node) {
- if (m != matrix_mdev && m->kvm == kvm) {
- release_update_locks_for_kvm(kvm);
- return -EPERM;
- }
+ if (kvm->arch.crypto.pqap_hook) {
+ release_update_locks_for_kvm(kvm);
+ return -EPERM;
}
+ kvm->arch.crypto.pqap_hook = &matrix_mdev->pqap_hook;
kvm_get_kvm(kvm);
matrix_mdev->kvm = kvm;
vfio_ap_mdev_update_guest_apcb(matrix_mdev);
-
release_update_locks_for_kvm(kvm);
}
@@ -1883,18 +1881,15 @@ static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev)
struct kvm *kvm = matrix_mdev->kvm;
if (kvm && kvm->arch.crypto.crycbd) {
- down_write(&kvm->arch.crypto.pqap_hook_rwsem);
- kvm->arch.crypto.pqap_hook = NULL;
- up_write(&kvm->arch.crypto.pqap_hook_rwsem);
-
get_update_locks_for_kvm(kvm);
+ kvm->arch.crypto.pqap_hook = NULL;
kvm_arch_crypto_clear_masks(kvm);
vfio_ap_mdev_reset_queues(matrix_mdev);
- kvm_put_kvm(kvm);
matrix_mdev->kvm = NULL;
release_update_locks_for_kvm(kvm);
+ kvm_put_kvm(kvm);
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()
2026-08-06 14:03 [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm() Anthony Krowiak
@ 2026-08-06 14:18 ` sashiko-bot
2026-08-06 18:13 ` Anthony Krowiak
2026-08-06 14:28 ` Matthew Rosato
2026-08-06 14:40 ` Christian Borntraeger
2 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-08-06 14:18 UTC (permalink / raw)
To: Anthony Krowiak
Cc: linux-s390, Vasily Gorbik, Alexander Gordeev, kvm, Heiko Carstens
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Host AP configuration changes trigger a NULL pointer dereference when accessing unassigned matrix mediated devices.
--
--- Patch [1]: [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm() ---
commit aef5cc63216cb49a3fd8ebd1da1a8d39b4fabb7b
Author: Anthony Krowiak <akrowiak@linux.ibm.com>
s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()
In vfio_ap_mdev_set_kvm(), kvm->arch.crypto.pqap_hook is set to
&matrix_mdev->pqap_hook before the update locks are acquired and the
mdev list is checked for a conflicting assignment. If another mdev is
already attached to the same KVM instance, the function returns -EPERM
without restoring the hook pointer. This patch rearranges lock acquisition
to prevent this stale pointer issue and potential deadlocks.
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 44b3a1dcc1b36..99a0efd999ef9 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
[ ... ]
> @@ -1883,18 +1881,15 @@ static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev)
> struct kvm *kvm = matrix_mdev->kvm;
>
> if (kvm && kvm->arch.crypto.crycbd) {
> - down_write(&kvm->arch.crypto.pqap_hook_rwsem);
> - kvm->arch.crypto.pqap_hook = NULL;
> - up_write(&kvm->arch.crypto.pqap_hook_rwsem);
> -
> get_update_locks_for_kvm(kvm);
> + kvm->arch.crypto.pqap_hook = NULL;
>
> kvm_arch_crypto_clear_masks(kvm);
> vfio_ap_mdev_reset_queues(matrix_mdev);
> - kvm_put_kvm(kvm);
> matrix_mdev->kvm = NULL;
>
> release_update_locks_for_kvm(kvm);
> + kvm_put_kvm(kvm);
> }
> }
While reviewing this file, I noticed a couple of related issues.
drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_cfg_remove() {
...
list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
mutex_lock(&matrix_mdev->kvm->lock);
mutex_lock(&matrix_dev->mdevs_lock);
...
}
[Severity: High]
This is a pre-existing issue, but if a mediated device is unassigned
(matrix_mdev->kvm is NULL) when the host AP configuration is modified,
will this unconditional lock of matrix_mdev->kvm->lock trigger a NULL pointer
dereference?
drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_hot_plug_cfg() {
...
DECLARE_BITMAP(apm_filtered, AP_DEVICES);
bool filter_domains, filter_adapters, filter_cdoms, do_hotplug = false;
mutex_lock(&matrix_mdev->kvm->lock);
mutex_lock(&matrix_dev->mdevs_lock);
...
}
[Severity: High]
This isn't a bug introduced by this patch, but similar to the above, does this
function also dereference matrix_mdev->kvm without checking if it's NULL,
potentially leading to a crash when unassigned matrix mediated devices are
accessed during host AP configuration changes?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260806140342.611294-1-akrowiak@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()
2026-08-06 14:03 [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm() Anthony Krowiak
2026-08-06 14:18 ` sashiko-bot
@ 2026-08-06 14:28 ` Matthew Rosato
2026-08-06 14:40 ` Christian Borntraeger
2 siblings, 0 replies; 6+ messages in thread
From: Matthew Rosato @ 2026-08-06 14:28 UTC (permalink / raw)
To: Anthony Krowiak, linux-s390, linux-kernel, kvm
Cc: jjherne, borntraeger, pasic, alex, kwankhede, hca, gor, agordeev,
stable
>
> Fixes: 86956e70761b3 ("s390/vfio-ap: replace open coded locks for VFIO_GROUP_NOTIFY_SET_KVM notification")
> Cc: stable@vger.kernel.org
> Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
I think you need to re-arrange the SoB tags based on :
https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
Co-developed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()
2026-08-06 14:03 [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm() Anthony Krowiak
2026-08-06 14:18 ` sashiko-bot
2026-08-06 14:28 ` Matthew Rosato
@ 2026-08-06 14:40 ` Christian Borntraeger
2 siblings, 0 replies; 6+ messages in thread
From: Christian Borntraeger @ 2026-08-06 14:40 UTC (permalink / raw)
To: Anthony Krowiak, linux-s390, linux-kernel, kvm
Cc: jjherne, mjrosato, pasic, alex, kwankhede, hca, gor, agordeev,
stable
Am 06.08.26 um 16:03 schrieb Anthony Krowiak:
> In vfio_ap_mdev_set_kvm(), kvm->arch.crypto.pqap_hook is set to
> &matrix_mdev->pqap_hook before the update locks are acquired and the
> mdev list is checked for a conflicting assignment. If another mdev is
> already attached to the same KVM instance, the function returns -EPERM
> without restoring the hook pointer, leaving kvm->arch.crypto.pqap_hook
> pointing at the failing matrix_mdev instead of the mdev that legitimately
> owns the KVM.
>
> Since matrix_mdev->kvm is never set on this error path,
> vfio_ap_mdev_unset_kvm() will not clean up the hook when matrix_mdev
> is later closed. If matrix_mdev is subsequently freed, any PQAP
> instruction executed by the guest will dereference the stale pointer
> through pqap_hook_rwsem, resulting in a use-after-free.
>
> Since kvm->arch.crypto.pqap_hook is only set in the vfio_ap_mdev_set_kvm()
> function and is cleared in the vfio_ap_mdev_unset_kvm() function, a check
> for 'kvm->arch.crypto.pqap_hook != NULL' is all that is needed to determine
> whether it belongs to another mdev. This will alleviate the need to iterate
> the matrix_dev->mdev_list list to see if the kvm object is assigned to
> another mdev.This was introduced in v3 to alleviate the need to take the
> mdevs_lock while iterating the list; however, this did not prevent a
> potential race condition.
>
> The pqap_hook_rwsem(write) is now performed inside
> get_update_locks_for_kvm(), which is updated to acquire
> pqap_hook_rwsem(write) between kvm->lock and mdevs_lock. This ordering
> is consistent with the PQAP intercept path, which acquires pqap_hook_rwsem
> in read mode while srcu is held under vcpu->mutex, establishing the
> dependency: kvm->lock -> vcpu->mutex -> srcu -> pqap_hook_rwsem(read).
>
> The pqap_hook_rwsem is now released inside the
> release_update_locks_for_kvm(), which is updated to release
> pqap_hook_rwsem(write) between mdevs_lock and kvm->lock.
>
> Additionally, kvm_put_kvm() in vfio_ap_mdev_unset_kvm() is moved
> after release_update_locks_for_kvm(). Previously it was called while
> kvm->lock was held; if it were ever the last reference, kvm_destroy_vm()
> would run under kvm->lock, which would deadlock.
>
> Fixes: 86956e70761b3 ("s390/vfio-ap: replace open coded locks for VFIO_GROUP_NOTIFY_SET_KVM notification")
> Cc: stable@vger.kernel.org
> Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()
2026-08-06 14:18 ` sashiko-bot
@ 2026-08-06 18:13 ` Anthony Krowiak
2026-08-06 19:18 ` Anthony Krowiak
0 siblings, 1 reply; 6+ messages in thread
From: Anthony Krowiak @ 2026-08-06 18:13 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-s390, Vasily Gorbik, Alexander Gordeev, kvm, Heiko Carstens
On 8/6/26 10:18 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Host AP configuration changes trigger a NULL pointer dereference when accessing unassigned matrix mediated devices.
> --
>
> --- Patch [1]: [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm() ---
> commit aef5cc63216cb49a3fd8ebd1da1a8d39b4fabb7b
> Author: Anthony Krowiak <akrowiak@linux.ibm.com>
>
> s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()
>
> In vfio_ap_mdev_set_kvm(), kvm->arch.crypto.pqap_hook is set to
> &matrix_mdev->pqap_hook before the update locks are acquired and the
> mdev list is checked for a conflicting assignment. If another mdev is
> already attached to the same KVM instance, the function returns -EPERM
> without restoring the hook pointer. This patch rearranges lock acquisition
> to prevent this stale pointer issue and potential deadlocks.
>
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
>> index 44b3a1dcc1b36..99a0efd999ef9 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> [ ... ]
>
>> @@ -1883,18 +1881,15 @@ static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev)
>> struct kvm *kvm = matrix_mdev->kvm;
>>
>> if (kvm && kvm->arch.crypto.crycbd) {
>> - down_write(&kvm->arch.crypto.pqap_hook_rwsem);
>> - kvm->arch.crypto.pqap_hook = NULL;
>> - up_write(&kvm->arch.crypto.pqap_hook_rwsem);
>> -
>> get_update_locks_for_kvm(kvm);
>> + kvm->arch.crypto.pqap_hook = NULL;
>>
>> kvm_arch_crypto_clear_masks(kvm);
>> vfio_ap_mdev_reset_queues(matrix_mdev);
>> - kvm_put_kvm(kvm);
>> matrix_mdev->kvm = NULL;
>>
>> release_update_locks_for_kvm(kvm);
>> + kvm_put_kvm(kvm);
>> }
>> }
> While reviewing this file, I noticed a couple of related issues.
>
> drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_cfg_remove() {
> ...
> list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
> mutex_lock(&matrix_mdev->kvm->lock);
> mutex_lock(&matrix_dev->mdevs_lock);
> ...
> }
>
> [Severity: High]
> This is a pre-existing issue, but if a mediated device is unassigned
> (matrix_mdev->kvm is NULL) when the host AP configuration is modified,
> will this unconditional lock of matrix_mdev->kvm->lock trigger a NULL pointer
> dereference?
>
>
> drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_hot_plug_cfg() {
> ...
> DECLARE_BITMAP(apm_filtered, AP_DEVICES);
> bool filter_domains, filter_adapters, filter_cdoms, do_hotplug = false;
>
> mutex_lock(&matrix_mdev->kvm->lock);
> mutex_lock(&matrix_dev->mdevs_lock);
> ...
> }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but similar to the above, does this
> function also dereference matrix_mdev->kvm without checking if it's NULL,
> potentially leading to a crash when unassigned matrix mediated devices are
> accessed during host AP configuration changes?
Both of the above the concerns above are legitimate for the case whereby
the matrix_mdev->kvm was never set via the vfio_ap_mdev_set_kvm()
function which is called only when a guest is started with the mdev attached
and the mdev fd is opened. If the mdev is not attached to a guest, then
matrix_mdev->kvm will be NULL.
The matrix_mdev->kvm pointer is set to NULL
in the vfio_ap_mdev_unset_kvm; however, all three functions (the unset_kvm
and the two above) all take the matrix_dev->guests_lock before proceeding,
so the matrix_dev->kvm pointer is guaranteed to not be NULL due to a
race condition.
Having said that, it is clear that a check must be introduced to verify that
matrix_mdev->kvm is not NULL before taking the matrix_mdev->kvm->lock
in the two functions above.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()
2026-08-06 18:13 ` Anthony Krowiak
@ 2026-08-06 19:18 ` Anthony Krowiak
0 siblings, 0 replies; 6+ messages in thread
From: Anthony Krowiak @ 2026-08-06 19:18 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-s390, Vasily Gorbik, Alexander Gordeev, kvm, Heiko Carstens,
Jason Herne, Christian Borntraeger, Matthew Rosato,
Claudio Imbrenda, Janosch Frank
On 8/6/26 2:13 PM, Anthony Krowiak wrote:
>
>
> On 8/6/26 10:18 AM, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential
>> issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] Host AP configuration changes trigger a NULL pointer
>> dereference when accessing unassigned matrix mediated devices.
>> --
>>
>> --- Patch [1]: [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer
>> on error in vfio_ap_mdev_set_kvm() ---
>> commit aef5cc63216cb49a3fd8ebd1da1a8d39b4fabb7b
>> Author: Anthony Krowiak <akrowiak@linux.ibm.com>
>>
>> s390/vfio-ap: fix stale pqap_hook pointer on error in
>> vfio_ap_mdev_set_kvm()
>>
>> In vfio_ap_mdev_set_kvm(), kvm->arch.crypto.pqap_hook is set to
>> &matrix_mdev->pqap_hook before the update locks are acquired and the
>> mdev list is checked for a conflicting assignment. If another mdev is
>> already attached to the same KVM instance, the function returns -EPERM
>> without restoring the hook pointer. This patch rearranges lock
>> acquisition
>> to prevent this stale pointer issue and potential deadlocks.
>>
>>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
>>> b/drivers/s390/crypto/vfio_ap_ops.c
>>> index 44b3a1dcc1b36..99a0efd999ef9 100644
>>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> [ ... ]
>>
>>> @@ -1883,18 +1881,15 @@ static void vfio_ap_mdev_unset_kvm(struct
>>> ap_matrix_mdev *matrix_mdev)
>>> struct kvm *kvm = matrix_mdev->kvm;
>>> if (kvm && kvm->arch.crypto.crycbd) {
>>> - down_write(&kvm->arch.crypto.pqap_hook_rwsem);
>>> - kvm->arch.crypto.pqap_hook = NULL;
>>> - up_write(&kvm->arch.crypto.pqap_hook_rwsem);
>>> -
>>> get_update_locks_for_kvm(kvm);
>>> + kvm->arch.crypto.pqap_hook = NULL;
>>> kvm_arch_crypto_clear_masks(kvm);
>>> vfio_ap_mdev_reset_queues(matrix_mdev);
>>> - kvm_put_kvm(kvm);
>>> matrix_mdev->kvm = NULL;
>>> release_update_locks_for_kvm(kvm);
>>> + kvm_put_kvm(kvm);
>>> }
>>> }
>> While reviewing this file, I noticed a couple of related issues.
>>
>> drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_cfg_remove() {
>> ...
>> list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
>> mutex_lock(&matrix_mdev->kvm->lock);
>> mutex_lock(&matrix_dev->mdevs_lock);
>> ...
>> }
>>
>> [Severity: High]
>> This is a pre-existing issue, but if a mediated device is unassigned
>> (matrix_mdev->kvm is NULL) when the host AP configuration is modified,
>> will this unconditional lock of matrix_mdev->kvm->lock trigger a NULL
>> pointer
>> dereference?
>>
>>
>> drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_hot_plug_cfg() {
>> ...
>> DECLARE_BITMAP(apm_filtered, AP_DEVICES);
>> bool filter_domains, filter_adapters, filter_cdoms, do_hotplug =
>> false;
>>
>> mutex_lock(&matrix_mdev->kvm->lock);
>> mutex_lock(&matrix_dev->mdevs_lock);
>> ...
>> }
>>
>> [Severity: High]
>> This isn't a bug introduced by this patch, but similar to the above,
>> does this
>> function also dereference matrix_mdev->kvm without checking if it's
>> NULL,
>> potentially leading to a crash when unassigned matrix mediated
>> devices are
>> accessed during host AP configuration changes?
>
> Both of the above the concerns above are legitimate for the case whereby
> the matrix_mdev->kvm was never set via the vfio_ap_mdev_set_kvm()
> function which is called only when a guest is started with the mdev
> attached
> and the mdev fd is opened. If the mdev is not attached to a guest, then
> matrix_mdev->kvm will be NULL.
>
> The matrix_mdev->kvm pointer is set to NULL
> in the vfio_ap_mdev_unset_kvm; however, all three functions (the
> unset_kvm
> and the two above) all take the matrix_dev->guests_lock before
> proceeding,
> so the matrix_dev->kvm pointer is guaranteed to not be NULL due to a
> race condition.
>
> Having said that, it is clear that a check must be introduced to
> verify that
> matrix_mdev->kvm is not NULL before taking the matrix_mdev->kvm->lock
> in the two functions above.
The fix for this bug will be posted via a separate patch.
>
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-06 19:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 14:03 [PATCH v4] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm() Anthony Krowiak
2026-08-06 14:18 ` sashiko-bot
2026-08-06 18:13 ` Anthony Krowiak
2026-08-06 19:18 ` Anthony Krowiak
2026-08-06 14:28 ` Matthew Rosato
2026-08-06 14:40 ` Christian Borntraeger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox