Re: [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] zpci_remove_reserved_devices() calls zpci_device_reserved(zdev) without holding the required zdev->state_lock, leading to a race condition and double-put Use-After-Free.
--

--- Patch [2]: [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking ---
commit f2856d16e29fb1c200332bdad2a6ab6ca714f024
Author: Heiko Carstens <hca@linux.ibm.com>

s390/pci: Rework__zpci_event_availability() to remove conditional locking

This commit reworks __zpci_event_availability() to eliminate conditional
locking of the zdev->state_lock, resolving a clang static analysis warning.
The handling for the 0x0306 multiple-device event is also moved to the
beginning of the function since it does not target a specific device.

> diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
> index 48fa26dcbee14..f96ee87405f98 100644
> --- a/arch/s390/pci/pci_event.c
> +++ b/arch/s390/pci/pci_event.c
> @@ -389,19 +389,25 @@ static void zpci_event_reappear(struct zpci_dev *zdev)
>  
>  static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
>  {

[ ... ]

> -	if (existing_zdev)
> -		mutex_lock(&zdev->state_lock);
> +	/* 0x0306 - No handle or fid stored */
> +	if (ccdf->pec == 0x0306) {
> +		/* 0x308 or 0x302 for multiple devices */
> +		zpci_remove_reserved_devices();

[Severity: High]
This is a pre-existing issue, but does zpci_remove_reserved_devices() call
zpci_device_reserved() without holding the required zdev->state_lock?

Looking at zpci_remove_reserved_devices() in arch/s390/pci/pci.c, it
iterates through the remove list and calls zpci_device_reserved() directly:

arch/s390/pci/pci.c:zpci_remove_reserved_devices() {
    ...
    list_for_each_entry_safe(zdev, tmp, &remove, entry)
        zpci_device_reserved(zdev);
}

However, zpci_device_reserved() expects the lock to be held to safely
serialize state transitions and reference dropping:

arch/s390/pci/pci.c:zpci_device_reserved() {
    lockdep_assert_held(&zdev->state_lock);
    if (zdev->state == ZPCI_FN_STATE_RESERVED)
        return;
    zdev->state = ZPCI_FN_STATE_RESERVED;
    ...
    zpci_zdev_put(zdev);
}

Can this lead to a race condition if two threads process events for the same
device concurrently (e.g., a specific 0x0308 event and a bulk 0x0306 event)?

If both threads read the state as STANDBY simultaneously, they could both
invoke zpci_device_reserved() and cause zpci_zdev_put() to be called twice
for a single reference, potentially leading to a use-after-free of the
zpci_dev object.

> +		zpci_scan_devices();
> +		return;
> +	}
>  
> -	switch (ccdf->pec) {
> -	case 0x0301: /* Reserved|Standby -> Configured */
> -		if (!zdev) {
> +	zdev = get_zdev_by_fid(ccdf->fid);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803142907.1771509-1-hca@linux.ibm.com?part=2