When getting invalid data from vring, virtqueue_pop used to print an
error and exit.
Add an errp parameter so it can return the error to callers.
Signed-off-by: Fam Zheng <f...@redhat.com>
---
hw/9pfs/virtio-9p.c | 2 +-
hw/block/virtio-blk.c | 2 +-
hw/char/virtio-serial-bus.c | 10 +++----
hw/net/virtio-net.c | 6 ++--
hw/scsi/virtio-scsi.c | 2 +-
hw/virtio/virtio-balloon.c | 4 +--
hw/virtio/virtio-rng.c | 2 +-
hw/virtio/virtio.c | 70 ++++++++++++++++++++++++++++++++++-----------
include/hw/virtio/virtio.h | 2 +-
9 files changed, 69 insertions(+), 31 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 4964da0..17d0c4a 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3259,7 +3259,7 @@ void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
ssize_t len;
while ((pdu = alloc_pdu(s)) &&
- (len = virtqueue_pop(vq, &pdu->elem)) != 0) {
+ (len = virtqueue_pop(vq, &pdu->elem, &error_abort)) != 0) {
uint8_t *ptr;
pdu->s = s;
BUG_ON(pdu->elem.out_num == 0 || pdu->elem.in_num == 0);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index f7d8528..0b66ee1 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -191,7 +191,7 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock
*s)
{
VirtIOBlockReq *req = virtio_blk_alloc_request(s);
- if (!virtqueue_pop(s->vq, &req->elem)) {
+ if (!virtqueue_pop(s->vq, &req->elem, &error_abort)) {
virtio_blk_free_request(req);
return NULL;
}
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index a56dafc..76a934b 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -94,7 +94,7 @@ static size_t write_to_port(VirtIOSerialPort *port,
while (offset < size) {
size_t len;
- if (!virtqueue_pop(vq, &elem)) {
+ if (!virtqueue_pop(vq, &elem, &error_abort)) {
break;
}
@@ -116,7 +116,7 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice
*vdev)
if (!virtio_queue_ready(vq)) {
return;
}
- while (virtqueue_pop(vq, &elem)) {
+ while (virtqueue_pop(vq, &elem, &error_abort)) {
virtqueue_push(vq, &elem, 0);
}
virtio_notify(vdev, vq);
@@ -137,7 +137,7 @@ static void do_flush_queued_data(VirtIOSerialPort *port,
VirtQueue *vq,
/* Pop an elem only if we haven't left off a previous one mid-way */
if (!port->elem.out_num) {
- if (!virtqueue_pop(vq, &port->elem)) {
+ if (!virtqueue_pop(vq, &port->elem, &error_abort)) {
break;
}
port->iov_idx = 0;
@@ -190,7 +190,7 @@ static size_t send_control_msg(VirtIOSerial *vser, void
*buf, size_t len)
if (!virtio_queue_ready(vq)) {
return 0;
}
- if (!virtqueue_pop(vq, &elem)) {
+ if (!virtqueue_pop(vq, &elem, &error_abort)) {
return 0;
}
@@ -420,7 +420,7 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
len = 0;
buf = NULL;
- while (virtqueue_pop(vq, &elem)) {
+ while (virtqueue_pop(vq, &elem, &error_abort)) {
size_t cur_len;
cur_len = iov_size(elem.out_sg, elem.out_num);
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 59f76bc..bbcb51f 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -804,7 +804,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
struct iovec *iov, *iov2;
unsigned int iov_cnt;
- while (virtqueue_pop(vq, &elem)) {
+ while (virtqueue_pop(vq, &elem, &error_abort)) {
if (iov_size(elem.in_sg, elem.in_num) < sizeof(status) ||
iov_size(elem.out_sg, elem.out_num) < sizeof(ctrl)) {
error_report("virtio-net ctrl missing headers");
@@ -1031,7 +1031,7 @@ static ssize_t virtio_net_receive(NetClientState *nc,
const uint8_t *buf, size_t
total = 0;
- if (virtqueue_pop(q->rx_vq, &elem) == 0) {
+ if (virtqueue_pop(q->rx_vq, &elem, &error_abort) == 0) {
if (i == 0)
return -1;
error_report("virtio-net unexpected empty queue: "
@@ -1134,7 +1134,7 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
return num_packets;
}
- while (virtqueue_pop(q->tx_vq, &elem)) {
+ while (virtqueue_pop(q->tx_vq, &elem, &error_abort)) {
ssize_t ret, len;
unsigned int out_num = elem.out_num;
struct iovec *out_sg = &elem.out_sg[0];
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index c9bea06..40ba03d 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -177,7 +177,7 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
static VirtIOSCSIReq *virtio_scsi_pop_req(VirtIOSCSI *s, VirtQueue *vq)
{
VirtIOSCSIReq *req = virtio_scsi_init_req(s, vq);
- if (!virtqueue_pop(vq, &req->elem)) {
+ if (!virtqueue_pop(vq, &req->elem, &error_abort)) {
virtio_scsi_free_req(req);
return NULL;
}
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 95b0643..e26c0a7 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -206,7 +206,7 @@ static void virtio_balloon_handle_output(VirtIODevice
*vdev, VirtQueue *vq)
VirtQueueElement elem;
MemoryRegionSection section;
- while (virtqueue_pop(vq, &elem)) {
+ while (virtqueue_pop(vq, &elem, &error_abort)) {
size_t offset = 0;
uint32_t pfn;
@@ -246,7 +246,7 @@ static void virtio_balloon_receive_stats(VirtIODevice
*vdev, VirtQueue *vq)
size_t offset = 0;
qemu_timeval tv;
- if (!virtqueue_pop(vq, elem)) {
+ if (!virtqueue_pop(vq, elem, &error_abort)) {
goto out;
}
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 9e1bd75..c3cbdc3 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -56,7 +56,7 @@ static void chr_read(void *opaque, const void *buf, size_t
size)
offset = 0;
while (offset < size) {
- if (!virtqueue_pop(vrng->vq, &elem)) {
+ if (!virtqueue_pop(vrng->vq, &elem, &error_abort)) {
break;
}
len = iov_from_buf(elem.in_sg, elem.in_num,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 1cd454b..e6f9f6b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -481,29 +481,49 @@ fail:
error_setg(errp, "virtio: error trying to map MMIO memory");
}
-int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
+static void virtqueue_undo_map_sg(struct iovec *sg, hwaddr *addr,
+ size_t num_sg, int is_write)
+{
+ int i;
+
+ for (i = 0; i < num_sg; i++) {
+ cpu_physical_memory_unmap(sg[i].iov_base, sg[i].iov_len,
+ is_write, 0);
+ }
+}
+
+int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem, Error **errp)
{
unsigned int i, head, max;
+ int ret;
hwaddr desc_pa = vq->vring.desc;
VirtIODevice *vdev = vq->vdev;
+ Error *local_err = NULL;
- if (!virtqueue_num_heads(vq, vq->last_avail_idx, &error_abort))
- return 0;
+ ret = virtqueue_num_heads(vq, vq->last_avail_idx, &local_err);
+ if (ret <= 0) {
+ goto err;
+ }
/* When we start there are none of either input nor output. */
elem->out_num = elem->in_num = 0;
max = vq->vring.num;
- i = head = virtqueue_get_head(vq, vq->last_avail_idx++, &error_abort);
+ ret = virtqueue_get_head(vq, vq->last_avail_idx++, &local_err);
+ if (ret < 0) {
+ goto err;
+ }
+ head = i = ret;
+
if (virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
vring_set_avail_event(vq, vq->last_avail_idx);
}
if (vring_desc_flags(vdev, desc_pa, i) & VRING_DESC_F_INDIRECT) {
if (vring_desc_len(vdev, desc_pa, i) % sizeof(VRingDesc)) {
- error_report("Invalid size for indirect buffer table");
- exit(1);
+ error_setg(errp, "Invalid size for indirect buffer table");
+ return -EINVAL;
}
/* loop over the indirect descriptor table */
@@ -518,15 +538,17 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
if (vring_desc_flags(vdev, desc_pa, i) & VRING_DESC_F_WRITE) {
if (elem->in_num >= ARRAY_SIZE(elem->in_sg)) {
- error_report("Too many write descriptors in indirect table");
- exit(1);
+ error_setg(errp,
+ "Too many write descriptors in indirect table");
+ return -EINVAL;
}
elem->in_addr[elem->in_num] = vring_desc_addr(vdev, desc_pa, i);
sg = &elem->in_sg[elem->in_num++];
} else {
if (elem->out_num >= ARRAY_SIZE(elem->out_sg)) {
- error_report("Too many read descriptors in indirect table");
- exit(1);
+ error_setg(errp,
+ "Too many read descriptors in indirect table");
+ return -EINVAL;
}
elem->out_addr[elem->out_num] = vring_desc_addr(vdev, desc_pa, i);
sg = &elem->out_sg[elem->out_num++];
@@ -536,20 +558,31 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
/* If we've got too many, that implies a descriptor loop. */
if ((elem->in_num + elem->out_num) > max) {
- error_report("Looped descriptor");
- exit(1);
+ error_setg(errp, "Looped descriptor");
+ return -EINVAL;
}
- i = virtqueue_next_desc(vdev, desc_pa, i, max, &error_abort);
- if (i == max) {
+ ret = virtqueue_next_desc(vdev, desc_pa, i, max, &local_err);
+ if (ret < 0) {
+ goto err;
+ } else if (ret == max) {
break;
}
+ i = ret;
}
/* Now map what we have collected */
virtqueue_map_sg(elem->in_sg, elem->in_addr, elem->in_num, 1,
- &error_abort);
+ &local_err);
+ if (local_err) {
+ ret = -EINVAL;
+ goto err;
+ }
virtqueue_map_sg(elem->out_sg, elem->out_addr, elem->out_num, 0,
- &error_abort);
+ &local_err);
+ if (local_err) {
+ ret = -EINVAL;
+ goto err_unmap;
+ }
elem->index = head;
@@ -557,6 +590,11 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num);
return elem->in_num + elem->out_num;
+err_unmap:
+ virtqueue_undo_map_sg(elem->in_sg, elem->in_addr, elem->in_num, 1);
+err:
+ error_propagate(errp, local_err);
+ return ret;
}
/* virtio device */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index a37ee64..c478f48 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -142,7 +142,7 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement
*elem,
void virtqueue_map_sg(struct iovec *sg, hwaddr *addr,
size_t num_sg, int is_write,
Error **errp);
-int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem);
+int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem, Error **errp);
int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
unsigned int out_bytes);
void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
--
1.9.3
