[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL v2 03/19] vhost-user: Fix double-close on slave_read() error path
From: |
Michael S. Tsirkin |
Subject: |
[PULL v2 03/19] vhost-user: Fix double-close on slave_read() error path |
Date: |
Mon, 22 Mar 2021 18:59:59 -0400 |
From: Greg Kurz <groug@kaod.org>
Some message types, e.g. VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG,
can convey file descriptors. These must be closed before returning
from slave_read() to avoid being leaked. This can currently be done
in two different places:
[1] just after the request has been processed
[2] on the error path, under the goto label err:
These path are supposed to be mutually exclusive but they are not
actually. If the VHOST_USER_NEED_REPLY_MASK flag was passed and the
sending of the reply fails, both [1] and [2] are performed with the
same descriptor values. This can potentially cause subtle bugs if one
of the descriptor was recycled by some other thread in the meantime.
This code duplication complicates rollback for no real good benefit.
Do the closing in a unique place, under a new fdcleanup: goto label
at the end of the function.
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <20210312092212.782255-3-groug@kaod.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/virtio/vhost-user.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 6af9b43a72..acde1d2936 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1475,13 +1475,6 @@ static void slave_read(void *opaque)
ret = -EINVAL;
}
- /* Close the remaining file descriptors. */
- for (i = 0; i < fdsize; i++) {
- if (fd[i] != -1) {
- close(fd[i]);
- }
- }
-
/*
* REPLY_ACK feature handling. Other reply types has to be managed
* directly in their request handlers.
@@ -1511,12 +1504,14 @@ static void slave_read(void *opaque)
}
}
- return;
+ goto fdcleanup;
err:
qemu_set_fd_handler(u->slave_fd, NULL, NULL, NULL);
close(u->slave_fd);
u->slave_fd = -1;
+
+fdcleanup:
for (i = 0; i < fdsize; i++) {
if (fd[i] != -1) {
close(fd[i]);
--
MST
- [PULL v2 00/19] pc,virtio,pci: fixes, features, Michael S. Tsirkin, 2021/03/22
- [PULL v2 02/19] vhost-user: Drop misleading EAGAIN checks in slave_read(), Michael S. Tsirkin, 2021/03/22
- [PULL v2 01/19] virtio: Fix virtio_mmio_read()/virtio_mmio_write(), Michael S. Tsirkin, 2021/03/22
- [PULL v2 04/19] vhost-user: Factor out duplicated slave_fd teardown code, Michael S. Tsirkin, 2021/03/22
- [PULL v2 03/19] vhost-user: Fix double-close on slave_read() error path,
Michael S. Tsirkin <=
- [PULL v2 05/19] vhost-user: Convert slave channel to QIOChannelSocket, Michael S. Tsirkin, 2021/03/22
- [PULL v2 06/19] vhost-user: Introduce nested event loop in vhost_user_read(), Michael S. Tsirkin, 2021/03/22
- [PULL v2 07/19] vhost-user: Monitor slave channel in vhost_user_read(), Michael S. Tsirkin, 2021/03/22
- [PULL v2 08/19] virtio-pmem: fix virtio_pmem_resp assign problem, Michael S. Tsirkin, 2021/03/22
- [PULL v2 09/19] tests: acpi: temporary whitelist DSDT changes, Michael S. Tsirkin, 2021/03/22
- [PULL v2 10/19] pci: introduce acpi-index property for PCI device, Michael S. Tsirkin, 2021/03/22
- [PULL v2 12/19] acpi: add aml_to_decimalstring() and aml_call6() helpers, Michael S. Tsirkin, 2021/03/22
- [PULL v2 11/19] pci: acpi: ensure that acpi-index is unique, Michael S. Tsirkin, 2021/03/22
- [PULL v2 15/19] acpi: Set proper maximum size for "etc/table-loader" blob, Michael S. Tsirkin, 2021/03/22
- [PULL v2 13/19] pci: acpi: add _DSM method to PCI devices, Michael S. Tsirkin, 2021/03/22