Re: [PATCH v6 09/15] s390/vfio-ap: Add method to set a new guest AP configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Latent NULL pointer dereference in `collect_queues_by_apid` due to contradictory logic.
--

--- Patch [9]: [PATCH v6 09/15] s390/vfio-ap: Add method to set a new guest AP configuration ---
commit 5b979f09ef9429e09eace5977dabbd22cc832bbf
Author: Anthony Krowiak <akrowiak@linux.ibm.com>

s390/vfio-ap: Add method to set a new guest AP configuration

This commit adds a new vfio_ap_set_new_config function to set a guest's AP
configuration. It refactors code from ap_config_store to handle changes to
the sysfs ap_config attribute and support live guest migration.

> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index d05372b50d2fc..0b49be22d5d8b 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c

[ ... ]

> -static void collect_queues_to_reset(struct ap_matrix_mdev *matrix_mdev,
> -				    unsigned long apid,
> -				    struct list_head *qlist)
> +static void collect_queues_by_apid(struct ap_matrix_mdev *matrix_mdev,
> +				   unsigned long apid,
> +				   struct list_head *qlist)
>  {
>  	struct vfio_ap_queue *q;
>  	unsigned long  apqi;
>  
>  	for_each_set_bit_inv(apqi, matrix_mdev->shadow_apcb.aqm, AP_DOMAINS) {
> -		q = vfio_ap_mdev_get_queue(matrix_mdev, AP_MKQID(apid, apqi));
> +		q = matrix_mdev ?
> +				vfio_ap_mdev_get_queue(matrix_mdev, AP_MKQID(apid, apqi)) :
> +				vfio_ap_find_queue(AP_MKQID(apid, apqi));

[Severity: Medium]
Will this result in a NULL pointer dereference if matrix_mdev is NULL?

The loop condition unconditionally accesses matrix_mdev->shadow_apcb.aqm.
However, the ternary operator inside the loop (matrix_mdev ?) suggests
that matrix_mdev could be NULL. 

If matrix_mdev can indeed be NULL, the unconditional dereference in 
for_each_set_bit_inv() will crash before the check is evaluated. If 
matrix_mdev is never NULL in practice, is the ternary check necessary?

>  		if (q)
>  			list_add_tail(&q->reset_qnode, qlist);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727173239.2420754-1-akrowiak@linux.ibm.com?part=9