Re: [PATCH RFC v2 08/10] vdpa: add vdpa simulator for block device
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2021-02-02 15:03:01
Also in:
kvm, lkml
On Thu, Jan 28, 2021 at 03:41:25PM +0100, Stefano Garzarella wrote:
quoted hunk ↗ jump to hunk
+static void vdpasim_blk_work(struct work_struct *work) +{ + struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); + u8 status = VIRTIO_BLK_S_OK; + int i; + + spin_lock(&vdpasim->lock); + + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) + goto out; + + for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { + struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; + + if (!vq->ready) + continue; + + while (vringh_getdesc_iotlb(&vq->vring, &vq->out_iov, + &vq->in_iov, &vq->head, + GFP_ATOMIC) > 0) { + int write; + + vq->in_iov.i = vq->in_iov.used - 1; + write = vringh_iov_push_iotlb(&vq->vring, &vq->in_iov, + &status, 1); + if (write <= 0) + break;
This code looks fragile: 1. Relying on unsigned underflow and the while loop in vringh_iov_push_iotlb() to handle the case where in_iov.used == 0 is risky and could break. 2. Does this assume that the last in_iov element has size 1? For example, the guest driver may send a single "in" iovec with size 513 when reading 512 bytes (with an extra byte for the request status). Please validate inputs fully, even in test/development code, because it's likely to be copied by others when writing production code (or deployed in production by unsuspecting users) :).