Re: [PATCH v1] s390/virtio_ccw: Also suppress -EINVAL on device detach

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] If deregistering indicators from the host fails with an error other than -ENODEV or -EINVAL (e.g., -EIO), virtio_ccw_drop_indicator() skips the cleanup of local adapter interrupt pointers, leading to a subsequent use-after-free when virtqueues are deleted.
--

commit 6f54104d3533aa779587af76d171dec3ef8e1b8f
Author: William Bezenah <wbezenah@linux.ibm.com>

s390/virtio_ccw: Also suppress -EINVAL on device detach
    
This patch updates the error handling in virtio_ccw_del_vq() and
virtio_ccw_drop_indicator() to suppress -EINVAL along with -ENODEV
during device detach. This avoids spurious warning messages in the
kernel log when detaching virtio devices with multiple queues.

> diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -429,7 +429,7 @@ static void virtio_ccw_drop_indicator(struct virtio_ccw_device *vcdev,
>  			    vcdev->is_thinint ?
>  			    VIRTIO_CCW_DOING_SET_IND_ADAPTER :
>  			    VIRTIO_CCW_DOING_SET_IND);
> -	if (ret && (ret != -ENODEV))
> +	if (ret && (ret != -ENODEV) && (ret != -EINVAL))
>  		dev_info(&vcdev->cdev->dev,
>  			 "Failed to deregister indicators (%d)\n", ret);
>  	else if (vcdev->is_thinint)
>  		virtio_ccw_drop_indicators(vcdev);

[Severity: Critical]
This is a pre-existing issue, but does this if/else structure create a
use-after-free risk during virtqueue deletion?

If ccw_io_helper() fails with an error other than -ENODEV or -EINVAL
(for example, -EIO), the first branch executes and logs the warning, but
the 'else if' branch is entirely skipped.

This means virtio_ccw_drop_indicators() is never called, failing to clear
the virtqueue pointers in the shared airq_info->aiv array.

Because virtio_ccw_del_vqs() subsequently proceeds to free these virtqueues
via vring_del_virtqueue(), the uncleared pointers in the shared airq_info
will point to freed memory.

If an adapter interrupt arrives later for this leaked bit, could
virtio_airq_handler() retrieve the dangling pointer and trigger a
use-after-free by calling vring_interrupt() on the freed queue?

[ ... ]

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