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

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

[William Bezenah]

When detaching virtio devices with multiple queues, spurious and
non-fatal error messages appear in the guest kernel log. While
virtio-net devices have multiple queues by default, this issue can
also be reproduced with other virtio device types (e.g., virtio-blk)
when configured with multiple queues:

[   33.820621] virtio_ccw 0.0.0001: Failed to deregister indicators (-22)
[   33.820628] virtio_net virtio2: Error -22 while deleting queue 0
[   33.820632] virtio_net virtio2: Error -22 while deleting queue 1
[   33.820634] virtio_net virtio2: Error -22 while deleting queue 2

Since commit 8c58a229688c ("s390/cio: Do not unregister the
subchannel based on DNV"), subchannel behavior following a device
detach has been updated and results in -EINVAL being propagated
rather than -ENODEV, originating from ccw_device_start_timeout_key()
in cio/device_ops. In the end, the virtio driver has no ability to
react to the difference between device and subchannel states here,
and during detach, both -ENODEV and -EINVAL indicate the device
cannot be used and should not be treated as errors requiring
attention. Update error handling in virtio_ccw_del_vq() and
virtio_ccw_drop_indicator() to suppress -EINVAL in addition to
-ENODEV.

Fixes: 8c58a229688c ("s390/cio: Do not unregister the subchannel based on DNV")
Signed-off-by: William Bezenah <wbezenah@linux.ibm.com>
---
 drivers/s390/virtio/virtio_ccw.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index bab6cad3fd5c..02fd8bf7e469 100644
--- 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)
@@ -515,10 +515,10 @@ static void virtio_ccw_del_vq(struct virtqueue *vq, struct ccw1 *ccw)
 	ret = ccw_io_helper(vcdev, ccw,
 			    VIRTIO_CCW_DOING_SET_VQ | index);
 	/*
-	 * -ENODEV isn't considered an error: The device is gone anyway.
+	 * -ENODEV and -EINVAL aren't considered errors: The device is gone anyway.
 	 * This may happen on device detach.
 	 */
-	if (ret && (ret != -ENODEV))
+	if (ret && (ret != -ENODEV) && (ret != -EINVAL))
 		dev_warn(&vq->vdev->dev, "Error %d while deleting queue %d\n",
 			 ret, index);
 
-- 
2.54.0

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

[sashiko-bot]

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

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

[Halil Pasic]

On Fri, 12 Jun 2026 16:05:10 +0000
sashiko-bot@kernel.org wrote:

> [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?

Looks like this could be more robust against scenarios where we fail
to execute the channel program that is supposed to deregister the
indicators from the device.

The key question here is, at what point, can we consider adpter
interruption and event notification quiesced. I will have to do some
reading on that, I'm afraid. 

To me, it looks like just setting the virtqueue pointers in
airq_info->aiv would not do the trick either, because I don't see
a corresponding null pointer check.

Regards,
Halil

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

[Halil Pasic]

On Fri, 12 Jun 2026 17:54:07 +0200
William Bezenah <wbezenah@linux.ibm.com> wrote:

> Since commit 8c58a229688c ("s390/cio: Do not unregister the
> subchannel based on DNV"), subchannel behavior following a device
> detach has been updated and results in -EINVAL being propagated
> rather than -ENODEV, originating from ccw_device_start_timeout_key()
> in cio/device_ops. In the end, the virtio driver has no ability to
> react to the difference between device and subchannel states here,
> and during detach, both -ENODEV and -EINVAL indicate the device
> cannot be used and should not be treated as errors requiring
> attention. Update error handling in virtio_ccw_del_vq() and
> virtio_ccw_drop_indicator() to suppress -EINVAL in addition to
> -ENODEV.

Hi William!

Are you saying that ccw_device_start() started returning -EINVAL
since 8c58a229688c ("s390/cio: Do not unregister the subchannel based on
DNV")? Or did I somehow read the paragraph wrong?

The funcition ccw_device_start is documented to return:
 * Returns:                                                                     
 *  %0, if the operation was successful;                                        
 *  -%EBUSY, if the device is busy, or status pending;                          
 *  -%EACCES, if no path specified in @lpm is operational;                      
 *  -%ENODEV, if the device is not operational. 
and the commit message does not say a thing about introducing -EINVAL to
the mix.

Regards,
Halil