[PATCH v1] Revert "i2c: virtio: Avoid hang by using interruptible completion wait"

[PATCH v1] Revert "i2c: virtio: Avoid hang by using interruptible completion wait"

[Yuho Choi]

This reverts commit a663b3c47ab10f66130818cf94eb59c971541c3f.

When a transfer is interrupted by a signal, virtio_i2c_complete_reqs()
stops waiting for the remaining requests and virtio_i2c_xfer() frees the
reqs array while the virtqueue descriptors are still in flight on the
device.  The backend can then write into freed memory, and
virtio_i2c_msg_done() calls complete() on already-freed requests.

Commit 84e1d0bf1d71 ("i2c: virtio: disable timeout handling") removed the
exact same failure mode caused by timeouts, concluding there was no simple
fix because the buffers must be held until the device returns them.  A hang
due to an unresponsive backend is preferable to guest memory corruption.
Restore the unconditional wait until request lifetime can be decoupled
safely.

Fixes: a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible completion wait")
Cc: stable@vger.kernel.org
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
A proper interruptible wait requires refcounting requests and bounce
buffering to hold memory until the backend returns descriptors (similar
to virtio_rtc/virtio_pmem). Since that is a larger rework unsuitable for
stable, revert to uninterruptible wait first.

 drivers/i2c/busses/i2c-virtio.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
index 5da6fef92bec3..581e55b5c65ba 100644
--- a/drivers/i2c/busses/i2c-virtio.c
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -116,16 +116,15 @@ static int virtio_i2c_complete_reqs(struct virtqueue *vq,
 	for (i = 0; i < num; i++) {
 		struct virtio_i2c_req *req = &reqs[i];
 
-		if (!failed) {
-			if (wait_for_completion_interruptible(&req->completion))
-				failed = true;
-			else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
-				failed = true;
-			else
-				j++;
-		}
+		wait_for_completion(&req->completion);
+
+		if (!failed && req->in_hdr.status != VIRTIO_I2C_MSG_OK)
+			failed = true;
 
 		i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
+
+		if (!failed)
+			j++;
 	}
 
 	return j;

base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
-- 
2.43.0

Re: [PATCH v1] Revert "i2c: virtio: Avoid hang by using interruptible completion wait"

[Michael S. Tsirkin]

On Tue, Sep 22, 2026 at 01:02:51AM -0400, Yuho Choi wrote:
> This reverts commit a663b3c47ab10f66130818cf94eb59c971541c3f.
> 
> When a transfer is interrupted by a signal, virtio_i2c_complete_reqs()
> stops waiting for the remaining requests and virtio_i2c_xfer() frees the
> reqs array while the virtqueue descriptors are still in flight on the
> device.  The backend can then write into freed memory, and
> virtio_i2c_msg_done() calls complete() on already-freed requests.
> 
> Commit 84e1d0bf1d71 ("i2c: virtio: disable timeout handling") removed the
> exact same failure mode caused by timeouts, concluding there was no simple
> fix because the buffers must be held until the device returns them.  A hang
> due to an unresponsive backend is preferable to guest memory corruption.
> Restore the unconditional wait until request lifetime can be decoupled
> safely.
> 
> Fixes: a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible completion wait")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> A proper interruptible wait requires refcounting requests and bounce
> buffering to hold memory until the backend returns descriptors (similar
> to virtio_rtc/virtio_pmem). Since that is a larger rework unsuitable for
> stable, revert to uninterruptible wait first.

another way is ring or device reset, if that is acceptable.

>  drivers/i2c/busses/i2c-virtio.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> index 5da6fef92bec3..581e55b5c65ba 100644
> --- a/drivers/i2c/busses/i2c-virtio.c
> +++ b/drivers/i2c/busses/i2c-virtio.c
> @@ -116,16 +116,15 @@ static int virtio_i2c_complete_reqs(struct virtqueue *vq,
>  	for (i = 0; i < num; i++) {
>  		struct virtio_i2c_req *req = &reqs[i];
>  
> -		if (!failed) {
> -			if (wait_for_completion_interruptible(&req->completion))
> -				failed = true;
> -			else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> -				failed = true;
> -			else
> -				j++;
> -		}
> +		wait_for_completion(&req->completion);
> +
> +		if (!failed && req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> +			failed = true;
>  
>  		i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> +
> +		if (!failed)
> +			j++;
>  	}
>  
>  	return j;
> 
> base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
> -- 
> 2.43.0

Re: [PATCH v1] Revert "i2c: virtio: Avoid hang by using interruptible completion wait"

[Andi Shyti]

Viresh, can I have a comment from you here, please?

Thanks,
Andi

On Tue, Sep 22, 2026 at 01:17:06AM -0400, Michael S. Tsirkin wrote:
> On Tue, Sep 22, 2026 at 01:02:51AM -0400, Yuho Choi wrote:
> > This reverts commit a663b3c47ab10f66130818cf94eb59c971541c3f.
> > 
> > When a transfer is interrupted by a signal, virtio_i2c_complete_reqs()
> > stops waiting for the remaining requests and virtio_i2c_xfer() frees the
> > reqs array while the virtqueue descriptors are still in flight on the
> > device.  The backend can then write into freed memory, and
> > virtio_i2c_msg_done() calls complete() on already-freed requests.
> > 
> > Commit 84e1d0bf1d71 ("i2c: virtio: disable timeout handling") removed the
> > exact same failure mode caused by timeouts, concluding there was no simple
> > fix because the buffers must be held until the device returns them.  A hang
> > due to an unresponsive backend is preferable to guest memory corruption.
> > Restore the unconditional wait until request lifetime can be decoupled
> > safely.
> > 
> > Fixes: a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible completion wait")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> > ---
> > A proper interruptible wait requires refcounting requests and bounce
> > buffering to hold memory until the backend returns descriptors (similar
> > to virtio_rtc/virtio_pmem). Since that is a larger rework unsuitable for
> > stable, revert to uninterruptible wait first.
> 
> another way is ring or device reset, if that is acceptable.
> 
> >  drivers/i2c/busses/i2c-virtio.c | 15 +++++++--------
> >  1 file changed, 7 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> > index 5da6fef92bec3..581e55b5c65ba 100644
> > --- a/drivers/i2c/busses/i2c-virtio.c
> > +++ b/drivers/i2c/busses/i2c-virtio.c
> > @@ -116,16 +116,15 @@ static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> >  	for (i = 0; i < num; i++) {
> >  		struct virtio_i2c_req *req = &reqs[i];
> >  
> > -		if (!failed) {
> > -			if (wait_for_completion_interruptible(&req->completion))
> > -				failed = true;
> > -			else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> > -				failed = true;
> > -			else
> > -				j++;
> > -		}
> > +		wait_for_completion(&req->completion);
> > +
> > +		if (!failed && req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> > +			failed = true;
> >  
> >  		i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
> > +
> > +		if (!failed)
> > +			j++;
> >  	}
> >  
> >  	return j;
> > 
> > base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
> > -- 
> > 2.43.0
> 

Re: [PATCH v1] Revert "i2c: virtio: Avoid hang by using interruptible completion wait"

[Viresh Kumar]

On 24-09-26, 22:28, Andi Shyti wrote:
> Viresh, can I have a comment from you here, please?

This was discussed earlier as well, can you have a look at this thread too ?

https://lore.kernel.org/all/yaeiwzxxh42y3oxiro5uxaxbvralwqxweb6cx4vzwx4y7nards@cysus3gbvple/

-- 
viresh