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: [email protected]
> > Signed-off-by: Yuho Choi <[email protected]>
>
> Acked-by: Michael S. Tsirkin <[email protected]>
>
> > ---
> > 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
>
