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:
+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) :).