From 7f06fac3c93dc2fd3ba4d88db49f76cf40531739 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Mon, 9 Dec 2019 21:09:57 +0000 Subject: [PATCH 01/20] virtio: don't enable notifications during polling Virtqueue notifications are not necessary during polling, so we disable them. This allows the guest driver to avoid MMIO vmexits. Unfortunately the virtio-blk and virtio-scsi handler functions re-enable notifications, defeating this optimization. Fix virtio-blk and virtio-scsi emulation so they leave notifications disabled. The key thing to remember for correctness is that polling always checks one last time after ending its loop, therefore it's safe to lose the race when re-enabling notifications at the end of polling. There is a measurable performance improvement of 5-10% with the null-co block driver. Real-life storage configurations will see a smaller improvement because the MMIO vmexit overhead contributes less to latency. Signed-off-by: Stefan Hajnoczi Message-Id: <20191209210957.65087-1-stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- ...-enable-notifications-during-polling.patch | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 virtio-don-t-enable-notifications-during-polling.patch diff --git a/virtio-don-t-enable-notifications-during-polling.patch b/virtio-don-t-enable-notifications-during-polling.patch new file mode 100644 index 00000000..cb77429e --- /dev/null +++ b/virtio-don-t-enable-notifications-during-polling.patch @@ -0,0 +1,146 @@ +From 0592b1e444e8ef7f00fb04a637dba72b732b70e4 Mon Sep 17 00:00:00 2001 +From: Stefan Hajnoczi +Date: Mon, 9 Dec 2019 21:09:57 +0000 +Subject: [PATCH] virtio: don't enable notifications during polling + +Virtqueue notifications are not necessary during polling, so we disable +them. This allows the guest driver to avoid MMIO vmexits. +Unfortunately the virtio-blk and virtio-scsi handler functions re-enable +notifications, defeating this optimization. + +Fix virtio-blk and virtio-scsi emulation so they leave notifications +disabled. The key thing to remember for correctness is that polling +always checks one last time after ending its loop, therefore it's safe +to lose the race when re-enabling notifications at the end of polling. + +There is a measurable performance improvement of 5-10% with the null-co +block driver. Real-life storage configurations will see a smaller +improvement because the MMIO vmexit overhead contributes less to +latency. + +Signed-off-by: Stefan Hajnoczi +Message-Id: <20191209210957.65087-1-stefanha@redhat.com> +Reviewed-by: Michael S. Tsirkin +Signed-off-by: Michael S. Tsirkin +--- + hw/block/virtio-blk.c | 9 +++++++-- + hw/scsi/virtio-scsi.c | 9 +++++++-- + hw/virtio/virtio.c | 12 ++++++------ + include/hw/virtio/virtio.h | 1 + + 4 files changed, 21 insertions(+), 10 deletions(-) + +diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c +index 2db9804cfe..fbe2ed6779 100644 +--- a/hw/block/virtio-blk.c ++++ b/hw/block/virtio-blk.c +@@ -766,13 +766,16 @@ bool virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) + { + VirtIOBlockReq *req; + MultiReqBuffer mrb = {}; ++ bool suppress_notifications = virtio_queue_get_notification(vq); + bool progress = false; + + aio_context_acquire(blk_get_aio_context(s->blk)); + blk_io_plug(s->blk); + + do { +- virtio_queue_set_notification(vq, 0); ++ if (suppress_notifications) { ++ virtio_queue_set_notification(vq, 0); ++ } + + while ((req = virtio_blk_get_request(s, vq))) { + progress = true; +@@ -783,7 +786,9 @@ bool virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) + } + } + +- virtio_queue_set_notification(vq, 1); ++ if (suppress_notifications) { ++ virtio_queue_set_notification(vq, 1); ++ } + } while (!virtio_queue_empty(vq)); + + if (mrb.num_reqs) { +diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c +index 8b9e5e2b49..eddb13e7c6 100644 +--- a/hw/scsi/virtio-scsi.c ++++ b/hw/scsi/virtio-scsi.c +@@ -594,12 +594,15 @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) + { + VirtIOSCSIReq *req, *next; + int ret = 0; ++ bool suppress_notifications = virtio_queue_get_notification(vq); + bool progress = false; + + QTAILQ_HEAD(, VirtIOSCSIReq) reqs = QTAILQ_HEAD_INITIALIZER(reqs); + + do { +- virtio_queue_set_notification(vq, 0); ++ if (suppress_notifications) { ++ virtio_queue_set_notification(vq, 0); ++ } + + while ((req = virtio_scsi_pop_req(s, vq))) { + progress = true; +@@ -619,7 +622,9 @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) + } + } + +- virtio_queue_set_notification(vq, 1); ++ if (suppress_notifications) { ++ virtio_queue_set_notification(vq, 1); ++ } + } while (ret != -EINVAL && !virtio_queue_empty(vq)); + + QTAILQ_FOREACH_SAFE(req, &reqs, next, next) { +diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c +index 90971f4afa..daa8250332 100644 +--- a/hw/virtio/virtio.c ++++ b/hw/virtio/virtio.c +@@ -390,6 +390,11 @@ void virtio_queue_set_notification(VirtQueue *vq, int enable) + rcu_read_unlock(); + } + ++bool virtio_queue_get_notification(VirtQueue *vq) ++{ ++ return vq->notification; ++} ++ + int virtio_queue_ready(VirtQueue *vq) + { + return vq->vring.avail != 0; +@@ -2572,17 +2577,12 @@ static bool virtio_queue_host_notifier_aio_poll(void *opaque) + { + EventNotifier *n = opaque; + VirtQueue *vq = container_of(n, VirtQueue, host_notifier); +- bool progress; + + if (!vq->vring.desc || virtio_queue_empty(vq)) { + return false; + } + +- progress = virtio_queue_notify_aio_vq(vq); +- +- /* In case the handler function re-enabled notifications */ +- virtio_queue_set_notification(vq, 0); +- return progress; ++ return virtio_queue_notify_aio_vq(vq); + } + + static void virtio_queue_host_notifier_aio_poll_end(EventNotifier *n) +diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h +index ca2fbaeb35..7394715407 100644 +--- a/include/hw/virtio/virtio.h ++++ b/include/hw/virtio/virtio.h +@@ -229,6 +229,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id); + + void virtio_notify_config(VirtIODevice *vdev); + ++bool virtio_queue_get_notification(VirtQueue *vq); + void virtio_queue_set_notification(VirtQueue *vq, int enable); + + int virtio_queue_ready(VirtQueue *vq); +-- +2.27.0 + -- Gitee From c478f4c43c37968f556de7d310bb3e3d51d764e6 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Wed, 18 Dec 2019 11:30:12 +0000 Subject: [PATCH 02/20] usbredir: Prevent recursion in usbredir_write I've got a case where usbredir_write manages to call back into itself via spice; this patch causes the recursion to fail (0 bytes) the write; this seems to avoid the deadlock I was previously seeing. I can't say I fully understand the interaction of usbredir and spice; but there are a few similar guards in spice and usbredir to catch other cases especially onces also related to spice_server_char_device_wakeup This case seems to be triggered by repeated migration+repeated reconnection of the viewer; but my debugging suggests the migration finished before this hits. The backtrace of the hang looks like: reds_handle_ticket reds_handle_other_links reds_channel_do_link red_channel_connect spicevmc_connect usbredir_create_parser usbredirparser_do_write usbredir_write qemu_chr_fe_write qemu_chr_write qemu_chr_write_buffer spice_chr_write spice_server_char_device_wakeup red_char_device_wakeup red_char_device_write_to_device vmc_write usbredirparser_do_write usbredir_write qemu_chr_fe_write qemu_chr_write qemu_chr_write_buffer qemu_mutex_lock_impl and we fail as we lang through qemu_chr_write_buffer's lock twice. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1752320 Signed-off-by: Dr. David Alan Gilbert Message-Id: <20191218113012.13331-1-dgilbert@redhat.com> Signed-off-by: Gerd Hoffmann --- ...-Prevent-recursion-in-usbredir_write.patch | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 usbredir-Prevent-recursion-in-usbredir_write.patch diff --git a/usbredir-Prevent-recursion-in-usbredir_write.patch b/usbredir-Prevent-recursion-in-usbredir_write.patch new file mode 100644 index 00000000..29eb50f2 --- /dev/null +++ b/usbredir-Prevent-recursion-in-usbredir_write.patch @@ -0,0 +1,90 @@ +From 30203c01fa1bb2a7b92575683f85695a2d420b38 Mon Sep 17 00:00:00 2001 +From: "Dr. David Alan Gilbert" +Date: Wed, 18 Dec 2019 11:30:12 +0000 +Subject: [PATCH] usbredir: Prevent recursion in usbredir_write + +I've got a case where usbredir_write manages to call back into itself +via spice; this patch causes the recursion to fail (0 bytes) the write; +this seems to avoid the deadlock I was previously seeing. + +I can't say I fully understand the interaction of usbredir and spice; +but there are a few similar guards in spice and usbredir +to catch other cases especially onces also related to spice_server_char_device_wakeup + +This case seems to be triggered by repeated migration+repeated +reconnection of the viewer; but my debugging suggests the migration +finished before this hits. + +The backtrace of the hang looks like: + reds_handle_ticket + reds_handle_other_links + reds_channel_do_link + red_channel_connect + spicevmc_connect + usbredir_create_parser + usbredirparser_do_write + usbredir_write + qemu_chr_fe_write + qemu_chr_write + qemu_chr_write_buffer + spice_chr_write + spice_server_char_device_wakeup + red_char_device_wakeup + red_char_device_write_to_device + vmc_write + usbredirparser_do_write + usbredir_write + qemu_chr_fe_write + qemu_chr_write + qemu_chr_write_buffer + qemu_mutex_lock_impl + +and we fail as we lang through qemu_chr_write_buffer's lock +twice. + +Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1752320 + +Signed-off-by: Dr. David Alan Gilbert +Message-Id: <20191218113012.13331-1-dgilbert@redhat.com> +Signed-off-by: Gerd Hoffmann +--- + hw/usb/redirect.c | 9 +++++++++ + 1 file changed, 9 insertions(+) + +diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c +index 9764a57987..3cf82589ed 100644 +--- a/hw/usb/redirect.c ++++ b/hw/usb/redirect.c +@@ -109,6 +109,7 @@ struct USBRedirDevice { + /* Properties */ + CharBackend cs; + bool enable_streams; ++ bool in_write; + uint8_t debug; + int32_t bootindex; + char *filter_str; +@@ -286,6 +287,13 @@ static int usbredir_write(void *priv, uint8_t *data, int count) + return 0; + } + ++ /* Recursion check */ ++ if (dev->in_write) { ++ DPRINTF("usbredir_write recursion\n"); ++ return 0; ++ } ++ dev->in_write = true; ++ + r = qemu_chr_fe_write(&dev->cs, data, count); + if (r < count) { + if (!dev->watch) { +@@ -296,6 +304,7 @@ static int usbredir_write(void *priv, uint8_t *data, int count) + r = 0; + } + } ++ dev->in_write = false; + return r; + } + +-- +2.27.0 + -- Gitee From 51fcddf40b6e76416b1a880acdc9ee47bd495539 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Tue, 7 Jan 2020 09:36:06 +0100 Subject: [PATCH 03/20] xhci: recheck slot status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Factor out slot status check into a helper function. Add an additional check after completing transfers. This is needed in case a guest queues multiple transfers in a row and a device unplug happens while qemu processes them. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1786413 Signed-off-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-id: 20200107083606.12393-1-kraxel@redhat.com --- xhci-recheck-slot-status.patch | 64 ++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 xhci-recheck-slot-status.patch diff --git a/xhci-recheck-slot-status.patch b/xhci-recheck-slot-status.patch new file mode 100644 index 00000000..d05c3c83 --- /dev/null +++ b/xhci-recheck-slot-status.patch @@ -0,0 +1,64 @@ +From 33d6a2bc0e432a85962b71bcb2c3b5eec39bf436 Mon Sep 17 00:00:00 2001 +From: Gerd Hoffmann +Date: Tue, 7 Jan 2020 09:36:06 +0100 +Subject: [PATCH] xhci: recheck slot status +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Factor out slot status check into a helper function. Add an additional +check after completing transfers. This is needed in case a guest +queues multiple transfers in a row and a device unplug happens while +qemu processes them. + +Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1786413 +Signed-off-by: Gerd Hoffmann +Reviewed-by: Philippe Mathieu-Daudé +Message-id: 20200107083606.12393-1-kraxel@redhat.com +--- + hw/usb/hcd-xhci.c | 15 ++++++++++++--- + 1 file changed, 12 insertions(+), 3 deletions(-) + +diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c +index 24565de1d1..4b42f53b9c 100644 +--- a/hw/usb/hcd-xhci.c ++++ b/hw/usb/hcd-xhci.c +@@ -1860,6 +1860,13 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, + xhci_kick_epctx(epctx, streamid); + } + ++static bool xhci_slot_ok(XHCIState *xhci, int slotid) ++{ ++ return (xhci->slots[slotid - 1].uport && ++ xhci->slots[slotid - 1].uport->dev && ++ xhci->slots[slotid - 1].uport->dev->attached); ++} ++ + static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid) + { + XHCIState *xhci = epctx->xhci; +@@ -1877,9 +1884,7 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid) + + /* If the device has been detached, but the guest has not noticed this + yet the 2 above checks will succeed, but we must NOT continue */ +- if (!xhci->slots[epctx->slotid - 1].uport || +- !xhci->slots[epctx->slotid - 1].uport->dev || +- !xhci->slots[epctx->slotid - 1].uport->dev->attached) { ++ if (!xhci_slot_ok(xhci, epctx->slotid)) { + return; + } + +@@ -1986,6 +1991,10 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid) + } else { + xhci_fire_transfer(xhci, xfer, epctx); + } ++ if (!xhci_slot_ok(xhci, epctx->slotid)) { ++ /* surprise removal -> stop processing */ ++ break; ++ } + if (xfer->complete) { + /* update ring dequeue ptr */ + xhci_set_ep_state(xhci, epctx, stctx, epctx->state); +-- +2.27.0 + -- Gitee From 954d314d88d1213dc39c4802837b5ad4be23af18 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 16 Jan 2020 20:24:13 +0000 Subject: [PATCH 04/20] vhost: Add names to section rounded warning Add the memory region names to section rounding/alignment warnings. Signed-off-by: Dr. David Alan Gilbert Message-Id: <20200116202414.157959-2-dgilbert@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- ...Add-names-to-section-rounded-warning.patch | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 vhost-Add-names-to-section-rounded-warning.patch diff --git a/vhost-Add-names-to-section-rounded-warning.patch b/vhost-Add-names-to-section-rounded-warning.patch new file mode 100644 index 00000000..09c36eda --- /dev/null +++ b/vhost-Add-names-to-section-rounded-warning.patch @@ -0,0 +1,37 @@ +From 437a9d2c7e48495ffc467808eece045579956c79 Mon Sep 17 00:00:00 2001 +From: "Dr. David Alan Gilbert" +Date: Thu, 16 Jan 2020 20:24:13 +0000 +Subject: [PATCH] vhost: Add names to section rounded warning + +Add the memory region names to section rounding/alignment +warnings. + +Signed-off-by: Dr. David Alan Gilbert +Message-Id: <20200116202414.157959-2-dgilbert@redhat.com> +Reviewed-by: Michael S. Tsirkin +Signed-off-by: Michael S. Tsirkin +--- + hw/virtio/vhost.c | 7 ++++--- + 1 file changed, 4 insertions(+), 3 deletions(-) + +diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c +index 9c16f0d107..ae61c33c15 100644 +--- a/hw/virtio/vhost.c ++++ b/hw/virtio/vhost.c +@@ -591,9 +591,10 @@ static void vhost_region_add_section(struct vhost_dev *dev, + * match up in the same RAMBlock if they do. + */ + if (mrs_gpa < prev_gpa_start) { +- error_report("%s:Section rounded to %"PRIx64 +- " prior to previous %"PRIx64, +- __func__, mrs_gpa, prev_gpa_start); ++ error_report("%s:Section '%s' rounded to %"PRIx64 ++ " prior to previous '%s' %"PRIx64, ++ __func__, section->mr->name, mrs_gpa, ++ prev_sec->mr->name, prev_gpa_start); + /* A way to cleanly fail here would be better */ + return; + } +-- +2.27.0 + -- Gitee From eb5868b7f19b3fd34a7b8ba5a1b280ee7e491335 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 7 Feb 2019 18:22:40 +0000 Subject: [PATCH 05/20] vhost-user: Print unexpected slave message types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we receive an unexpected message type on the slave fd, print the type. Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Dr. David Alan Gilbert --- ...Print-unexpected-slave-message-types.patch | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 vhost-user-Print-unexpected-slave-message-types.patch diff --git a/vhost-user-Print-unexpected-slave-message-types.patch b/vhost-user-Print-unexpected-slave-message-types.patch new file mode 100644 index 00000000..4287428e --- /dev/null +++ b/vhost-user-Print-unexpected-slave-message-types.patch @@ -0,0 +1,35 @@ +From 6e084ff24ad73eb4f7541573c6097013f5b94959 Mon Sep 17 00:00:00 2001 +From: "Dr. David Alan Gilbert" +Date: Thu, 7 Feb 2019 18:22:40 +0000 +Subject: [PATCH] vhost-user: Print unexpected slave message types +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +When we receive an unexpected message type on the slave fd, print +the type. + +Signed-off-by: Dr. David Alan Gilbert +Reviewed-by: Daniel P. Berrangé +Reviewed-by: Philippe Mathieu-Daudé +Signed-off-by: Dr. David Alan Gilbert +--- + hw/virtio/vhost-user.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c +index 4ca5b2551e..f012774210 100644 +--- a/hw/virtio/vhost-user.c ++++ b/hw/virtio/vhost-user.c +@@ -1054,7 +1054,7 @@ static void slave_read(void *opaque) + fd[0]); + break; + default: +- error_report("Received unexpected msg type."); ++ error_report("Received unexpected msg type: %d.", hdr.request); + ret = -EINVAL; + } + +-- +2.27.0 + -- Gitee From c597707bd4b2da2e7caa420cbb481e95a01e4f1b Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Fri, 1 Mar 2019 11:18:30 +0000 Subject: [PATCH 06/20] contrib/libvhost-user: Protect slave fd with mutex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In future patches we'll be performing commands on the slave-fd driven by commands on queues, since those queues will be driven by individual threads we need to make sure they don't attempt to use the slave-fd for multiple commands in parallel. Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Daniel P. Berrangé Signed-off-by: Dr. David Alan Gilbert --- ...ost-user-Protect-slave-fd-with-mutex.patch | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 contrib-libvhost-user-Protect-slave-fd-with-mutex.patch diff --git a/contrib-libvhost-user-Protect-slave-fd-with-mutex.patch b/contrib-libvhost-user-Protect-slave-fd-with-mutex.patch new file mode 100644 index 00000000..44fc4283 --- /dev/null +++ b/contrib-libvhost-user-Protect-slave-fd-with-mutex.patch @@ -0,0 +1,121 @@ +From f076af734a5964c3e48b2d223130f855b86f40e5 Mon Sep 17 00:00:00 2001 +From: "Dr. David Alan Gilbert" +Date: Fri, 1 Mar 2019 11:18:30 +0000 +Subject: [PATCH] contrib/libvhost-user: Protect slave fd with mutex +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +In future patches we'll be performing commands on the slave-fd driven +by commands on queues, since those queues will be driven by individual +threads we need to make sure they don't attempt to use the slave-fd +for multiple commands in parallel. + +Signed-off-by: Dr. David Alan Gilbert +Reviewed-by: Daniel P. Berrangé +Signed-off-by: Dr. David Alan Gilbert +--- + contrib/libvhost-user/libvhost-user.c | 24 ++++++++++++++++++++---- + contrib/libvhost-user/libvhost-user.h | 3 +++ + 2 files changed, 23 insertions(+), 4 deletions(-) + +diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c +index cb5f5770e4..fb75837032 100644 +--- a/contrib/libvhost-user/libvhost-user.c ++++ b/contrib/libvhost-user/libvhost-user.c +@@ -387,26 +387,37 @@ vu_send_reply(VuDev *dev, int conn_fd, VhostUserMsg *vmsg) + return vu_message_write(dev, conn_fd, vmsg); + } + ++/* ++ * Processes a reply on the slave channel. ++ * Entered with slave_mutex held and releases it before exit. ++ * Returns true on success. ++ */ + static bool + vu_process_message_reply(VuDev *dev, const VhostUserMsg *vmsg) + { + VhostUserMsg msg_reply; ++ bool result = false; + + if ((vmsg->flags & VHOST_USER_NEED_REPLY_MASK) == 0) { +- return true; ++ result = true; ++ goto out; + } + + if (!vu_message_read(dev, dev->slave_fd, &msg_reply)) { +- return false; ++ goto out; + } + + if (msg_reply.request != vmsg->request) { + DPRINT("Received unexpected msg type. Expected %d received %d", + vmsg->request, msg_reply.request); +- return false; ++ goto out; + } + +- return msg_reply.payload.u64 == 0; ++ result = msg_reply.payload.u64 == 0; ++ ++out: ++ pthread_mutex_unlock(&dev->slave_mutex); ++ return result; + } + + /* Kick the log_call_fd if required. */ +@@ -1102,10 +1113,13 @@ bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd, + return false; + } + ++ pthread_mutex_lock(&dev->slave_mutex); + if (!vu_message_write(dev, dev->slave_fd, &vmsg)) { ++ pthread_mutex_unlock(&dev->slave_mutex); + return false; + } + ++ /* Also unlocks the slave_mutex */ + return vu_process_message_reply(dev, &vmsg); + } + +@@ -1625,6 +1639,7 @@ vu_deinit(VuDev *dev) + close(dev->slave_fd); + dev->slave_fd = -1; + } ++ pthread_mutex_destroy(&dev->slave_mutex); + + if (dev->sock != -1) { + close(dev->sock); +@@ -1660,6 +1675,7 @@ vu_init(VuDev *dev, + dev->remove_watch = remove_watch; + dev->iface = iface; + dev->log_call_fd = -1; ++ pthread_mutex_init(&dev->slave_mutex, NULL); + dev->slave_fd = -1; + dev->max_queues = max_queues; + +diff --git a/contrib/libvhost-user/libvhost-user.h b/contrib/libvhost-user/libvhost-user.h +index 46b600799b..1844b6f8d4 100644 +--- a/contrib/libvhost-user/libvhost-user.h ++++ b/contrib/libvhost-user/libvhost-user.h +@@ -19,6 +19,7 @@ + #include + #include + #include ++#include + #include "standard-headers/linux/virtio_ring.h" + + /* Based on qemu/hw/virtio/vhost-user.c */ +@@ -355,6 +356,8 @@ struct VuDev { + VuVirtq *vq; + VuDevInflightInfo inflight_info; + int log_call_fd; ++ /* Must be held while using slave_fd */ ++ pthread_mutex_t slave_mutex; + int slave_fd; + uint64_t log_size; + uint8_t *log_table; +-- +2.27.0 + -- Gitee From 7c2cfdc326a1b810536850f340208fd42e7690aa Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Mon, 12 Aug 2019 17:35:19 +0100 Subject: [PATCH 07/20] libvhost-user: Fix some memtable remap cases If a new setmemtable command comes in once the vhost threads are running, it will remap the guests address space and the threads will now be looking in the wrong place. Fortunately we're running this command under lock, so we can update the queue mappings so that threads will look in the new-right place. Note: This doesn't fix things that the threads might be doing without a lock (e.g. a readv/writev!) That's for another time. Signed-off-by: Dr. David Alan Gilbert --- ...t-user-Fix-some-memtable-remap-cases.patch | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 libvhost-user-Fix-some-memtable-remap-cases.patch diff --git a/libvhost-user-Fix-some-memtable-remap-cases.patch b/libvhost-user-Fix-some-memtable-remap-cases.patch new file mode 100644 index 00000000..4f4d0c9f --- /dev/null +++ b/libvhost-user-Fix-some-memtable-remap-cases.patch @@ -0,0 +1,101 @@ +From 8fa62daca5978e77ed690797a882c3d0aad8d0d4 Mon Sep 17 00:00:00 2001 +From: "Dr. David Alan Gilbert" +Date: Mon, 12 Aug 2019 17:35:19 +0100 +Subject: [PATCH] libvhost-user: Fix some memtable remap cases + +If a new setmemtable command comes in once the vhost threads are +running, it will remap the guests address space and the threads +will now be looking in the wrong place. + +Fortunately we're running this command under lock, so we can +update the queue mappings so that threads will look in the new-right +place. + +Note: This doesn't fix things that the threads might be doing +without a lock (e.g. a readv/writev!) That's for another time. + +Signed-off-by: Dr. David Alan Gilbert +--- + contrib/libvhost-user/libvhost-user.c | 33 ++++++++++++++++++++------- + contrib/libvhost-user/libvhost-user.h | 3 +++ + 2 files changed, 28 insertions(+), 8 deletions(-) + +diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c +index fb75837032..164e6d1df8 100644 +--- a/contrib/libvhost-user/libvhost-user.c ++++ b/contrib/libvhost-user/libvhost-user.c +@@ -559,6 +559,21 @@ vu_reset_device_exec(VuDev *dev, VhostUserMsg *vmsg) + return false; + } + ++static bool ++map_ring(VuDev *dev, VuVirtq *vq) ++{ ++ vq->vring.desc = qva_to_va(dev, vq->vra.desc_user_addr); ++ vq->vring.used = qva_to_va(dev, vq->vra.used_user_addr); ++ vq->vring.avail = qva_to_va(dev, vq->vra.avail_user_addr); ++ ++ DPRINT("Setting virtq addresses:\n"); ++ DPRINT(" vring_desc at %p\n", vq->vring.desc); ++ DPRINT(" vring_used at %p\n", vq->vring.used); ++ DPRINT(" vring_avail at %p\n", vq->vring.avail); ++ ++ return !(vq->vring.desc && vq->vring.used && vq->vring.avail); ++} ++ + static bool + vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg) + { +@@ -762,6 +777,14 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg) + close(vmsg->fds[i]); + } + ++ for (i = 0; i < dev->max_queues; i++) { ++ if (dev->vq[i].vring.desc) { ++ if (map_ring(dev, &dev->vq[i])) { ++ vu_panic(dev, "remaping queue %d during setmemtable", i); ++ } ++ } ++ } ++ + return false; + } + +@@ -848,18 +871,12 @@ vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg) + DPRINT(" avail_user_addr: 0x%016" PRIx64 "\n", vra->avail_user_addr); + DPRINT(" log_guest_addr: 0x%016" PRIx64 "\n", vra->log_guest_addr); + ++ vq->vra = *vra; + vq->vring.flags = vra->flags; +- vq->vring.desc = qva_to_va(dev, vra->desc_user_addr); +- vq->vring.used = qva_to_va(dev, vra->used_user_addr); +- vq->vring.avail = qva_to_va(dev, vra->avail_user_addr); + vq->vring.log_guest_addr = vra->log_guest_addr; + +- DPRINT("Setting virtq addresses:\n"); +- DPRINT(" vring_desc at %p\n", vq->vring.desc); +- DPRINT(" vring_used at %p\n", vq->vring.used); +- DPRINT(" vring_avail at %p\n", vq->vring.avail); + +- if (!(vq->vring.desc && vq->vring.used && vq->vring.avail)) { ++ if (map_ring(dev, vq)) { + vu_panic(dev, "Invalid vring_addr message"); + return false; + } +diff --git a/contrib/libvhost-user/libvhost-user.h b/contrib/libvhost-user/libvhost-user.h +index 1844b6f8d4..5cb7708559 100644 +--- a/contrib/libvhost-user/libvhost-user.h ++++ b/contrib/libvhost-user/libvhost-user.h +@@ -327,6 +327,9 @@ typedef struct VuVirtq { + int err_fd; + unsigned int enable; + bool started; ++ ++ /* Guest addresses of our ring */ ++ struct vhost_vring_addr vra; + } VuVirtq; + + enum VuWatchCondtion { +-- +2.27.0 + -- Gitee From b5b6f23aae65d4696a10d017f6707daddfd66a14 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Wed, 4 Dec 2019 20:43:43 +0100 Subject: [PATCH 08/20] xics: Don't deassert outputs The correct way to do this is to deassert the input pins on the CPU side. This is the case since a previous change. Signed-off-by: Greg Kurz Message-Id: <157548862298.3650476.1228720391270249433.stgit@bahia.lan> Signed-off-by: David Gibson --- xics-Don-t-deassert-outputs.patch | 32 +++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 xics-Don-t-deassert-outputs.patch diff --git a/xics-Don-t-deassert-outputs.patch b/xics-Don-t-deassert-outputs.patch new file mode 100644 index 00000000..083a9a2e --- /dev/null +++ b/xics-Don-t-deassert-outputs.patch @@ -0,0 +1,32 @@ +From 5b137b37ef7c4941200798cca99200e80ef17a01 Mon Sep 17 00:00:00 2001 +From: Greg Kurz +Date: Wed, 4 Dec 2019 20:43:43 +0100 +Subject: [PATCH] xics: Don't deassert outputs + +The correct way to do this is to deassert the input pins on the CPU side. +This is the case since a previous change. + +Signed-off-by: Greg Kurz +Message-Id: <157548862298.3650476.1228720391270249433.stgit@bahia.lan> +Signed-off-by: David Gibson +--- + hw/intc/xics.c | 3 --- + 1 file changed, 3 deletions(-) + +diff --git a/hw/intc/xics.c b/hw/intc/xics.c +index faa976e2f8..d2d377fc85 100644 +--- a/hw/intc/xics.c ++++ b/hw/intc/xics.c +@@ -303,9 +303,6 @@ static void icp_reset_handler(void *dev) + icp->pending_priority = 0xff; + icp->mfrr = 0xff; + +- /* Make all outputs are deasserted */ +- qemu_set_irq(icp->output, 0); +- + if (kvm_irqchip_in_kernel()) { + Error *local_err = NULL; + +-- +2.27.0 + -- Gitee From d951287a0641bc3e7e1719c285e9af58aef4da35 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 5 Dec 2019 19:33:39 -0300 Subject: [PATCH 09/20] i386: Resolve CPU models to v1 by default When using `query-cpu-definitions` using `-machine none`, QEMU is resolving all CPU models to their latest versions. The actual CPU model version being used by another machine type (e.g. `pc-q35-4.0`) might be different. In theory, this was OK because the correct CPU model version is returned when using the correct `-machine` argument. Except that in practice, this breaks libvirt expectations: libvirt always use `-machine none` when checking if a CPU model is runnable, because runnability is not expected to be affected when the machine type is changed. For example, when running on a Haswell host without TSX, Haswell-v4 is runnable, but Haswell-v1 is not. On those hosts, `query-cpu-definitions` says Haswell is runnable if using `-machine none`, but Haswell is actually not runnable using any of the `pc-*` machine types (because they resolve Haswell to Haswell-v1). In other words, we're breaking the "runnability guarantee" we promised to not break for a few releases (see qemu-deprecated.texi). To address this issue, change the default CPU model version to v1 on all machine types, so we make `query-cpu-definitions` output when using `-machine none` match the results when using `pc-*`. This will change in the future (the plan is to always return the latest CPU model version if using `-machine none`), but only after giving libvirt the opportunity to adapt. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1779078 Signed-off-by: Eduardo Habkost Message-Id: <20191205223339.764534-1-ehabkost@redhat.com> Signed-off-by: Eduardo Habkost --- ...-Resolve-CPU-models-to-v1-by-default.patch | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 i386-Resolve-CPU-models-to-v1-by-default.patch diff --git a/i386-Resolve-CPU-models-to-v1-by-default.patch b/i386-Resolve-CPU-models-to-v1-by-default.patch new file mode 100644 index 00000000..f8c63158 --- /dev/null +++ b/i386-Resolve-CPU-models-to-v1-by-default.patch @@ -0,0 +1,81 @@ +From 6a5e994c1dec959143f6d3f83169a7adcb173fc4 Mon Sep 17 00:00:00 2001 +From: Eduardo Habkost +Date: Thu, 5 Dec 2019 19:33:39 -0300 +Subject: [PATCH] i386: Resolve CPU models to v1 by default + +When using `query-cpu-definitions` using `-machine none`, +QEMU is resolving all CPU models to their latest versions. The +actual CPU model version being used by another machine type (e.g. +`pc-q35-4.0`) might be different. + +In theory, this was OK because the correct CPU model +version is returned when using the correct `-machine` argument. + +Except that in practice, this breaks libvirt expectations: +libvirt always use `-machine none` when checking if a CPU model +is runnable, because runnability is not expected to be affected +when the machine type is changed. + +For example, when running on a Haswell host without TSX, +Haswell-v4 is runnable, but Haswell-v1 is not. On those hosts, +`query-cpu-definitions` says Haswell is runnable if using +`-machine none`, but Haswell is actually not runnable using any +of the `pc-*` machine types (because they resolve Haswell to +Haswell-v1). In other words, we're breaking the "runnability +guarantee" we promised to not break for a few releases (see +qemu-deprecated.texi). + +To address this issue, change the default CPU model version to v1 +on all machine types, so we make `query-cpu-definitions` output +when using `-machine none` match the results when using `pc-*`. +This will change in the future (the plan is to always return the +latest CPU model version if using `-machine none`), but only +after giving libvirt the opportunity to adapt. + +Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1779078 +Signed-off-by: Eduardo Habkost +Message-Id: <20191205223339.764534-1-ehabkost@redhat.com> +Signed-off-by: Eduardo Habkost +--- + qemu-deprecated.texi | 8 ++++++++ + target/i386/cpu.c | 8 +++++++- + 2 files changed, 15 insertions(+), 1 deletion(-) + +diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi +index fff07bb2a3..719ac23d72 100644 +--- a/qemu-deprecated.texi ++++ b/qemu-deprecated.texi +@@ -331,3 +331,11 @@ existing CPU models. Management software that needs runnability + guarantees must resolve the CPU model aliases using te + ``alias-of'' field returned by the ``query-cpu-definitions'' QMP + command. ++ ++While those guarantees are kept, the return value of ++``query-cpu-definitions'' will have existing CPU model aliases ++point to a version that doesn't break runnability guarantees ++(specifically, version 1 of those CPU models). In future QEMU ++versions, aliases will point to newer CPU model versions ++depending on the machine type, so management software must ++resolve CPU model aliases before starting a virtual machine. +diff --git a/target/i386/cpu.c b/target/i386/cpu.c +index e0f3a2dd99..22e0e89718 100644 +--- a/target/i386/cpu.c ++++ b/target/i386/cpu.c +@@ -3933,7 +3933,13 @@ static PropValue tcg_default_props[] = { + }; + + +-X86CPUVersion default_cpu_version = CPU_VERSION_LATEST; ++/* ++ * We resolve CPU model aliases using -v1 when using "-machine ++ * none", but this is just for compatibility while libvirt isn't ++ * adapted to resolve CPU model versions before creating VMs. ++ * See "Runnability guarantee of CPU models" at * qemu-deprecated.texi. ++ */ ++X86CPUVersion default_cpu_version = 1; + + void x86_cpu_set_default_version(X86CPUVersion version) + { +-- +2.27.0 + -- Gitee From 7cdb42a8eb051a3c703cecad9e904ecb395d680e Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Wed, 21 Jul 2021 21:27:20 +0800 Subject: [PATCH 10/20] spec: Update patch and changelog with !164 qemu-4.1 bugfix !164 virtio: don't enable notifications during polling usbredir: Prevent recursion in usbredir_write xhci: recheck slot status vhost: Add names to section rounded warning vhost-user: Print unexpected slave message types contrib/libvhost-user: Protect slave fd with mutex libvhost-user: Fix some memtable remap cases xics: Don't deassert outputs i386: Resolve CPU models to v1 by default Signed-off-by: Chen Qun --- qemu.spec | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/qemu.spec b/qemu.spec index d6512a9b..7187f2bc 100644 --- a/qemu.spec +++ b/qemu.spec @@ -367,6 +367,15 @@ Patch0354: i386-cpu-Don-t-add-unavailable_features-to-env-user_.patch Patch0355: target-i386-do-not-set-unsupported-VMX-secondary-exe.patch Patch0356: migration-fix-multifd_send_pages-next-channel.patch Patch0357: migration-Make-sure-that-we-don-t-call-write-in-case.patch +Patch0358: virtio-don-t-enable-notifications-during-polling.patch +Patch0359: usbredir-Prevent-recursion-in-usbredir_write.patch +Patch0360: xhci-recheck-slot-status.patch +Patch0361: vhost-Add-names-to-section-rounded-warning.patch +Patch0362: vhost-user-Print-unexpected-slave-message-types.patch +Patch0363: contrib-libvhost-user-Protect-slave-fd-with-mutex.patch +Patch0364: libvhost-user-Fix-some-memtable-remap-cases.patch +Patch0365: xics-Don-t-deassert-outputs.patch +Patch0366: i386-Resolve-CPU-models-to-v1-by-default.patch BuildRequires: flex BuildRequires: gcc @@ -761,6 +770,17 @@ getent passwd qemu >/dev/null || \ %endif %changelog +* Wed Jul 21 2021 Chen Qun +- virtio: don't enable notifications during polling +- usbredir: Prevent recursion in usbredir_write +- xhci: recheck slot status +- vhost: Add names to section rounded warning +- vhost-user: Print unexpected slave message types +- contrib/libvhost-user: Protect slave fd with mutex +- libvhost-user: Fix some memtable remap cases +- xics: Don't deassert outputs +- i386: Resolve CPU models to v1 by default + * Wed Jul 21 2021 imxcc - target/i386: handle filtered_features in a new function mark_unavailable_features - target/i386: introduce generic feature dependency mechanism -- Gitee From a3094362d26551724c8e7d3e52fb3fd041d05ac7 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 28 May 2020 14:27:36 +0100 Subject: [PATCH 11/20] block/curl: HTTP header fields allow whitespace around values RH-Author: Richard Jones Message-id: <20200528142737.17318-2-rjones@redhat.com> Patchwork-id: 96894 O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 1/2] block/curl: HTTP header fields allow whitespace around values Bugzilla: 1841038 RH-Acked-by: Eric Blake RH-Acked-by: Max Reitz RH-Acked-by: Danilo de Paula From: David Edmondson RFC 7230 section 3.2 indicates that whitespace is permitted between the field name and field value and after the field value. Signed-off-by: David Edmondson Message-Id: <20200224101310.101169-2-david.edmondson@oracle.com> Reviewed-by: Max Reitz Signed-off-by: Max Reitz (cherry picked from commit 7788a319399f17476ff1dd43164c869e320820a2) Signed-off-by: Danilo C. L. de Paula --- ...header-fields-allow-whitespace-aroun.patch | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 block-curl-HTTP-header-fields-allow-whitespace-aroun.patch diff --git a/block-curl-HTTP-header-fields-allow-whitespace-aroun.patch b/block-curl-HTTP-header-fields-allow-whitespace-aroun.patch new file mode 100644 index 00000000..6f3aade4 --- /dev/null +++ b/block-curl-HTTP-header-fields-allow-whitespace-aroun.patch @@ -0,0 +1,75 @@ +From c8fd37c06fd24d1242629dda329dd16bea20f319 Mon Sep 17 00:00:00 2001 +From: Richard Jones +Date: Thu, 28 May 2020 14:27:36 +0100 +Subject: [PATCH] block/curl: HTTP header fields allow whitespace around values + +RH-Author: Richard Jones +Message-id: <20200528142737.17318-2-rjones@redhat.com> +Patchwork-id: 96894 +O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 1/2] block/curl: HTTP header fields allow whitespace around values +Bugzilla: 1841038 +RH-Acked-by: Eric Blake +RH-Acked-by: Max Reitz +RH-Acked-by: Danilo de Paula + +From: David Edmondson + +RFC 7230 section 3.2 indicates that whitespace is permitted between +the field name and field value and after the field value. + +Signed-off-by: David Edmondson +Message-Id: <20200224101310.101169-2-david.edmondson@oracle.com> +Reviewed-by: Max Reitz +Signed-off-by: Max Reitz +(cherry picked from commit 7788a319399f17476ff1dd43164c869e320820a2) +Signed-off-by: Danilo C. L. de Paula +--- + block/curl.c | 31 +++++++++++++++++++++++++++---- + 1 file changed, 27 insertions(+), 4 deletions(-) + +diff --git a/block/curl.c b/block/curl.c +index d4c8e94f3e..bfabe7eabd 100644 +--- a/block/curl.c ++++ b/block/curl.c +@@ -212,11 +212,34 @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque) + { + BDRVCURLState *s = opaque; + size_t realsize = size * nmemb; +- const char *accept_line = "Accept-Ranges: bytes"; ++ const char *header = (char *)ptr; ++ const char *end = header + realsize; ++ const char *accept_ranges = "Accept-Ranges:"; ++ const char *bytes = "bytes"; + +- if (realsize >= strlen(accept_line) +- && strncmp((char *)ptr, accept_line, strlen(accept_line)) == 0) { +- s->accept_range = true; ++ if (realsize >= strlen(accept_ranges) ++ && strncmp(header, accept_ranges, strlen(accept_ranges)) == 0) { ++ ++ char *p = strchr(header, ':') + 1; ++ ++ /* Skip whitespace between the header name and value. */ ++ while (p < end && *p && g_ascii_isspace(*p)) { ++ p++; ++ } ++ ++ if (end - p >= strlen(bytes) ++ && strncmp(p, bytes, strlen(bytes)) == 0) { ++ ++ /* Check that there is nothing but whitespace after the value. */ ++ p += strlen(bytes); ++ while (p < end && *p && g_ascii_isspace(*p)) { ++ p++; ++ } ++ ++ if (p == end || !*p) { ++ s->accept_range = true; ++ } ++ } + } + + return realsize; +-- +2.27.0 + -- Gitee From 03e474ad1957c709f246bbf12ece8677554e5083 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 28 May 2020 14:27:37 +0100 Subject: [PATCH 12/20] block/curl: HTTP header field names are case insensitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RH-Author: Richard Jones Message-id: <20200528142737.17318-3-rjones@redhat.com> Patchwork-id: 96895 O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 2/2] block/curl: HTTP header field names are case insensitive Bugzilla: 1841038 RH-Acked-by: Eric Blake RH-Acked-by: Max Reitz RH-Acked-by: Philippe Mathieu-Daudé From: David Edmondson RFC 7230 section 3.2 indicates that HTTP header field names are case insensitive. Signed-off-by: David Edmondson Message-Id: <20200224101310.101169-3-david.edmondson@oracle.com> Reviewed-by: Max Reitz Signed-off-by: Max Reitz (cherry picked from commit 69032253c33ae1774233c63cedf36d32242a85fc) Signed-off-by: Danilo C. L. de Paula --- ...header-field-names-are-case-insensit.patch | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 block-curl-HTTP-header-field-names-are-case-insensit.patch diff --git a/block-curl-HTTP-header-field-names-are-case-insensit.patch b/block-curl-HTTP-header-field-names-are-case-insensit.patch new file mode 100644 index 00000000..8f1028d7 --- /dev/null +++ b/block-curl-HTTP-header-field-names-are-case-insensit.patch @@ -0,0 +1,54 @@ +From ae2c6d13c4ac625a2c6b217a7f6a17506a2b26e5 Mon Sep 17 00:00:00 2001 +From: Richard Jones +Date: Thu, 28 May 2020 14:27:37 +0100 +Subject: [PATCH] block/curl: HTTP header field names are case insensitive +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +RH-Author: Richard Jones +Message-id: <20200528142737.17318-3-rjones@redhat.com> +Patchwork-id: 96895 +O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 2/2] block/curl: HTTP header field names are case insensitive +Bugzilla: 1841038 +RH-Acked-by: Eric Blake +RH-Acked-by: Max Reitz +RH-Acked-by: Philippe Mathieu-Daudé + +From: David Edmondson + +RFC 7230 section 3.2 indicates that HTTP header field names are case +insensitive. + +Signed-off-by: David Edmondson +Message-Id: <20200224101310.101169-3-david.edmondson@oracle.com> +Reviewed-by: Max Reitz +Signed-off-by: Max Reitz +(cherry picked from commit 69032253c33ae1774233c63cedf36d32242a85fc) +Signed-off-by: Danilo C. L. de Paula +--- + block/curl.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/block/curl.c b/block/curl.c +index bfabe7eabd..a298fcc591 100644 +--- a/block/curl.c ++++ b/block/curl.c +@@ -214,11 +214,12 @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque) + size_t realsize = size * nmemb; + const char *header = (char *)ptr; + const char *end = header + realsize; +- const char *accept_ranges = "Accept-Ranges:"; ++ const char *accept_ranges = "accept-ranges:"; + const char *bytes = "bytes"; + + if (realsize >= strlen(accept_ranges) +- && strncmp(header, accept_ranges, strlen(accept_ranges)) == 0) { ++ && g_ascii_strncasecmp(header, accept_ranges, ++ strlen(accept_ranges)) == 0) { + + char *p = strchr(header, ':') + 1; + +-- +2.27.0 + -- Gitee From a09d406df8ffa04f91135027b082385954e314f8 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Wed, 3 Jun 2020 16:03:19 +0100 Subject: [PATCH 13/20] backup: Improve error for bdrv_getlength() failure RH-Author: Kevin Wolf Message-id: <20200603160325.67506-6-kwolf@redhat.com> Patchwork-id: 97103 O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH v2 05/11] backup: Improve error for bdrv_getlength() failure Bugzilla: 1778593 RH-Acked-by: Eric Blake RH-Acked-by: Max Reitz RH-Acked-by: Stefano Garzarella bdrv_get_device_name() will be an empty string with modern management tools that don't use -drive. Use bdrv_get_device_or_node_name() instead so that the node name is used if the BlockBackend is anonymous. While at it, start with upper case to make the message consistent with the rest of the function. Signed-off-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Message-Id: <20200430142755.315494-3-kwolf@redhat.com> Signed-off-by: Kevin Wolf (cherry picked from commit 58226634c4b02af7b10862f7fbd3610a344bfb7f) Signed-off-by: Kevin Wolf Signed-off-by: Danilo C. L. de Paula --- ...ove-error-for-bdrv_getlength-failure.patch | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 backup-Improve-error-for-bdrv_getlength-failure.patch diff --git a/backup-Improve-error-for-bdrv_getlength-failure.patch b/backup-Improve-error-for-bdrv_getlength-failure.patch new file mode 100644 index 00000000..df188942 --- /dev/null +++ b/backup-Improve-error-for-bdrv_getlength-failure.patch @@ -0,0 +1,51 @@ +From 0b66aef5389d622434128fc7db9abd2cd4724b51 Mon Sep 17 00:00:00 2001 +From: Kevin Wolf +Date: Wed, 3 Jun 2020 16:03:19 +0100 +Subject: [PATCH] backup: Improve error for bdrv_getlength() failure + +RH-Author: Kevin Wolf +Message-id: <20200603160325.67506-6-kwolf@redhat.com> +Patchwork-id: 97103 +O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH v2 05/11] backup: Improve error for bdrv_getlength() failure +Bugzilla: 1778593 +RH-Acked-by: Eric Blake +RH-Acked-by: Max Reitz +RH-Acked-by: Stefano Garzarella + +bdrv_get_device_name() will be an empty string with modern management +tools that don't use -drive. Use bdrv_get_device_or_node_name() instead +so that the node name is used if the BlockBackend is anonymous. + +While at it, start with upper case to make the message consistent with +the rest of the function. + +Signed-off-by: Kevin Wolf +Reviewed-by: Vladimir Sementsov-Ogievskiy +Reviewed-by: Alberto Garcia +Message-Id: <20200430142755.315494-3-kwolf@redhat.com> +Signed-off-by: Kevin Wolf +(cherry picked from commit 58226634c4b02af7b10862f7fbd3610a344bfb7f) +Signed-off-by: Kevin Wolf +Signed-off-by: Danilo C. L. de Paula +--- + block/backup.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/block/backup.c b/block/backup.c +index 8761f1f9a7..88354dcb32 100644 +--- a/block/backup.c ++++ b/block/backup.c +@@ -613,8 +613,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, + + len = bdrv_getlength(bs); + if (len < 0) { +- error_setg_errno(errp, -len, "unable to get length for '%s'", +- bdrv_get_device_name(bs)); ++ error_setg_errno(errp, -len, "Unable to get length for '%s'", ++ bdrv_get_device_or_node_name(bs)); + goto error; + } + +-- +2.27.0 + -- Gitee From 4d0a386d5fc807a0fc5a753505360ca0f88d8228 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Wed, 3 Jun 2020 16:03:24 +0100 Subject: [PATCH 14/20] mirror: Make sure that source and target size match RH-Author: Kevin Wolf Message-id: <20200603160325.67506-11-kwolf@redhat.com> Patchwork-id: 97110 O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH v2 10/11] mirror: Make sure that source and target size match Bugzilla: 1778593 RH-Acked-by: Eric Blake RH-Acked-by: Max Reitz RH-Acked-by: Stefano Garzarella If the target is shorter than the source, mirror would copy data until it reaches the end of the target and then fail with an I/O error when trying to write past the end. If the target is longer than the source, the mirror job would complete successfully, but the target wouldn't actually be an accurate copy of the source image (it would contain some additional garbage at the end). Fix this by checking that both images have the same size when the job starts. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Message-Id: <20200511135825.219437-4-kwolf@redhat.com> Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Kevin Wolf (cherry picked from commit e83dd6808c6e0975970f37b49b27cc37bb54eea8) Signed-off-by: Kevin Wolf Signed-off-by: Danilo C. L. de Paula --- ...re-that-source-and-target-size-match.patch | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 mirror-Make-sure-that-source-and-target-size-match.patch diff --git a/mirror-Make-sure-that-source-and-target-size-match.patch b/mirror-Make-sure-that-source-and-target-size-match.patch new file mode 100644 index 00000000..5e4edd26 --- /dev/null +++ b/mirror-Make-sure-that-source-and-target-size-match.patch @@ -0,0 +1,89 @@ +From 9f57569d541acaa4a76513d09ede7d2b19aa69ea Mon Sep 17 00:00:00 2001 +From: Kevin Wolf +Date: Wed, 3 Jun 2020 16:03:24 +0100 +Subject: [PATCH] mirror: Make sure that source and target size match + +RH-Author: Kevin Wolf +Message-id: <20200603160325.67506-11-kwolf@redhat.com> +Patchwork-id: 97110 +O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH v2 10/11] mirror: Make sure that source and target size match +Bugzilla: 1778593 +RH-Acked-by: Eric Blake +RH-Acked-by: Max Reitz +RH-Acked-by: Stefano Garzarella + +If the target is shorter than the source, mirror would copy data until +it reaches the end of the target and then fail with an I/O error when +trying to write past the end. + +If the target is longer than the source, the mirror job would complete +successfully, but the target wouldn't actually be an accurate copy of +the source image (it would contain some additional garbage at the end). + +Fix this by checking that both images have the same size when the job +starts. + +Signed-off-by: Kevin Wolf +Reviewed-by: Eric Blake +Message-Id: <20200511135825.219437-4-kwolf@redhat.com> +Reviewed-by: Max Reitz +Reviewed-by: Vladimir Sementsov-Ogievskiy +Signed-off-by: Kevin Wolf +(cherry picked from commit e83dd6808c6e0975970f37b49b27cc37bb54eea8) +Signed-off-by: Kevin Wolf +Signed-off-by: Danilo C. L. de Paula +--- + block/mirror.c | 21 ++++++++++++--------- + 1 file changed, 12 insertions(+), 9 deletions(-) + +diff --git a/block/mirror.c b/block/mirror.c +index ef6c958ff9..8f0d4544d8 100644 +--- a/block/mirror.c ++++ b/block/mirror.c +@@ -853,6 +853,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) + BlockDriverState *target_bs = blk_bs(s->target); + bool need_drain = true; + int64_t length; ++ int64_t target_length; + BlockDriverInfo bdi; + char backing_filename[2]; /* we only need 2 characters because we are only + checking for a NULL string */ +@@ -868,24 +869,26 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) + goto immediate_exit; + } + ++ target_length = blk_getlength(s->target); ++ if (target_length < 0) { ++ ret = target_length; ++ goto immediate_exit; ++ } ++ + /* Active commit must resize the base image if its size differs from the + * active layer. */ + if (s->base == blk_bs(s->target)) { +- int64_t base_length; +- +- base_length = blk_getlength(s->target); +- if (base_length < 0) { +- ret = base_length; +- goto immediate_exit; +- } +- +- if (s->bdev_length > base_length) { ++ if (s->bdev_length > target_length) { + ret = blk_truncate(s->target, s->bdev_length, PREALLOC_MODE_OFF, + NULL); + if (ret < 0) { + goto immediate_exit; + } + } ++ } else if (s->bdev_length != target_length) { ++ error_setg(errp, "Source and target image have different sizes"); ++ ret = -EINVAL; ++ goto immediate_exit; + } + + if (s->bdev_length == 0) { +-- +2.27.0 + -- Gitee From ce754b00bd55088d9cfdd7cb09443266a7d8a39d Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 17 Oct 2019 15:31:40 +0200 Subject: [PATCH 15/20] iotests/143: Create socket in $SOCK_DIR Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Thomas Huth Message-id: 20191017133155.5327-9-mreitz@redhat.com Signed-off-by: Max Reitz --- iotests-143-Create-socket-in-SOCK_DIR.patch | 59 +++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 iotests-143-Create-socket-in-SOCK_DIR.patch diff --git a/iotests-143-Create-socket-in-SOCK_DIR.patch b/iotests-143-Create-socket-in-SOCK_DIR.patch new file mode 100644 index 00000000..31d6a842 --- /dev/null +++ b/iotests-143-Create-socket-in-SOCK_DIR.patch @@ -0,0 +1,59 @@ +From 2e8fecd9e963c740cfe73d0de4491541423e185f Mon Sep 17 00:00:00 2001 +From: Max Reitz +Date: Thu, 17 Oct 2019 15:31:40 +0200 +Subject: [PATCH] iotests/143: Create socket in $SOCK_DIR + +Signed-off-by: Max Reitz +Reviewed-by: Eric Blake +Reviewed-by: Thomas Huth +Message-id: 20191017133155.5327-9-mreitz@redhat.com +Signed-off-by: Max Reitz +--- + tests/qemu-iotests/143 | 6 +++--- + tests/qemu-iotests/143.out | 2 +- + 2 files changed, 4 insertions(+), 4 deletions(-) + +diff --git a/tests/qemu-iotests/143 b/tests/qemu-iotests/143 +index 92249ac8da..f649b36195 100755 +--- a/tests/qemu-iotests/143 ++++ b/tests/qemu-iotests/143 +@@ -29,7 +29,7 @@ status=1 # failure is the default! + _cleanup() + { + _cleanup_qemu +- rm -f "$TEST_DIR/nbd" ++ rm -f "$SOCK_DIR/nbd" + } + trap "_cleanup; exit \$status" 0 1 2 3 15 + +@@ -51,12 +51,12 @@ _send_qemu_cmd $QEMU_HANDLE \ + _send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'nbd-server-start', + 'arguments': { 'addr': { 'type': 'unix', +- 'data': { 'path': '$TEST_DIR/nbd' }}}}" \ ++ 'data': { 'path': '$SOCK_DIR/nbd' }}}}" \ + 'return' + + # This should just result in a client error, not in the server crashing + $QEMU_IO_PROG -f raw -c quit \ +- "nbd+unix:///no_such_export?socket=$TEST_DIR/nbd" 2>&1 \ ++ "nbd+unix:///no_such_export?socket=$SOCK_DIR/nbd" 2>&1 \ + | _filter_qemu_io | _filter_nbd + + _send_qemu_cmd $QEMU_HANDLE \ +diff --git a/tests/qemu-iotests/143.out b/tests/qemu-iotests/143.out +index ee71b5aa42..037d34a409 100644 +--- a/tests/qemu-iotests/143.out ++++ b/tests/qemu-iotests/143.out +@@ -1,7 +1,7 @@ + QA output created by 143 + {"return": {}} + {"return": {}} +-qemu-io: can't open device nbd+unix:///no_such_export?socket=TEST_DIR/nbd: Requested export not available ++qemu-io: can't open device nbd+unix:///no_such_export?socket=SOCK_DIR/nbd: Requested export not available + server reported: export 'no_such_export' not present + {"return": {}} + {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}} +-- +2.27.0 + -- Gitee From b79f9f3239c2b7600995f397d1a3381144c3c361 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Wed, 10 Jun 2020 18:32:01 -0400 Subject: [PATCH 16/20] nbd/server: Avoid long error message assertions CVE-2020-10761 RH-Author: Eric Blake Message-id: <20200610183202.3780750-2-eblake@redhat.com> Patchwork-id: 97494 O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 1/2] nbd/server: Avoid long error message assertions CVE-2020-10761 Bugzilla: 1845384 RH-Acked-by: Sergio Lopez Pascual RH-Acked-by: Max Reitz RH-Acked-by: Stefan Hajnoczi Ever since commit 36683283 (v2.8), the server code asserts that error strings sent to the client are well-formed per the protocol by not exceeding the maximum string length of 4096. At the time the server first started sending error messages, the assertion could not be triggered, because messages were completely under our control. However, over the years, we have added latent scenarios where a client could trigger the server to attempt an error message that would include the client's information if it passed other checks first: - requesting NBD_OPT_INFO/GO on an export name that is not present (commit 0cfae925 in v2.12 echoes the name) - requesting NBD_OPT_LIST/SET_META_CONTEXT on an export name that is not present (commit e7b1948d in v2.12 echoes the name) At the time, those were still safe because we flagged names larger than 256 bytes with a different message; but that changed in commit 93676c88 (v4.2) when we raised the name limit to 4096 to match the NBD string limit. (That commit also failed to change the magic number 4096 in nbd_negotiate_send_rep_err to the just-introduced named constant.) So with that commit, long client names appended to server text can now trigger the assertion, and thus be used as a denial of service attack against a server. As a mitigating factor, if the server requires TLS, the client cannot trigger the problematic paths unless it first supplies TLS credentials, and such trusted clients are less likely to try to intentionally crash the server. We may later want to further sanitize the user-supplied strings we place into our error messages, such as scrubbing out control characters, but that is less important to the CVE fix, so it can be a later patch to the new nbd_sanitize_name. Consideration was given to changing the assertion in nbd_negotiate_send_rep_verr to instead merely log a server error and truncate the message, to avoid leaving a latent path that could trigger a future CVE DoS on any new error message. However, this merely complicates the code for something that is already (correctly) flagging coding errors, and now that we are aware of the long message pitfall, we are less likely to introduce such errors in the future, which would make such error handling dead code. Reported-by: Xueqiang Wei CC: qemu-stable@nongnu.org Fixes: https://bugzilla.redhat.com/1843684 CVE-2020-10761 Fixes: 93676c88d7 Signed-off-by: Eric Blake Message-Id: <20200610163741.3745251-2-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy (cherry picked from commit 5c4fe018c025740fef4a0a4421e8162db0c3eefd) Signed-off-by: Eric Blake Signed-off-by: Eduardo Lima (Etrunko) --- ...-long-error-message-assertions-CVE-2.patch | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 nbd-server-Avoid-long-error-message-assertions-CVE-2.patch diff --git a/nbd-server-Avoid-long-error-message-assertions-CVE-2.patch b/nbd-server-Avoid-long-error-message-assertions-CVE-2.patch new file mode 100644 index 00000000..71ce6cab --- /dev/null +++ b/nbd-server-Avoid-long-error-message-assertions-CVE-2.patch @@ -0,0 +1,152 @@ +From 719292175d391e77487f3c55f5f97a065e44d9f8 Mon Sep 17 00:00:00 2001 +From: Eric Blake +Date: Wed, 10 Jun 2020 18:32:01 -0400 +Subject: [PATCH] nbd/server: Avoid long error message assertions + CVE-2020-10761 + +RH-Author: Eric Blake +Message-id: <20200610183202.3780750-2-eblake@redhat.com> +Patchwork-id: 97494 +O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 1/2] nbd/server: Avoid long error message assertions CVE-2020-10761 +Bugzilla: 1845384 +RH-Acked-by: Sergio Lopez Pascual +RH-Acked-by: Max Reitz +RH-Acked-by: Stefan Hajnoczi + +Ever since commit 36683283 (v2.8), the server code asserts that error +strings sent to the client are well-formed per the protocol by not +exceeding the maximum string length of 4096. At the time the server +first started sending error messages, the assertion could not be +triggered, because messages were completely under our control. +However, over the years, we have added latent scenarios where a client +could trigger the server to attempt an error message that would +include the client's information if it passed other checks first: + +- requesting NBD_OPT_INFO/GO on an export name that is not present + (commit 0cfae925 in v2.12 echoes the name) + +- requesting NBD_OPT_LIST/SET_META_CONTEXT on an export name that is + not present (commit e7b1948d in v2.12 echoes the name) + +At the time, those were still safe because we flagged names larger +than 256 bytes with a different message; but that changed in commit +93676c88 (v4.2) when we raised the name limit to 4096 to match the NBD +string limit. (That commit also failed to change the magic number +4096 in nbd_negotiate_send_rep_err to the just-introduced named +constant.) So with that commit, long client names appended to server +text can now trigger the assertion, and thus be used as a denial of +service attack against a server. As a mitigating factor, if the +server requires TLS, the client cannot trigger the problematic paths +unless it first supplies TLS credentials, and such trusted clients are +less likely to try to intentionally crash the server. + +We may later want to further sanitize the user-supplied strings we +place into our error messages, such as scrubbing out control +characters, but that is less important to the CVE fix, so it can be a +later patch to the new nbd_sanitize_name. + +Consideration was given to changing the assertion in +nbd_negotiate_send_rep_verr to instead merely log a server error and +truncate the message, to avoid leaving a latent path that could +trigger a future CVE DoS on any new error message. However, this +merely complicates the code for something that is already (correctly) +flagging coding errors, and now that we are aware of the long message +pitfall, we are less likely to introduce such errors in the future, +which would make such error handling dead code. + +Reported-by: Xueqiang Wei +CC: qemu-stable@nongnu.org +Fixes: https://bugzilla.redhat.com/1843684 CVE-2020-10761 +Fixes: 93676c88d7 +Signed-off-by: Eric Blake +Message-Id: <20200610163741.3745251-2-eblake@redhat.com> +Reviewed-by: Vladimir Sementsov-Ogievskiy +(cherry picked from commit 5c4fe018c025740fef4a0a4421e8162db0c3eefd) +Signed-off-by: Eric Blake +Signed-off-by: Eduardo Lima (Etrunko) +--- + nbd/server.c | 21 +++++++++++++++++++-- + tests/qemu-iotests/143 | 4 ++++ + tests/qemu-iotests/143.out | 2 ++ + 3 files changed, 25 insertions(+), 2 deletions(-) + +diff --git a/nbd/server.c b/nbd/server.c +index 2d81248967..115e8f06ed 100644 +--- a/nbd/server.c ++++ b/nbd/server.c +@@ -229,6 +229,19 @@ out: + return ret; + } + ++/* ++ * Return a malloc'd copy of @name suitable for use in an error reply. ++ */ ++static char * ++nbd_sanitize_name(const char *name) ++{ ++ if (strnlen(name, 80) < 80) { ++ return g_strdup(name); ++ } ++ /* XXX Should we also try to sanitize any control characters? */ ++ return g_strdup_printf("%.80s...", name); ++} ++ + /* Send an error reply. + * Return -errno on error, 0 on success. */ + static int GCC_FMT_ATTR(4, 5) +@@ -584,9 +597,11 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint16_t myflags, + + exp = nbd_export_find(name); + if (!exp) { ++ g_autofree char *sane_name = nbd_sanitize_name(name); ++ + return nbd_negotiate_send_rep_err(client, NBD_REP_ERR_UNKNOWN, + errp, "export '%s' not present", +- name); ++ sane_name); + } + + /* Don't bother sending NBD_INFO_NAME unless client requested it */ +@@ -975,8 +990,10 @@ static int nbd_negotiate_meta_queries(NBDClient *client, + + meta->exp = nbd_export_find(export_name); + if (meta->exp == NULL) { ++ g_autofree char *sane_name = nbd_sanitize_name(export_name); ++ + return nbd_opt_drop(client, NBD_REP_ERR_UNKNOWN, errp, +- "export '%s' not present", export_name); ++ "export '%s' not present", sane_name); + } + + ret = nbd_opt_read(client, &nb_queries, sizeof(nb_queries), errp); +diff --git a/tests/qemu-iotests/143 b/tests/qemu-iotests/143 +index f649b36195..d2349903b1 100755 +--- a/tests/qemu-iotests/143 ++++ b/tests/qemu-iotests/143 +@@ -58,6 +58,10 @@ _send_qemu_cmd $QEMU_HANDLE \ + $QEMU_IO_PROG -f raw -c quit \ + "nbd+unix:///no_such_export?socket=$SOCK_DIR/nbd" 2>&1 \ + | _filter_qemu_io | _filter_nbd ++# Likewise, with longest possible name permitted in NBD protocol ++$QEMU_IO_PROG -f raw -c quit \ ++ "nbd+unix:///$(printf %4096d 1 | tr ' ' a)?socket=$SOCK_DIR/nbd" 2>&1 \ ++ | _filter_qemu_io | _filter_nbd | sed 's/aaaa*aa/aa--aa/' + + _send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'quit' }" \ +diff --git a/tests/qemu-iotests/143.out b/tests/qemu-iotests/143.out +index 037d34a409..fc7bab3129 100644 +--- a/tests/qemu-iotests/143.out ++++ b/tests/qemu-iotests/143.out +@@ -3,6 +3,8 @@ QA output created by 143 + {"return": {}} + qemu-io: can't open device nbd+unix:///no_such_export?socket=SOCK_DIR/nbd: Requested export not available + server reported: export 'no_such_export' not present ++qemu-io: can't open device nbd+unix:///aa--aa1?socket=SOCK_DIR/nbd: Requested export not available ++server reported: export 'aa--aa...' not present + {"return": {}} + {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}} + *** done +-- +2.27.0 + -- Gitee From 126dbf2602e1f2abb84618c785e7d848e9b67582 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Wed, 10 Jun 2020 18:32:02 -0400 Subject: [PATCH 17/20] block: Call attention to truncation of long NBD exports RH-Author: Eric Blake Message-id: <20200610183202.3780750-3-eblake@redhat.com> Patchwork-id: 97495 O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 2/2] block: Call attention to truncation of long NBD exports Bugzilla: 1845384 RH-Acked-by: Sergio Lopez Pascual RH-Acked-by: Max Reitz RH-Acked-by: Stefan Hajnoczi Commit 93676c88 relaxed our NBD client code to request export names up to the NBD protocol maximum of 4096 bytes without NUL terminator, even though the block layer can't store anything longer than 4096 bytes including NUL terminator for display to the user. Since this means there are some export names where we have to truncate things, we can at least try to make the truncation a bit more obvious for the user. Note that in spite of the truncated display name, we can still communicate with an NBD server using such a long export name; this was deemed nicer than refusing to even connect to such a server (since the server may not be under our control, and since determining our actual length limits gets tricky when nbd://host:port/export and nbd+unix:///export?socket=/path are themselves variable-length expansions beyond the export name but count towards the block layer name length). Reported-by: Xueqiang Wei Fixes: https://bugzilla.redhat.com/1843684 Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200610163741.3745251-3-eblake@redhat.com> (cherry picked from commit 5c86bdf1208916ece0b87e1151c9b48ee54faa3e) Signed-off-by: Eric Blake Signed-off-by: Eduardo Lima (Etrunko) --- ...tion-to-truncation-of-long-NBD-expor.patch | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 block-Call-attention-to-truncation-of-long-NBD-expor.patch diff --git a/block-Call-attention-to-truncation-of-long-NBD-expor.patch b/block-Call-attention-to-truncation-of-long-NBD-expor.patch new file mode 100644 index 00000000..91745acf --- /dev/null +++ b/block-Call-attention-to-truncation-of-long-NBD-expor.patch @@ -0,0 +1,105 @@ +From e94c1625c0f8155740b1bb7b2c749df759e04526 Mon Sep 17 00:00:00 2001 +From: Eric Blake +Date: Wed, 10 Jun 2020 18:32:02 -0400 +Subject: [PATCH] block: Call attention to truncation of long NBD exports + +RH-Author: Eric Blake +Message-id: <20200610183202.3780750-3-eblake@redhat.com> +Patchwork-id: 97495 +O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 2/2] block: Call attention to truncation of long NBD exports +Bugzilla: 1845384 +RH-Acked-by: Sergio Lopez Pascual +RH-Acked-by: Max Reitz +RH-Acked-by: Stefan Hajnoczi + +Commit 93676c88 relaxed our NBD client code to request export names up +to the NBD protocol maximum of 4096 bytes without NUL terminator, even +though the block layer can't store anything longer than 4096 bytes +including NUL terminator for display to the user. Since this means +there are some export names where we have to truncate things, we can +at least try to make the truncation a bit more obvious for the user. +Note that in spite of the truncated display name, we can still +communicate with an NBD server using such a long export name; this was +deemed nicer than refusing to even connect to such a server (since the +server may not be under our control, and since determining our actual +length limits gets tricky when nbd://host:port/export and +nbd+unix:///export?socket=/path are themselves variable-length +expansions beyond the export name but count towards the block layer +name length). + +Reported-by: Xueqiang Wei +Fixes: https://bugzilla.redhat.com/1843684 +Signed-off-by: Eric Blake +Reviewed-by: Vladimir Sementsov-Ogievskiy +Message-Id: <20200610163741.3745251-3-eblake@redhat.com> +(cherry picked from commit 5c86bdf1208916ece0b87e1151c9b48ee54faa3e) +Signed-off-by: Eric Blake +Signed-off-by: Eduardo Lima (Etrunko) +--- + block.c | 7 +++++-- + block/nbd.c | 21 +++++++++++++-------- + 2 files changed, 18 insertions(+), 10 deletions(-) + +diff --git a/block.c b/block.c +index 38880eabf8..ba36b53a00 100644 +--- a/block.c ++++ b/block.c +@@ -6444,8 +6444,11 @@ void bdrv_refresh_filename(BlockDriverState *bs) + pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename); + } else { + QString *json = qobject_to_json(QOBJECT(bs->full_open_options)); +- snprintf(bs->filename, sizeof(bs->filename), "json:%s", +- qstring_get_str(json)); ++ if (snprintf(bs->filename, sizeof(bs->filename), "json:%s", ++ qstring_get_str(json)) >= sizeof(bs->filename)) { ++ /* Give user a hint if we truncated things. */ ++ strcpy(bs->filename + sizeof(bs->filename) - 4, "..."); ++ } + qobject_unref(json); + } + } +diff --git a/block/nbd.c b/block/nbd.c +index 3977b1efc7..63cdd051ab 100644 +--- a/block/nbd.c ++++ b/block/nbd.c +@@ -1714,6 +1714,7 @@ static void nbd_refresh_filename(BlockDriverState *bs) + { + BDRVNBDState *s = bs->opaque; + const char *host = NULL, *port = NULL, *path = NULL; ++ size_t len = 0; + + if (s->saddr->type == SOCKET_ADDRESS_TYPE_INET) { + const InetSocketAddress *inet = &s->saddr->u.inet; +@@ -1726,17 +1727,21 @@ static void nbd_refresh_filename(BlockDriverState *bs) + } /* else can't represent as pseudo-filename */ + + if (path && s->export) { +- snprintf(bs->exact_filename, sizeof(bs->exact_filename), +- "nbd+unix:///%s?socket=%s", s->export, path); ++ len = snprintf(bs->exact_filename, sizeof(bs->exact_filename), ++ "nbd+unix:///%s?socket=%s", s->export, path); + } else if (path && !s->export) { +- snprintf(bs->exact_filename, sizeof(bs->exact_filename), +- "nbd+unix://?socket=%s", path); ++ len = snprintf(bs->exact_filename, sizeof(bs->exact_filename), ++ "nbd+unix://?socket=%s", path); + } else if (host && s->export) { +- snprintf(bs->exact_filename, sizeof(bs->exact_filename), +- "nbd://%s:%s/%s", host, port, s->export); ++ len = snprintf(bs->exact_filename, sizeof(bs->exact_filename), ++ "nbd://%s:%s/%s", host, port, s->export); + } else if (host && !s->export) { +- snprintf(bs->exact_filename, sizeof(bs->exact_filename), +- "nbd://%s:%s", host, port); ++ len = snprintf(bs->exact_filename, sizeof(bs->exact_filename), ++ "nbd://%s:%s", host, port); ++ } ++ if (len > sizeof(bs->exact_filename)) { ++ /* Name is too long to represent exactly, so leave it empty. */ ++ bs->exact_filename[0] = '\0'; + } + } + +-- +2.27.0 + -- Gitee From ea6cf8b5479d8fa9422fb8154f85d10648031ae4 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Fri, 31 Jul 2020 08:18:31 -0400 Subject: [PATCH 18/20] qemu-img convert: Don't pre-zero images RH-Author: Kevin Wolf Message-id: <20200731081831.13781-2-kwolf@redhat.com> Patchwork-id: 98117 O-Subject: [RHEL-AV-8.2.1.z qemu-kvm PATCH 1/1] qemu-img convert: Don't pre-zero images Bugzilla: 1861682 RH-Acked-by: Stefano Garzarella RH-Acked-by: Max Reitz RH-Acked-by: Eric Blake Since commit 5a37b60a61c, qemu-img create will pre-zero the target image if it isn't already zero-initialised (most importantly, for host block devices, but also iscsi etc.), so that writing explicit zeros wouldn't be necessary later. This could speed up the operation significantly, in particular when the source image file was only sparsely populated. However, it also means that some block are written twice: Once when pre-zeroing them, and then when they are overwritten with actual data. On a full image, the pre-zeroing is wasted work because everything will be overwritten. In practice, write_zeroes typically turns out faster than writing explicit zero buffers, but slow enough that first zeroing everything and then overwriting parts can be a significant net loss. Meanwhile, qemu-img convert was rewritten in 690c7301600 and zero blocks are now written to the target using bdrv_co_pwrite_zeroes() if the target could be pre-zeroed. This way we already make use of the faster write_zeroes operation, but avoid writing any blocks twice. Remove the pre-zeroing because these days this former optimisation has actually turned into a pessimisation in the common case. Reported-by: Nir Soffer Signed-off-by: Kevin Wolf Message-Id: <20200622151203.35624-1-kwolf@redhat.com> Tested-by: Nir Soffer Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf (cherry picked from commit edafc70c0c8510862f2f213a3acf7067113bcd08) Signed-off-by: Kevin Wolf Signed-off-by: Danilo C. L. de Paula --- qemu-img-convert-Don-t-pre-zero-images.patch | 73 ++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 qemu-img-convert-Don-t-pre-zero-images.patch diff --git a/qemu-img-convert-Don-t-pre-zero-images.patch b/qemu-img-convert-Don-t-pre-zero-images.patch new file mode 100644 index 00000000..925590c3 --- /dev/null +++ b/qemu-img-convert-Don-t-pre-zero-images.patch @@ -0,0 +1,73 @@ +From a2fcbe2b82c42f890a857ad8d4edcfdb273106ea Mon Sep 17 00:00:00 2001 +From: Kevin Wolf +Date: Fri, 31 Jul 2020 08:18:31 -0400 +Subject: [PATCH] qemu-img convert: Don't pre-zero images + +RH-Author: Kevin Wolf +Message-id: <20200731081831.13781-2-kwolf@redhat.com> +Patchwork-id: 98117 +O-Subject: [RHEL-AV-8.2.1.z qemu-kvm PATCH 1/1] qemu-img convert: Don't pre-zero images +Bugzilla: 1861682 +RH-Acked-by: Stefano Garzarella +RH-Acked-by: Max Reitz +RH-Acked-by: Eric Blake + +Since commit 5a37b60a61c, qemu-img create will pre-zero the target image +if it isn't already zero-initialised (most importantly, for host block +devices, but also iscsi etc.), so that writing explicit zeros wouldn't +be necessary later. + +This could speed up the operation significantly, in particular when the +source image file was only sparsely populated. However, it also means +that some block are written twice: Once when pre-zeroing them, and then +when they are overwritten with actual data. On a full image, the +pre-zeroing is wasted work because everything will be overwritten. + +In practice, write_zeroes typically turns out faster than writing +explicit zero buffers, but slow enough that first zeroing everything and +then overwriting parts can be a significant net loss. + +Meanwhile, qemu-img convert was rewritten in 690c7301600 and zero blocks +are now written to the target using bdrv_co_pwrite_zeroes() if the +target could be pre-zeroed. This way we already make use of the faster +write_zeroes operation, but avoid writing any blocks twice. + +Remove the pre-zeroing because these days this former optimisation has +actually turned into a pessimisation in the common case. + +Reported-by: Nir Soffer +Signed-off-by: Kevin Wolf +Message-Id: <20200622151203.35624-1-kwolf@redhat.com> +Tested-by: Nir Soffer +Reviewed-by: Eric Blake +Signed-off-by: Kevin Wolf +(cherry picked from commit edafc70c0c8510862f2f213a3acf7067113bcd08) +Signed-off-by: Kevin Wolf +Signed-off-by: Danilo C. L. de Paula +--- + qemu-img.c | 9 --------- + 1 file changed, 9 deletions(-) + +diff --git a/qemu-img.c b/qemu-img.c +index 2e9cc5db7c..e4abd4978a 100644 +--- a/qemu-img.c ++++ b/qemu-img.c +@@ -1981,15 +1981,6 @@ static int convert_do_copy(ImgConvertState *s) + ? bdrv_has_zero_init(blk_bs(s->target)) + : false; + +- if (!s->has_zero_init && !s->target_has_backing && +- bdrv_can_write_zeroes_with_unmap(blk_bs(s->target))) +- { +- ret = blk_make_zero(s->target, BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK); +- if (ret == 0) { +- s->has_zero_init = true; +- } +- } +- + /* Allocate buffer for copied data. For compressed images, only one cluster + * can be copied at a time. */ + if (s->compressed) { +-- +2.27.0 + -- Gitee From e94ff0db36045550016e51fcaac56c9f62c98bbd Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Wed, 21 Jul 2021 21:27:22 +0800 Subject: [PATCH 19/20] spec: Update patch and changelog with !165 backport qemu-4.1 bugfix and CVE fix !165 block/curl: HTTP header fields allow whitespace around values block/curl: HTTP header field names are case insensitive backup: Improve error for bdrv_getlength() failure mirror: Make sure that source and target size match iotests/143: Create socket in $SOCK_DIR nbd/server: Avoid long error message assertions CVE-2020-10761 block: Call attention to truncation of long NBD exports qemu-img convert: Don't pre-zero images Signed-off-by: Chen Qun --- qemu.spec | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/qemu.spec b/qemu.spec index 7187f2bc..02166067 100644 --- a/qemu.spec +++ b/qemu.spec @@ -376,6 +376,14 @@ Patch0363: contrib-libvhost-user-Protect-slave-fd-with-mutex.patch Patch0364: libvhost-user-Fix-some-memtable-remap-cases.patch Patch0365: xics-Don-t-deassert-outputs.patch Patch0366: i386-Resolve-CPU-models-to-v1-by-default.patch +Patch0367: block-curl-HTTP-header-fields-allow-whitespace-aroun.patch +Patch0368: block-curl-HTTP-header-field-names-are-case-insensit.patch +Patch0369: backup-Improve-error-for-bdrv_getlength-failure.patch +Patch0370: mirror-Make-sure-that-source-and-target-size-match.patch +Patch0371: iotests-143-Create-socket-in-SOCK_DIR.patch +Patch0372: nbd-server-Avoid-long-error-message-assertions-CVE-2.patch +Patch0373: block-Call-attention-to-truncation-of-long-NBD-expor.patch +Patch0374: qemu-img-convert-Don-t-pre-zero-images.patch BuildRequires: flex BuildRequires: gcc @@ -770,6 +778,16 @@ getent passwd qemu >/dev/null || \ %endif %changelog +* Wed Jul 21 2021 Chen Qun +- block/curl: HTTP header fields allow whitespace around values +- block/curl: HTTP header field names are case insensitive +- backup: Improve error for bdrv_getlength() failure +- mirror: Make sure that source and target size match +- iotests/143: Create socket in $SOCK_DIR +- nbd/server: Avoid long error message assertions CVE-2020-10761 +- block: Call attention to truncation of long NBD exports +- qemu-img convert: Don't pre-zero images + * Wed Jul 21 2021 Chen Qun - virtio: don't enable notifications during polling - usbredir: Prevent recursion in usbredir_write -- Gitee From 3072b3a63122ffdc356d57dc229ea567a9726792 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Wed, 21 Jul 2021 21:27:22 +0800 Subject: [PATCH 20/20] spec: Update release version with !164 !165 increase release verison by one Signed-off-by: Chen Qun --- qemu.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu.spec b/qemu.spec index 02166067..9170229f 100644 --- a/qemu.spec +++ b/qemu.spec @@ -1,6 +1,6 @@ Name: qemu Version: 4.1.0 -Release: 65 +Release: 66 Epoch: 2 Summary: QEMU is a generic and open source machine emulator and virtualizer License: GPLv2 and BSD and MIT and CC-BY-SA-4.0 -- Gitee