From 8360fa3a325371f89d1502833a276a02235c8fbb3e06706c0adc63e9196e58b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Ricardo=20Ziviani?= Date: Tue, 25 May 2021 03:20:35 +0000 Subject: [PATCH] Accepting request 895224 from home:jziviani:branches:Virtualization - Fix CVE-2021-3527 in usb/redir: usb-redir-avoid-dynamic-stack-allocation.patch - Fix issues found upstream: hw-block-nvme-consider-metadata-read-aio.patch sockets-update-SOCKET_ADDRESS_TYPE_FD-li.patch vfio-ccw-Permit-missing-IRQs.patch vhost-user-blk-Check-that-num-queues-is-.patch vhost-user-blk-Don-t-reconnect-during-in.patch vhost-user-blk-Fail-gracefully-on-too-la.patch vhost-user-blk-Get-more-feature-flags-fr.patch vhost-user-blk-Make-sure-to-set-Error-on.patch virtio-blk-Fix-rollback-path-in-virtio_b.patch virtio-Fail-if-iommu_platform-is-request.patch virtiofsd-Fix-side-effect-in-assert.patch monitor-qmp-fix-race-on-CHR_EVENT_CLOSED.patch OBS-URL: https://build.opensuse.org/request/show/895224 OBS-URL: https://build.opensuse.org/package/show/Virtualization/qemu?expand=0&rev=650 --- bundles.tar.xz | 4 +- ...lock-nvme-consider-metadata-read-aio.patch | 50 +++++ ...tor-qmp-fix-race-on-CHR_EVENT_CLOSED.patch | 86 +++++++++ qemu.changes | 24 ++- qemu.spec | 26 +++ ...ets-update-SOCKET_ADDRESS_TYPE_FD-li.patch | 97 ++++++++++ ...redir-avoid-dynamic-stack-allocation.patch | 53 ++++++ vfio-ccw-Permit-missing-IRQs.patch | 71 ++++++++ ...t-user-blk-Check-that-num-queues-is-.patch | 74 ++++++++ ...t-user-blk-Don-t-reconnect-during-in.patch | 171 ++++++++++++++++++ ...t-user-blk-Fail-gracefully-on-too-la.patch | 47 +++++ ...t-user-blk-Get-more-feature-flags-fr.patch | 35 ++++ ...t-user-blk-Make-sure-to-set-Error-on.patch | 44 +++++ ...io-Fail-if-iommu_platform-is-request.patch | 44 +++++ ...io-blk-Fix-rollback-path-in-virtio_b.patch | 68 +++++++ virtiofsd-Fix-side-effect-in-assert.patch | 100 ++++++++++ 16 files changed, 987 insertions(+), 7 deletions(-) create mode 100644 hw-block-nvme-consider-metadata-read-aio.patch create mode 100644 monitor-qmp-fix-race-on-CHR_EVENT_CLOSED.patch create mode 100644 sockets-update-SOCKET_ADDRESS_TYPE_FD-li.patch create mode 100644 usb-redir-avoid-dynamic-stack-allocation.patch create mode 100644 vfio-ccw-Permit-missing-IRQs.patch create mode 100644 vhost-user-blk-Check-that-num-queues-is-.patch create mode 100644 vhost-user-blk-Don-t-reconnect-during-in.patch create mode 100644 vhost-user-blk-Fail-gracefully-on-too-la.patch create mode 100644 vhost-user-blk-Get-more-feature-flags-fr.patch create mode 100644 vhost-user-blk-Make-sure-to-set-Error-on.patch create mode 100644 virtio-Fail-if-iommu_platform-is-request.patch create mode 100644 virtio-blk-Fix-rollback-path-in-virtio_b.patch create mode 100644 virtiofsd-Fix-side-effect-in-assert.patch diff --git a/bundles.tar.xz b/bundles.tar.xz index 78fa237b..d9b80d33 100644 --- a/bundles.tar.xz +++ b/bundles.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:2c4897a39161af89a93dc5d80baac237a8839cfb246867cf2f440baa42dce9db -size 44964 +oid sha256:0300c7a1fdea7e0c7c20b707481ea1894523eee636f7746cc50d5d12425036b2 +size 58808 diff --git a/hw-block-nvme-consider-metadata-read-aio.patch b/hw-block-nvme-consider-metadata-read-aio.patch new file mode 100644 index 00000000..bc4b325c --- /dev/null +++ b/hw-block-nvme-consider-metadata-read-aio.patch @@ -0,0 +1,50 @@ +From: Gollu Appalanaidu +Date: Fri, 16 Apr 2021 12:52:33 +0530 +Subject: hw/block/nvme: consider metadata read aio return value in compare + +Git-commit: b4a983239343efd0a2d8a6cdf0690d0d707ec4ea + +Currently in compare command metadata aio read blk_aio_preadv return +value ignored. Consider it and complete the block accounting. + +Signed-off-by: Gollu Appalanaidu +Fixes: 0a384f923f51 ("hw/block/nvme: add compare command") +Signed-off-by: Klaus Jensen +Signed-off-by: Jose R. Ziviani +--- + hw/block/nvme.c | 11 +++++++++++ + 1 file changed, 11 insertions(+) + +diff --git a/hw/block/nvme.c b/hw/block/nvme.c +index 5fe082ec34c57471fab0fa7e8a0c..ba90053b63ed4884deb98d62b6d6 100644 +--- a/hw/block/nvme.c ++++ b/hw/block/nvme.c +@@ -2369,10 +2369,19 @@ static void nvme_compare_mdata_cb(void *opaque, int ret) + uint32_t reftag = le32_to_cpu(rw->reftag); + struct nvme_compare_ctx *ctx = req->opaque; + g_autofree uint8_t *buf = NULL; ++ BlockBackend *blk = ns->blkconf.blk; ++ BlockAcctCookie *acct = &req->acct; ++ BlockAcctStats *stats = blk_get_stats(blk); + uint16_t status = NVME_SUCCESS; + + trace_pci_nvme_compare_mdata_cb(nvme_cid(req)); + ++ if (ret) { ++ block_acct_failed(stats, acct); ++ nvme_aio_err(req, ret); ++ goto out; ++ } ++ + buf = g_malloc(ctx->mdata.iov.size); + + status = nvme_bounce_mdata(n, buf, ctx->mdata.iov.size, +@@ -2421,6 +2430,8 @@ static void nvme_compare_mdata_cb(void *opaque, int ret) + goto out; + } + ++ block_acct_done(stats, acct); ++ + out: + qemu_iovec_destroy(&ctx->data.iov); + g_free(ctx->data.bounce); diff --git a/monitor-qmp-fix-race-on-CHR_EVENT_CLOSED.patch b/monitor-qmp-fix-race-on-CHR_EVENT_CLOSED.patch new file mode 100644 index 00000000..b926c7e5 --- /dev/null +++ b/monitor-qmp-fix-race-on-CHR_EVENT_CLOSED.patch @@ -0,0 +1,86 @@ +From: Stefan Reiter +Date: Mon, 22 Mar 2021 16:40:24 +0100 +Subject: monitor/qmp: fix race on CHR_EVENT_CLOSED without OOB + +Git-commit: a67b996e7894edfafbcd3fd007c9f58f26d25908 + +The QMP dispatcher coroutine holds the qmp_queue_lock over a yield +point, where it expects to be rescheduled from the main context. If a +CHR_EVENT_CLOSED event is received just then, it can race and block the +main thread on the mutex in monitor_qmp_cleanup_queue_and_resume. + +monitor_resume does not need to be called from main context, so we can +call it immediately after popping a request from the queue, which allows +us to drop the qmp_queue_lock mutex before yielding. + +Suggested-by: Wolfgang Bumiller +Signed-off-by: Stefan Reiter +Message-Id: <20210322154024.15011-1-s.reiter@proxmox.com> +Reviewed-by: Kevin Wolf +Cc: qemu-stable@nongnu.org +Signed-off-by: Markus Armbruster +Signed-off-by: Jose R. Ziviani +--- + monitor/qmp.c | 40 ++++++++++++++++++++++------------------ + 1 file changed, 22 insertions(+), 18 deletions(-) + +diff --git a/monitor/qmp.c b/monitor/qmp.c +index 2b0308f93371dde1a8085ac9c402..092c527b6fc9c6363f4bf81d8573 100644 +--- a/monitor/qmp.c ++++ b/monitor/qmp.c +@@ -257,24 +257,6 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) + trace_monitor_qmp_in_band_dequeue(req_obj, + req_obj->mon->qmp_requests->length); + +- if (qatomic_xchg(&qmp_dispatcher_co_busy, true) == true) { +- /* +- * Someone rescheduled us (probably because a new requests +- * came in), but we didn't actually yield. Do that now, +- * only to be immediately reentered and removed from the +- * list of scheduled coroutines. +- */ +- qemu_coroutine_yield(); +- } +- +- /* +- * Move the coroutine from iohandler_ctx to qemu_aio_context for +- * executing the command handler so that it can make progress if it +- * involves an AIO_WAIT_WHILE(). +- */ +- aio_co_schedule(qemu_get_aio_context(), qmp_dispatcher_co); +- qemu_coroutine_yield(); +- + /* + * @req_obj has a request, we hold req_obj->mon->qmp_queue_lock + */ +@@ -298,8 +280,30 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) + monitor_resume(&mon->common); + } + ++ /* ++ * Drop the queue mutex now, before yielding, otherwise we might ++ * deadlock if the main thread tries to lock it. ++ */ + qemu_mutex_unlock(&mon->qmp_queue_lock); + ++ if (qatomic_xchg(&qmp_dispatcher_co_busy, true) == true) { ++ /* ++ * Someone rescheduled us (probably because a new requests ++ * came in), but we didn't actually yield. Do that now, ++ * only to be immediately reentered and removed from the ++ * list of scheduled coroutines. ++ */ ++ qemu_coroutine_yield(); ++ } ++ ++ /* ++ * Move the coroutine from iohandler_ctx to qemu_aio_context for ++ * executing the command handler so that it can make progress if it ++ * involves an AIO_WAIT_WHILE(). ++ */ ++ aio_co_schedule(qemu_get_aio_context(), qmp_dispatcher_co); ++ qemu_coroutine_yield(); ++ + /* Process request */ + if (req_obj->req) { + if (trace_event_get_state(TRACE_MONITOR_QMP_CMD_IN_BAND)) { diff --git a/qemu.changes b/qemu.changes index 81bead4a..df27ae48 100644 --- a/qemu.changes +++ b/qemu.changes @@ -1,3 +1,22 @@ +------------------------------------------------------------------- +Mon May 24 23:20:04 UTC 2021 - José Ricardo Ziviani + +- Fix CVE-2021-3527 in usb/redir: + usb-redir-avoid-dynamic-stack-allocation.patch +- Fix issues found upstream: + hw-block-nvme-consider-metadata-read-aio.patch + sockets-update-SOCKET_ADDRESS_TYPE_FD-li.patch + vfio-ccw-Permit-missing-IRQs.patch + vhost-user-blk-Check-that-num-queues-is-.patch + vhost-user-blk-Don-t-reconnect-during-in.patch + vhost-user-blk-Fail-gracefully-on-too-la.patch + vhost-user-blk-Get-more-feature-flags-fr.patch + vhost-user-blk-Make-sure-to-set-Error-on.patch + virtio-blk-Fix-rollback-path-in-virtio_b.patch + virtio-Fail-if-iommu_platform-is-request.patch + virtiofsd-Fix-side-effect-in-assert.patch + monitor-qmp-fix-race-on-CHR_EVENT_CLOSED.patch + ------------------------------------------------------------------- Mon May 17 20:34:14 UTC 2021 - José Ricardo Ziviani @@ -8,11 +27,6 @@ Mon May 17 20:34:14 UTC 2021 - José Ricardo Ziviani hw-rx-rx-gdbsim-Do-not-accept-invalid-me.patch ui-Fix-memory-leak-in-qemu_xkeymap_mappi.patch -------------------------------------------------------------------- -Mon May 17 09:30:58 UTC 2021 - Martin Liška - -- Add fix-brotli-vla-error.patch in order to fix bsc#1181922. - ------------------------------------------------------------------- Thu May 6 00:33:36 UTC 2021 - Bruce Rogers diff --git a/qemu.spec b/qemu.spec index 82b491ef..e0a32f35 100644 --- a/qemu.spec +++ b/qemu.spec @@ -179,6 +179,19 @@ Patch00043: qom-handle-case-of-chardev-spice-module-.patch Patch00044: doc-add-our-support-doc-to-the-main-proj.patch Patch00045: ui-Fix-memory-leak-in-qemu_xkeymap_mappi.patch Patch00046: hw-rx-rx-gdbsim-Do-not-accept-invalid-me.patch +Patch00047: monitor-qmp-fix-race-on-CHR_EVENT_CLOSED.patch +Patch00048: vhost-user-blk-Fail-gracefully-on-too-la.patch +Patch00049: usb-redir-avoid-dynamic-stack-allocation.patch +Patch00050: virtiofsd-Fix-side-effect-in-assert.patch +Patch00051: sockets-update-SOCKET_ADDRESS_TYPE_FD-li.patch +Patch00052: virtio-blk-Fix-rollback-path-in-virtio_b.patch +Patch00053: hw-block-nvme-consider-metadata-read-aio.patch +Patch00054: vhost-user-blk-Make-sure-to-set-Error-on.patch +Patch00055: vhost-user-blk-Don-t-reconnect-during-in.patch +Patch00056: vhost-user-blk-Get-more-feature-flags-fr.patch +Patch00057: virtio-Fail-if-iommu_platform-is-request.patch +Patch00058: vhost-user-blk-Check-that-num-queues-is-.patch +Patch00059: vfio-ccw-Permit-missing-IRQs.patch # Patches applied in roms/seabios/: Patch01000: seabios-use-python2-explicitly-as-needed.patch Patch01001: seabios-switch-to-python3-as-needed.patch @@ -1058,6 +1071,19 @@ This package records qemu testsuite results and represents successful testing. %endif %patch00045 -p1 %patch00046 -p1 +%patch00047 -p1 +%patch00048 -p1 +%patch00049 -p1 +%patch00050 -p1 +%patch00051 -p1 +%patch00052 -p1 +%patch00053 -p1 +%patch00054 -p1 +%patch00055 -p1 +%patch00056 -p1 +%patch00057 -p1 +%patch00058 -p1 +%patch00059 -p1 %patch01000 -p1 %patch01001 -p1 %patch01002 -p1 diff --git a/sockets-update-SOCKET_ADDRESS_TYPE_FD-li.patch b/sockets-update-SOCKET_ADDRESS_TYPE_FD-li.patch new file mode 100644 index 00000000..e74bb0e1 --- /dev/null +++ b/sockets-update-SOCKET_ADDRESS_TYPE_FD-li.patch @@ -0,0 +1,97 @@ +From: Stefan Hajnoczi +Date: Wed, 10 Mar 2021 17:30:04 +0000 +Subject: sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog + +Git-commit: 37179e9ea45d6428b29ae789209c119ac18c1d39 + +socket_get_fd() fails with the error "socket_get_fd: too many +connections" if the given listen backlog value is not 1. + +Not all callers set the backlog to 1. For example, commit +582d4210eb2f2ab5baac328fe4b479cd86da1647 ("qemu-nbd: Use SOMAXCONN for +socket listen() backlog") uses SOMAXCONN. This will always fail with in +socket_get_fd(). + +This patch calls listen(2) on the fd to update the backlog value. The +socket may already be in the listen state. I have tested that this works +on Linux 5.10 and macOS Catalina. + +As a bonus this allows us to detect when the fd cannot listen. Now we'll +be able to catch unbound or connected fds in socket_listen(). + +Drop the num argument from socket_get_fd() since this function is also +called by socket_connect() where a listen backlog value does not make +sense. + +Fixes: e5b6353cf25c99c3f08bf51e29933352f7140e8f ("socket: Add backlog parameter to socket_listen") +Reported-by: Richard W.M. Jones +Cc: Juan Quintela +Cc: Eric Blake +Signed-off-by: Stefan Hajnoczi +Message-Id: <20210310173004.420190-1-stefanha@redhat.com> +Tested-by: Richard W.M. Jones +Reviewed-by: Eric Blake +Reviewed-by: Stefano Garzarella +Signed-off-by: Eric Blake +Signed-off-by: Jose R. Ziviani +--- + util/qemu-sockets.c | 29 ++++++++++++++++++++++------- + 1 file changed, 22 insertions(+), 7 deletions(-) + +diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c +index 8af0278f15c69fea136192e91650..2463c49773eae5ccac4c6c832c76 100644 +--- a/util/qemu-sockets.c ++++ b/util/qemu-sockets.c +@@ -1116,14 +1116,10 @@ fail: + return NULL; + } + +-static int socket_get_fd(const char *fdstr, int num, Error **errp) ++static int socket_get_fd(const char *fdstr, Error **errp) + { + Monitor *cur_mon = monitor_cur(); + int fd; +- if (num != 1) { +- error_setg_errno(errp, EINVAL, "socket_get_fd: too many connections"); +- return -1; +- } + if (cur_mon) { + fd = monitor_get_fd(cur_mon, fdstr, errp); + if (fd < 0) { +@@ -1159,7 +1155,7 @@ int socket_connect(SocketAddress *addr, Error **errp) + break; + + case SOCKET_ADDRESS_TYPE_FD: +- fd = socket_get_fd(addr->u.fd.str, 1, errp); ++ fd = socket_get_fd(addr->u.fd.str, errp); + break; + + case SOCKET_ADDRESS_TYPE_VSOCK: +@@ -1187,7 +1183,26 @@ int socket_listen(SocketAddress *addr, int num, Error **errp) + break; + + case SOCKET_ADDRESS_TYPE_FD: +- fd = socket_get_fd(addr->u.fd.str, num, errp); ++ fd = socket_get_fd(addr->u.fd.str, errp); ++ if (fd < 0) { ++ return -1; ++ } ++ ++ /* ++ * If the socket is not yet in the listen state, then transition it to ++ * the listen state now. ++ * ++ * If it's already listening then this updates the backlog value as ++ * requested. ++ * ++ * If this socket cannot listen because it's already in another state ++ * (e.g. unbound or connected) then we'll catch the error here. ++ */ ++ if (listen(fd, num) != 0) { ++ error_setg_errno(errp, errno, "Failed to listen on fd socket"); ++ closesocket(fd); ++ return -1; ++ } + break; + + case SOCKET_ADDRESS_TYPE_VSOCK: diff --git a/usb-redir-avoid-dynamic-stack-allocation.patch b/usb-redir-avoid-dynamic-stack-allocation.patch new file mode 100644 index 00000000..8ac14fb5 --- /dev/null +++ b/usb-redir-avoid-dynamic-stack-allocation.patch @@ -0,0 +1,53 @@ +From: Gerd Hoffmann +Date: Mon, 3 May 2021 15:29:12 +0200 +Subject: usb/redir: avoid dynamic stack allocation (CVE-2021-3527) +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Git-commit: 7ec54f9eb62b5d177e30eb8b1cad795a5f8d8986 +References: CVE-2021-3527 + +Use autofree heap allocation instead. + +Fixes: 4f4321c11ff ("usb: use iovecs in USBPacket") +Reviewed-by: Philippe Mathieu-Daudé +Signed-off-by: Gerd Hoffmann +Tested-by: Philippe Mathieu-Daudé +Message-Id: <20210503132915.2335822-3-kraxel@redhat.com> +Signed-off-by: Jose R. Ziviani +--- + hw/usb/redirect.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c +index 17f06f34179a257e3fd2b354164e..6a75b0dc4ab295a70b4c507c9821 100644 +--- a/hw/usb/redirect.c ++++ b/hw/usb/redirect.c +@@ -620,7 +620,7 @@ static void usbredir_handle_iso_data(USBRedirDevice *dev, USBPacket *p, + .endpoint = ep, + .length = p->iov.size + }; +- uint8_t buf[p->iov.size]; ++ g_autofree uint8_t *buf = g_malloc(p->iov.size); + /* No id, we look at the ep when receiving a status back */ + usb_packet_copy(p, buf, p->iov.size); + usbredirparser_send_iso_packet(dev->parser, 0, &iso_packet, +@@ -818,7 +818,7 @@ static void usbredir_handle_bulk_data(USBRedirDevice *dev, USBPacket *p, + usbredirparser_send_bulk_packet(dev->parser, p->id, + &bulk_packet, NULL, 0); + } else { +- uint8_t buf[size]; ++ g_autofree uint8_t *buf = g_malloc(size); + usb_packet_copy(p, buf, size); + usbredir_log_data(dev, "bulk data out:", buf, size); + usbredirparser_send_bulk_packet(dev->parser, p->id, +@@ -923,7 +923,7 @@ static void usbredir_handle_interrupt_out_data(USBRedirDevice *dev, + USBPacket *p, uint8_t ep) + { + struct usb_redir_interrupt_packet_header interrupt_packet; +- uint8_t buf[p->iov.size]; ++ g_autofree uint8_t *buf = g_malloc(p->iov.size); + + DPRINTF("interrupt-out ep %02X len %zd id %"PRIu64"\n", ep, + p->iov.size, p->id); diff --git a/vfio-ccw-Permit-missing-IRQs.patch b/vfio-ccw-Permit-missing-IRQs.patch new file mode 100644 index 00000000..b5d2d3f0 --- /dev/null +++ b/vfio-ccw-Permit-missing-IRQs.patch @@ -0,0 +1,71 @@ +From: Eric Farman +Date: Wed, 21 Apr 2021 17:20:53 +0200 +Subject: vfio-ccw: Permit missing IRQs + +Git-commit: 6178d4689a1e6a0d2b6dea1dad990e74148fa9d1 + +Commit 690e29b91102 ("vfio-ccw: Refactor ccw irq handler") changed +one of the checks for the IRQ notifier registration from saying +"the host needs to recognize the only IRQ that exists" to saying +"the host needs to recognize ANY IRQ that exists." + +And this worked fine, because the subsequent change to support the +CRW IRQ notifier doesn't get into this code when running on an older +kernel, thanks to a guard by a capability region. The later addition +of the REQ(uest) IRQ by commit b2f96f9e4f5f ("vfio-ccw: Connect the +device request notifier") broke this assumption because there is no +matching capability region. Thus, running new QEMU on an older +kernel fails with: + + vfio: unexpected number of irqs 2 + +Let's adapt the message here so that there's a better clue of what +IRQ is missing. + +Furthermore, let's make the REQ(uest) IRQ not fail when attempting +to register it, to permit running vfio-ccw on a newer QEMU with an +older kernel. + +Fixes: b2f96f9e4f5f ("vfio-ccw: Connect the device request notifier") +Signed-off-by: Eric Farman +Message-Id: <20210421152053.2379873-1-farman@linux.ibm.com> +Signed-off-by: Cornelia Huck +Signed-off-by: Jose R. Ziviani +--- + hw/vfio/ccw.c | 12 +++++++----- + 1 file changed, 7 insertions(+), 5 deletions(-) + +diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c +index b2df708e4b0192cc6af898edeca4..400bc07fe260837953de87d0f272 100644 +--- a/hw/vfio/ccw.c ++++ b/hw/vfio/ccw.c +@@ -412,8 +412,8 @@ static void vfio_ccw_register_irq_notifier(VFIOCCWDevice *vcdev, + } + + if (vdev->num_irqs < irq + 1) { +- error_setg(errp, "vfio: unexpected number of irqs %u", +- vdev->num_irqs); ++ error_setg(errp, "vfio: IRQ %u not available (number of irqs %u)", ++ irq, vdev->num_irqs); + return; + } + +@@ -696,13 +696,15 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) + + vfio_ccw_register_irq_notifier(vcdev, VFIO_CCW_REQ_IRQ_INDEX, &err); + if (err) { +- goto out_req_notifier_err; ++ /* ++ * Report this error, but do not make it a failing condition. ++ * Lack of this IRQ in the host does not prevent normal operation. ++ */ ++ error_report_err(err); + } + + return; + +-out_req_notifier_err: +- vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_CRW_IRQ_INDEX); + out_crw_notifier_err: + vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_IO_IRQ_INDEX); + out_io_notifier_err: diff --git a/vhost-user-blk-Check-that-num-queues-is-.patch b/vhost-user-blk-Check-that-num-queues-is-.patch new file mode 100644 index 00000000..0ef7f253 --- /dev/null +++ b/vhost-user-blk-Check-that-num-queues-is-.patch @@ -0,0 +1,74 @@ +From: Kevin Wolf +Date: Thu, 29 Apr 2021 19:13:16 +0200 +Subject: vhost-user-blk: Check that num-queues is supported by backend + +Git-commit: c90bd505a3e8210c23d69fecab9ee6f56ec4a161 + +Creating a device with a number of queues that isn't supported by the +backend is pointless, the device won't work properly and the error +messages are rather confusing. + +Just fail to create the device if num-queues is higher than what the +backend supports. + +Since the relationship between num-queues and the number of virtqueues +depends on the specific device, this is an additional value that needs +to be initialised by the device. For convenience, allow leaving it 0 if +the check should be skipped. This makes sense for vhost-user-net where +separate vhost devices are used for the queues and custom initialisation +code is needed to perform the check. + +Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935031 +Signed-off-by: Kevin Wolf +Reviewed-by: Raphael Norwitz +Message-Id: <20210429171316.162022-7-kwolf@redhat.com> +Reviewed-by: Michael S. Tsirkin +Signed-off-by: Kevin Wolf +Signed-off-by: Jose R. Ziviani +--- + hw/block/vhost-user-blk.c | 1 + + hw/virtio/vhost-user.c | 5 +++++ + include/hw/virtio/vhost.h | 2 ++ + 3 files changed, 8 insertions(+) + +diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c +index 738e8498b4a1d650047f7190c435..ceb6bdde71e57640677a48425148 100644 +--- a/hw/block/vhost-user-blk.c ++++ b/hw/block/vhost-user-blk.c +@@ -324,6 +324,7 @@ static int vhost_user_blk_connect(DeviceState *dev) + } + s->connected = true; + ++ s->dev.num_queues = s->num_queues; + s->dev.nvqs = s->num_queues; + s->dev.vqs = s->vhost_vqs; + s->dev.vq_index = 0; +diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c +index ded0c10453095830e24b6e53e8f8..ee57abe04526f6c55d983cb0254c 100644 +--- a/hw/virtio/vhost-user.c ++++ b/hw/virtio/vhost-user.c +@@ -1909,6 +1909,11 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque) + return err; + } + } ++ if (dev->num_queues && dev->max_queues < dev->num_queues) { ++ error_report("The maximum number of queues supported by the " ++ "backend is %" PRIu64, dev->max_queues); ++ return -EINVAL; ++ } + + if (virtio_has_feature(features, VIRTIO_F_IOMMU_PLATFORM) && + !(virtio_has_feature(dev->protocol_features, +diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h +index 4a8bc75415f6bba597c195e10a47..21a9a52088dd01838099046587fd 100644 +--- a/include/hw/virtio/vhost.h ++++ b/include/hw/virtio/vhost.h +@@ -74,6 +74,8 @@ struct vhost_dev { + int nvqs; + /* the first virtqueue which would be used by this vhost dev */ + int vq_index; ++ /* if non-zero, minimum required value for max_queues */ ++ int num_queues; + uint64_t features; + uint64_t acked_features; + uint64_t backend_features; diff --git a/vhost-user-blk-Don-t-reconnect-during-in.patch b/vhost-user-blk-Don-t-reconnect-during-in.patch new file mode 100644 index 00000000..af9277bc --- /dev/null +++ b/vhost-user-blk-Don-t-reconnect-during-in.patch @@ -0,0 +1,171 @@ +From: Kevin Wolf +Date: Thu, 29 Apr 2021 19:13:12 +0200 +Subject: vhost-user-blk: Don't reconnect during initialisation + +Git-commit: dabefdd6abcbc7d858e9413e4734aab2e0b5c8d9 + +This is a partial revert of commits 77542d43149 and bc79c87bcde. + +Usually, an error during initialisation means that the configuration was +wrong. Reconnecting won't make the error go away, but just turn the +error condition into an endless loop. Avoid this and return errors +again. + +Additionally, calling vhost_user_blk_disconnect() from the chardev event +handler could result in use-after-free because none of the +initialisation code expects that the device could just go away in the +middle. So removing the call fixes crashes in several places. + +For example, using a num-queues setting that is incompatible with the +backend would result in a crash like this (dereferencing dev->opaque, +which is already NULL): + + #0 0x0000555555d0a4bd in vhost_user_read_cb (source=0x5555568f4690, condition=(G_IO_IN | G_IO_HUP), opaque=0x7fffffffcbf0) at ../hw/virtio/vhost-user.c:313 + #1 0x0000555555d950d3 in qio_channel_fd_source_dispatch (source=0x555557c3f750, callback=0x555555d0a478 , user_data=0x7fffffffcbf0) at ../io/channel-watch.c:84 + #2 0x00007ffff7b32a9f in g_main_context_dispatch () at /lib64/libglib-2.0.so.0 + #3 0x00007ffff7b84a98 in g_main_context_iterate.constprop () at /lib64/libglib-2.0.so.0 + #4 0x00007ffff7b32163 in g_main_loop_run () at /lib64/libglib-2.0.so.0 + #5 0x0000555555d0a724 in vhost_user_read (dev=0x555557bc62f8, msg=0x7fffffffcc50) at ../hw/virtio/vhost-user.c:402 + #6 0x0000555555d0ee6b in vhost_user_get_config (dev=0x555557bc62f8, config=0x555557bc62ac "", config_len=60) at ../hw/virtio/vhost-user.c:2133 + #7 0x0000555555d56d46 in vhost_dev_get_config (hdev=0x555557bc62f8, config=0x555557bc62ac "", config_len=60) at ../hw/virtio/vhost.c:1566 + #8 0x0000555555cdd150 in vhost_user_blk_device_realize (dev=0x555557bc60b0, errp=0x7fffffffcf90) at ../hw/block/vhost-user-blk.c:510 + #9 0x0000555555d08f6d in virtio_device_realize (dev=0x555557bc60b0, errp=0x7fffffffcff0) at ../hw/virtio/virtio.c:3660 + +Note that this removes the ability to reconnect during initialisation +(but not during operation) when there is no permanent error, but the +backend restarts, as the implementation was buggy. This feature can be +added back in a follow-up series after changing error paths to +distinguish cases where retrying could help from cases with permanent +errors. + +Signed-off-by: Kevin Wolf +Message-Id: <20210429171316.162022-3-kwolf@redhat.com> +Reviewed-by: Michael S. Tsirkin +Signed-off-by: Kevin Wolf +Signed-off-by: Jose R. Ziviani +--- + hw/block/vhost-user-blk.c | 59 +++++++++++---------------------------- + 1 file changed, 17 insertions(+), 42 deletions(-) + +diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c +index 7c85248a7b78b9d9ec8614a3b5fe..c0b9958da1b4e155e063fb3426d0 100644 +--- a/hw/block/vhost-user-blk.c ++++ b/hw/block/vhost-user-blk.c +@@ -50,6 +50,8 @@ static const int user_feature_bits[] = { + VHOST_INVALID_FEATURE_BIT + }; + ++static void vhost_user_blk_event(void *opaque, QEMUChrEvent event); ++ + static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config) + { + VHostUserBlk *s = VHOST_USER_BLK(vdev); +@@ -362,19 +364,6 @@ static void vhost_user_blk_disconnect(DeviceState *dev) + vhost_dev_cleanup(&s->dev); + } + +-static void vhost_user_blk_event(void *opaque, QEMUChrEvent event, +- bool realized); +- +-static void vhost_user_blk_event_realize(void *opaque, QEMUChrEvent event) +-{ +- vhost_user_blk_event(opaque, event, false); +-} +- +-static void vhost_user_blk_event_oper(void *opaque, QEMUChrEvent event) +-{ +- vhost_user_blk_event(opaque, event, true); +-} +- + static void vhost_user_blk_chr_closed_bh(void *opaque) + { + DeviceState *dev = opaque; +@@ -382,12 +371,11 @@ static void vhost_user_blk_chr_closed_bh(void *opaque) + VHostUserBlk *s = VHOST_USER_BLK(vdev); + + vhost_user_blk_disconnect(dev); +- qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, +- vhost_user_blk_event_oper, NULL, opaque, NULL, true); ++ qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, vhost_user_blk_event, ++ NULL, opaque, NULL, true); + } + +-static void vhost_user_blk_event(void *opaque, QEMUChrEvent event, +- bool realized) ++static void vhost_user_blk_event(void *opaque, QEMUChrEvent event) + { + DeviceState *dev = opaque; + VirtIODevice *vdev = VIRTIO_DEVICE(dev); +@@ -401,17 +389,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent event, + } + break; + case CHR_EVENT_CLOSED: +- /* +- * Closing the connection should happen differently on device +- * initialization and operation stages. +- * On initalization, we want to re-start vhost_dev initialization +- * from the very beginning right away when the connection is closed, +- * so we clean up vhost_dev on each connection closing. +- * On operation, we want to postpone vhost_dev cleanup to let the +- * other code perform its own cleanup sequence using vhost_dev data +- * (e.g. vhost_dev_set_log). +- */ +- if (realized && !runstate_check(RUN_STATE_SHUTDOWN)) { ++ if (!runstate_check(RUN_STATE_SHUTDOWN)) { + /* + * A close event may happen during a read/write, but vhost + * code assumes the vhost_dev remains setup, so delay the +@@ -431,8 +409,6 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent event, + * knowing its type (in this case vhost-user). + */ + s->dev.started = false; +- } else { +- vhost_user_blk_disconnect(dev); + } + break; + case CHR_EVENT_BREAK: +@@ -489,33 +465,32 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) + s->vhost_vqs = g_new0(struct vhost_virtqueue, s->num_queues); + s->connected = false; + +- qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, +- vhost_user_blk_event_realize, NULL, (void *)dev, +- NULL, true); +- +-reconnect: + if (qemu_chr_fe_wait_connected(&s->chardev, errp) < 0) { + goto virtio_err; + } + +- /* check whether vhost_user_blk_connect() failed or not */ +- if (!s->connected) { +- goto reconnect; ++ if (vhost_user_blk_connect(dev) < 0) { ++ error_setg(errp, "vhost-user-blk: could not connect"); ++ qemu_chr_fe_disconnect(&s->chardev); ++ goto virtio_err; + } ++ assert(s->connected); + + ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg, + sizeof(struct virtio_blk_config)); + if (ret < 0) { +- error_report("vhost-user-blk: get block config failed"); +- goto reconnect; ++ error_setg(errp, "vhost-user-blk: get block config failed"); ++ goto vhost_err; + } + +- /* we're fully initialized, now we can operate, so change the handler */ ++ /* we're fully initialized, now we can operate, so add the handler */ + qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, +- vhost_user_blk_event_oper, NULL, (void *)dev, ++ vhost_user_blk_event, NULL, (void *)dev, + NULL, true); + return; + ++vhost_err: ++ vhost_dev_cleanup(&s->dev); + virtio_err: + g_free(s->vhost_vqs); + s->vhost_vqs = NULL; diff --git a/vhost-user-blk-Fail-gracefully-on-too-la.patch b/vhost-user-blk-Fail-gracefully-on-too-la.patch new file mode 100644 index 00000000..9e6166c0 --- /dev/null +++ b/vhost-user-blk-Fail-gracefully-on-too-la.patch @@ -0,0 +1,47 @@ +From: Kevin Wolf +Date: Tue, 13 Apr 2021 18:56:54 +0200 +Subject: vhost-user-blk: Fail gracefully on too large queue size +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Git-commit: 68bf7336533faa6aa90fdd4558edddbf5d8ef814 + +virtio_add_queue() aborts when queue_size > VIRTQUEUE_MAX_SIZE, so +vhost_user_blk_device_realize() should check this before calling it. + +Simple reproducer: + +qemu-system-x86_64 \ + -chardev null,id=foo \ + -device vhost-user-blk-pci,queue-size=4096,chardev=foo + +Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935014 +Signed-off-by: Kevin Wolf +Message-Id: <20210413165654.50810-1-kwolf@redhat.com> +Reviewed-by: Stefan Hajnoczi +Reviewed-by: Raphael Norwitz +Reviewed-by: Philippe Mathieu-Daudé +Tested-by: Philippe Mathieu-Daudé +Signed-off-by: Kevin Wolf +Signed-off-by: Jose R. Ziviani +--- + hw/block/vhost-user-blk.c | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c +index 0b5b9d44cdb0ed4d4a43974e7cdd..f5e9682703f3433c4b363003f90f 100644 +--- a/hw/block/vhost-user-blk.c ++++ b/hw/block/vhost-user-blk.c +@@ -467,6 +467,11 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) + error_setg(errp, "vhost-user-blk: queue size must be non-zero"); + return; + } ++ if (s->queue_size > VIRTQUEUE_MAX_SIZE) { ++ error_setg(errp, "vhost-user-blk: queue size must not exceed %d", ++ VIRTQUEUE_MAX_SIZE); ++ return; ++ } + + if (!vhost_user_init(&s->vhost_user, &s->chardev, errp)) { + return; diff --git a/vhost-user-blk-Get-more-feature-flags-fr.patch b/vhost-user-blk-Get-more-feature-flags-fr.patch new file mode 100644 index 00000000..2d3b1d3a --- /dev/null +++ b/vhost-user-blk-Get-more-feature-flags-fr.patch @@ -0,0 +1,35 @@ +From: Kevin Wolf +Date: Thu, 29 Apr 2021 19:13:14 +0200 +Subject: vhost-user-blk: Get more feature flags from vhost device + +Git-commit: 7556a320c98812ca6648b707393f4513387faf73 + +VIRTIO_F_RING_PACKED and VIRTIO_F_IOMMU_PLATFORM need to be supported by +the vhost device, otherwise advertising it to the guest doesn't result +in a working configuration. They are currently not supported by the +vhost-user-blk export in QEMU. + +Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935020 +Signed-off-by: Kevin Wolf +Acked-by: Raphael Norwitz +Message-Id: <20210429171316.162022-5-kwolf@redhat.com> +Reviewed-by: Michael S. Tsirkin +Signed-off-by: Kevin Wolf +Signed-off-by: Jose R. Ziviani +--- + hw/block/vhost-user-blk.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c +index c0b9958da1b4e155e063fb3426d0..738e8498b4a1d650047f7190c435 100644 +--- a/hw/block/vhost-user-blk.c ++++ b/hw/block/vhost-user-blk.c +@@ -47,6 +47,8 @@ static const int user_feature_bits[] = { + VIRTIO_RING_F_INDIRECT_DESC, + VIRTIO_RING_F_EVENT_IDX, + VIRTIO_F_NOTIFY_ON_EMPTY, ++ VIRTIO_F_RING_PACKED, ++ VIRTIO_F_IOMMU_PLATFORM, + VHOST_INVALID_FEATURE_BIT + }; + diff --git a/vhost-user-blk-Make-sure-to-set-Error-on.patch b/vhost-user-blk-Make-sure-to-set-Error-on.patch new file mode 100644 index 00000000..d6a42643 --- /dev/null +++ b/vhost-user-blk-Make-sure-to-set-Error-on.patch @@ -0,0 +1,44 @@ +From: Kevin Wolf +Date: Thu, 29 Apr 2021 19:13:11 +0200 +Subject: vhost-user-blk: Make sure to set Error on realize failure + +Git-commit: f26729715ef21325f972f693607580a829ad1cbb + +We have to set errp before jumping to virtio_err, otherwise the caller +(virtio_device_realize()) will take this as success and crash when it +later tries to access things that we've already freed in the error path. + +Fixes: 77542d431491788d1e8e79d93ce10172ef207775 +Signed-off-by: Kevin Wolf +Message-Id: <20210429171316.162022-2-kwolf@redhat.com> +Reviewed-by: Michael S. Tsirkin +Reviewed-by: Eric Blake +Acked-by: Raphael Norwitz +Signed-off-by: Kevin Wolf +Signed-off-by: Jose R. Ziviani +--- + hw/block/vhost-user-blk.c | 4 +--- + 1 file changed, 1 insertion(+), 3 deletions(-) + +diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c +index f5e9682703f3433c4b363003f90f..7c85248a7b78b9d9ec8614a3b5fe 100644 +--- a/hw/block/vhost-user-blk.c ++++ b/hw/block/vhost-user-blk.c +@@ -447,7 +447,6 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) + { + VirtIODevice *vdev = VIRTIO_DEVICE(dev); + VHostUserBlk *s = VHOST_USER_BLK(vdev); +- Error *err = NULL; + int i, ret; + + if (!s->chardev.chr) { +@@ -495,8 +494,7 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) + NULL, true); + + reconnect: +- if (qemu_chr_fe_wait_connected(&s->chardev, &err) < 0) { +- error_report_err(err); ++ if (qemu_chr_fe_wait_connected(&s->chardev, errp) < 0) { + goto virtio_err; + } + diff --git a/virtio-Fail-if-iommu_platform-is-request.patch b/virtio-Fail-if-iommu_platform-is-request.patch new file mode 100644 index 00000000..ccf3a99d --- /dev/null +++ b/virtio-Fail-if-iommu_platform-is-request.patch @@ -0,0 +1,44 @@ +From: Kevin Wolf +Date: Thu, 29 Apr 2021 19:13:15 +0200 +Subject: virtio: Fail if iommu_platform is requested, but unsupported + +Git-commit: 04ceb61a4075fadbf374ef89662c41999da83489 + +Commit 2943b53f6 (' virtio: force VIRTIO_F_IOMMU_PLATFORM') made sure +that vhost can't just reject VIRTIO_F_IOMMU_PLATFORM when it was +requested. However, just adding it back to the negotiated flags isn't +right either because it promises support to the guest that the device +actually doesn't support. One example of a vhost-user device that +doesn't have support for the flag is the vhost-user-blk export of QEMU. + +Instead of successfully creating a device that doesn't work, just fail +to plug the device when it doesn't support the feature, but it was +requested. This results in much clearer error messages. + +Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935019 +Signed-off-by: Kevin Wolf +Reviewed-by: Raphael Norwitz +Message-Id: <20210429171316.162022-6-kwolf@redhat.com> +Reviewed-by: Michael S. Tsirkin +Signed-off-by: Kevin Wolf +Signed-off-by: Jose R. Ziviani +--- + hw/virtio/virtio-bus.c | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c +index d6332d45c3b201d6528d84306da9..859978d24877a04ed5eaa03d060d 100644 +--- a/hw/virtio/virtio-bus.c ++++ b/hw/virtio/virtio-bus.c +@@ -69,6 +69,11 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) + return; + } + ++ if (has_iommu && !virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { ++ error_setg(errp, "iommu_platform=true is not supported by the device"); ++ return; ++ } ++ + if (klass->device_plugged != NULL) { + klass->device_plugged(qbus->parent, &local_err); + } diff --git a/virtio-blk-Fix-rollback-path-in-virtio_b.patch b/virtio-blk-Fix-rollback-path-in-virtio_b.patch new file mode 100644 index 00000000..4673d4a8 --- /dev/null +++ b/virtio-blk-Fix-rollback-path-in-virtio_b.patch @@ -0,0 +1,68 @@ +From: Greg Kurz +Date: Wed, 7 Apr 2021 16:34:58 +0200 +Subject: virtio-blk: Fix rollback path in virtio_blk_data_plane_start() + +Git-commit: 570fe439e5d1b8626cf344c6bc97d90cfcaf0c79 + +When dataplane multiqueue support was added in QEMU 2.7, the path +that would rollback guest notifiers assignment in case of error +simply got dropped. + +Later on, when Error was added to blk_set_aio_context() in QEMU 4.1, +another error path was introduced, but it ommits to rollback both +host and guest notifiers. + +It seems cleaner to fix the rollback path in one go. The patch is +simple enough that it can be adjusted if backported to a pre-4.1 +QEMU. + +Fixes: 51b04ac5c6a6 ("virtio-blk: dataplane multiqueue support") +Cc: stefanha@redhat.com +Fixes: 97896a4887a0 ("block: Add Error to blk_set_aio_context()") +Cc: kwolf@redhat.com +Signed-off-by: Greg Kurz +Reviewed-by: Stefan Hajnoczi +Message-Id: <20210407143501.244343-2-groug@kaod.org> +Reviewed-by: Michael S. Tsirkin +Signed-off-by: Michael S. Tsirkin +Signed-off-by: Jose R. Ziviani +--- + hw/block/dataplane/virtio-blk.c | 11 +++++++++-- + 1 file changed, 9 insertions(+), 2 deletions(-) + +diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c +index e9050c8987e7d4c8496135dd87ea..d7b5c95d26d9ec818118513b40c3 100644 +--- a/hw/block/dataplane/virtio-blk.c ++++ b/hw/block/dataplane/virtio-blk.c +@@ -207,7 +207,7 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) + virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false); + virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i); + } +- goto fail_guest_notifiers; ++ goto fail_host_notifiers; + } + } + +@@ -221,7 +221,7 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) + aio_context_release(old_context); + if (r < 0) { + error_report_err(local_err); +- goto fail_guest_notifiers; ++ goto fail_aio_context; + } + + /* Process queued requests before the ones in vring */ +@@ -245,6 +245,13 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) + aio_context_release(s->ctx); + return 0; + ++ fail_aio_context: ++ for (i = 0; i < nvqs; i++) { ++ virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false); ++ virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i); ++ } ++ fail_host_notifiers: ++ k->set_guest_notifiers(qbus->parent, nvqs, false); + fail_guest_notifiers: + /* + * If we failed to set up the guest notifiers queued requests will be diff --git a/virtiofsd-Fix-side-effect-in-assert.patch b/virtiofsd-Fix-side-effect-in-assert.patch new file mode 100644 index 00000000..af127e5f --- /dev/null +++ b/virtiofsd-Fix-side-effect-in-assert.patch @@ -0,0 +1,100 @@ +From: Greg Kurz +Date: Fri, 9 Apr 2021 12:06:27 +0200 +Subject: virtiofsd: Fix side-effect in assert() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Git-commit: 0adb3aff3932d05b069bd2cb13480f1611cce654 + +It is bad practice to put an expression with a side-effect in +assert() because the side-effect won't happen if the code is +compiled with -DNDEBUG. + +Use an intermediate variable. Consolidate this in an macro to +have proper line numbers when the assertion is hit. + +virtiofsd: ../../tools/virtiofsd/passthrough_ll.c:2797: lo_getxattr: + Assertion `fchdir_res == 0' failed. +Aborted + + 2796 /* fchdir should not fail here */ +=>2797 FCHDIR_NOFAIL(lo->proc_self_fd); + 2798 ret = getxattr(procname, name, value, size); + 2799 FCHDIR_NOFAIL(lo->root.fd); + +Fixes: bdfd66788349 ("virtiofsd: Fix xattr operations") +Cc: misono.tomohiro@jp.fujitsu.com +Signed-off-by: Greg Kurz +Message-Id: <20210409100627.451573-1-groug@kaod.org> +Signed-off-by: Dr. David Alan Gilbert +Reviewed-by: Philippe Mathieu-Daudé +Signed-off-by: Jose R. Ziviani +--- + tools/virtiofsd/passthrough_ll.c | 21 +++++++++++++-------- + 1 file changed, 13 insertions(+), 8 deletions(-) + +diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c +index 1553d2ef454f55a3103b452841d5..6592f96f685e52fecf5703739e7d 100644 +--- a/tools/virtiofsd/passthrough_ll.c ++++ b/tools/virtiofsd/passthrough_ll.c +@@ -2723,6 +2723,11 @@ static int xattr_map_server(const struct lo_data *lo, const char *server_name, + return -ENODATA; + } + ++#define FCHDIR_NOFAIL(fd) do { \ ++ int fchdir_res = fchdir(fd); \ ++ assert(fchdir_res == 0); \ ++ } while (0) ++ + static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, + size_t size) + { +@@ -2789,9 +2794,9 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, + ret = fgetxattr(fd, name, value, size); + } else { + /* fchdir should not fail here */ +- assert(fchdir(lo->proc_self_fd) == 0); ++ FCHDIR_NOFAIL(lo->proc_self_fd); + ret = getxattr(procname, name, value, size); +- assert(fchdir(lo->root.fd) == 0); ++ FCHDIR_NOFAIL(lo->root.fd); + } + + if (ret == -1) { +@@ -2864,9 +2869,9 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) + ret = flistxattr(fd, value, size); + } else { + /* fchdir should not fail here */ +- assert(fchdir(lo->proc_self_fd) == 0); ++ FCHDIR_NOFAIL(lo->proc_self_fd); + ret = listxattr(procname, value, size); +- assert(fchdir(lo->root.fd) == 0); ++ FCHDIR_NOFAIL(lo->root.fd); + } + + if (ret == -1) { +@@ -3000,9 +3005,9 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, + ret = fsetxattr(fd, name, value, size, flags); + } else { + /* fchdir should not fail here */ +- assert(fchdir(lo->proc_self_fd) == 0); ++ FCHDIR_NOFAIL(lo->proc_self_fd); + ret = setxattr(procname, name, value, size, flags); +- assert(fchdir(lo->root.fd) == 0); ++ FCHDIR_NOFAIL(lo->root.fd); + } + + saverr = ret == -1 ? errno : 0; +@@ -3066,9 +3071,9 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name) + ret = fremovexattr(fd, name); + } else { + /* fchdir should not fail here */ +- assert(fchdir(lo->proc_self_fd) == 0); ++ FCHDIR_NOFAIL(lo->proc_self_fd); + ret = removexattr(procname, name); +- assert(fchdir(lo->root.fd) == 0); ++ FCHDIR_NOFAIL(lo->root.fd); + } + + saverr = ret == -1 ? errno : 0;