From patchwork Fri Apr 17 07:59:21 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fam Zheng X-Patchwork-Id: 461965 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 20EFB1402D5 for ; Fri, 17 Apr 2015 18:02:04 +1000 (AEST) Received: from localhost ([::1]:40010 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj1Dt-0001nJ-Nz for incoming@patchwork.ozlabs.org; Fri, 17 Apr 2015 04:02:01 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59837) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj1CE-0006TL-Vj for qemu-devel@nongnu.org; Fri, 17 Apr 2015 04:00:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yj1CB-0001Of-E8 for qemu-devel@nongnu.org; Fri, 17 Apr 2015 04:00:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37923) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj1CB-0001OM-4c for qemu-devel@nongnu.org; Fri, 17 Apr 2015 04:00:15 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3H80AU5021379 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 17 Apr 2015 04:00:11 -0400 Received: from ad.nay.redhat.com (dhcp-14-137.nay.redhat.com [10.66.14.137]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3H7xbeE004102; Fri, 17 Apr 2015 04:00:06 -0400 From: Fam Zheng To: qemu-devel@nongnu.org Date: Fri, 17 Apr 2015 15:59:21 +0800 Message-Id: <1429257573-7359-7-git-send-email-famz@redhat.com> In-Reply-To: <1429257573-7359-1-git-send-email-famz@redhat.com> References: <1429257573-7359-1-git-send-email-famz@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Kevin Wolf , "Michael S. Tsirkin" , "Aneesh Kumar K.V" , Stefan Hajnoczi , Amit Shah , Paolo Bonzini Subject: [Qemu-devel] [PATCH 06/18] virtio: Return error from virtqueue_pop X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 --- 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,