On 05-07-26, 23:59, Guangshuo Li wrote:
> virtio_i2c_complete_reqs() waits for each queued request to complete
> before releasing the per-message DMA-safe buffer. The request array is
> then freed by virtio_i2c_xfer().
>
> After wait_for_completion_interruptible() was introduced, the wait can
> return before the current request has completed. In that case, the
> request and the remaining requests may still be owned by the virtqueue
> and may still be completed by the device later.
>
> The error path nevertheless continues to release each message buffer and
> virtio_i2c_xfer() frees the request array. A later device completion can
> then write to freed request memory or call complete() on a freed
> completion object.
>
> Reset the virtqueue when the wait is interrupted so that outstanding
> buffers are recovered before the request array and message buffers are
> freed.
>
> Fixes: a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible
> completion wait")
> Signed-off-by: Guangshuo Li <[email protected]>
> ---
> drivers/i2c/busses/i2c-virtio.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> index 5da6fef92bec..6ef09eda3ed0 100644
> --- a/drivers/i2c/busses/i2c-virtio.c
> +++ b/drivers/i2c/busses/i2c-virtio.c
> @@ -117,12 +117,19 @@ static int virtio_i2c_complete_reqs(struct virtqueue
> *vq,
> struct virtio_i2c_req *req = &reqs[i];
>
> if (!failed) {
> - if (wait_for_completion_interruptible(&req->completion))
> + if
> (wait_for_completion_interruptible(&req->completion)) {
> + /*
> + * The remaining buffers may still be owned by
> + * the device. Reset the virtqueue before
> freeing
> + * the request array and message buffers.
> + */
> + virtqueue_reset(vq, NULL, NULL);
> failed = true;
> - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> + } else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> failed = true;
> - else
> + } else {
> j++;
> + }
> }
>
> i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
+ Gavin, who was also trying to solve this path recently.
https://lore.kernel.org/all/[email protected]/
--
viresh
