[dpdk-stable] patch 'vhost: fix error path when setting memory tables' has been queued to stable release 19.11.6
luca.boccassi at gmail.com
luca.boccassi at gmail.com
Tue Nov 17 12:14:02 CET 2020
Hi,
FYI, your patch has been queued to stable release 19.11.6
Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 11/19/20. So please
shout if anyone has objections.
Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.
Queued patches are on a temporary branch at:
https://github.com/bluca/dpdk-stable
This queued commit can be viewed at:
https://github.com/bluca/dpdk-stable/commit/f650e9c222af91a4f639a2b2dd94a1c96700e8f4
Thanks.
Luca Boccassi
---
>From f650e9c222af91a4f639a2b2dd94a1c96700e8f4 Mon Sep 17 00:00:00 2001
From: Maxime Coquelin <maxime.coquelin at redhat.com>
Date: Thu, 12 Nov 2020 18:10:27 +0100
Subject: [PATCH] vhost: fix error path when setting memory tables
[ upstream commit 726a14eb83a594011aba5e09159b47f12bc1bad0 ]
If an error is encountered before the memory regions are
parsed, the file descriptors for these shared buffers are
leaked.
This patch fixes this by closing the message file descriptors
on error, taking care of avoiding double closing of the file
descriptors. guest_pages is also freed, even though it was not
leaked as its pointer was not overridden on subsequent function
calls.
Fixes: 8f972312b8f4 ("vhost: support vhost-user")
Reported-by: Xuan Ding <xuan.ding at intel.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin at redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia at intel.com>
Reviewed-by: Xueming Li <xuemingl at nvidia.com>
---
lib/librte_vhost/vhost_user.c | 60 ++++++++++++++++++++++-------------
1 file changed, 38 insertions(+), 22 deletions(-)
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 647f0e9cde..f8366c9be2 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -97,8 +97,15 @@ close_msg_fds(struct VhostUserMsg *msg)
{
int i;
- for (i = 0; i < msg->fd_num; i++)
- close(msg->fds[i]);
+ for (i = 0; i < msg->fd_num; i++) {
+ int fd = msg->fds[i];
+
+ if (fd == -1)
+ continue;
+
+ msg->fds[i] = -1;
+ close(fd);
+ }
}
/*
@@ -1032,7 +1039,6 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
uint64_t alignment;
uint32_t i;
int populate;
- int fd;
if (validate_msg_fds(msg, memory->nregions) != 0)
return RTE_VHOST_MSG_RESULT_ERR;
@@ -1040,7 +1046,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
if (memory->nregions > VHOST_MEMORY_MAX_NREGIONS) {
RTE_LOG(ERR, VHOST_CONFIG,
"too many memory regions (%u)\n", memory->nregions);
- return RTE_VHOST_MSG_RESULT_ERR;
+ goto close_msg_fds;
}
if (dev->mem && !vhost_memory_changed(memory, dev->mem)) {
@@ -1073,7 +1079,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
"(%d) failed to allocate memory "
"for dev->guest_pages\n",
dev->vid);
- return RTE_VHOST_MSG_RESULT_ERR;
+ goto close_msg_fds;
}
}
@@ -1083,18 +1089,23 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
RTE_LOG(ERR, VHOST_CONFIG,
"(%d) failed to allocate memory for dev->mem\n",
dev->vid);
- return RTE_VHOST_MSG_RESULT_ERR;
+ goto free_guest_pages;
}
dev->mem->nregions = memory->nregions;
for (i = 0; i < memory->nregions; i++) {
- fd = msg->fds[i];
reg = &dev->mem->regions[i];
reg->guest_phys_addr = memory->regions[i].guest_phys_addr;
reg->guest_user_addr = memory->regions[i].userspace_addr;
reg->size = memory->regions[i].memory_size;
- reg->fd = fd;
+ reg->fd = msg->fds[i];
+
+ /*
+ * Assign invalid file descriptor value to avoid double
+ * closing on error path.
+ */
+ msg->fds[i] = -1;
mmap_offset = memory->regions[i].mmap_offset;
@@ -1104,7 +1115,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
"mmap_offset (%#"PRIx64") and memory_size "
"(%#"PRIx64") overflow\n",
mmap_offset, reg->size);
- goto err_mmap;
+ goto free_mem_table;
}
mmap_size = reg->size + mmap_offset;
@@ -1117,11 +1128,11 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
* to avoid failure, make sure in caller to keep length
* aligned.
*/
- alignment = get_blk_size(fd);
+ alignment = get_blk_size(reg->fd);
if (alignment == (uint64_t)-1) {
RTE_LOG(ERR, VHOST_CONFIG,
"couldn't get hugepage size through fstat\n");
- goto err_mmap;
+ goto free_mem_table;
}
mmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);
if (mmap_size == 0) {
@@ -1137,17 +1148,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
RTE_LOG(ERR, VHOST_CONFIG, "mmap size (0x%" PRIx64 ") "
"or alignment (0x%" PRIx64 ") is invalid\n",
reg->size + mmap_offset, alignment);
- goto err_mmap;
+ goto free_mem_table;
}
populate = (dev->dequeue_zero_copy) ? MAP_POPULATE : 0;
mmap_addr = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
- MAP_SHARED | populate, fd, 0);
+ MAP_SHARED | populate, reg->fd, 0);
if (mmap_addr == MAP_FAILED) {
RTE_LOG(ERR, VHOST_CONFIG,
"mmap region %u failed.\n", i);
- goto err_mmap;
+ goto free_mem_table;
}
reg->mmap_addr = mmap_addr;
@@ -1160,7 +1171,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
RTE_LOG(ERR, VHOST_CONFIG,
"adding guest pages to region %u failed.\n",
i);
- goto err_mmap;
+ goto free_mem_table;
}
RTE_LOG(INFO, VHOST_CONFIG,
@@ -1203,17 +1214,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
if (read_vhost_message(main_fd, &ack_msg) <= 0) {
RTE_LOG(ERR, VHOST_CONFIG,
"Failed to read qemu ack on postcopy set-mem-table\n");
- goto err_mmap;
+ goto free_mem_table;
}
if (validate_msg_fds(&ack_msg, 0) != 0)
- goto err_mmap;
+ goto free_mem_table;
if (ack_msg.request.master != VHOST_USER_SET_MEM_TABLE) {
RTE_LOG(ERR, VHOST_CONFIG,
"Bad qemu ack on postcopy set-mem-table (%d)\n",
ack_msg.request.master);
- goto err_mmap;
+ goto free_mem_table;
}
/* Now userfault register and we can use the memory */
@@ -1237,7 +1248,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
"Failed to register ufd for region %d: (ufd = %d) %s\n",
i, dev->postcopy_ufd,
strerror(errno));
- goto err_mmap;
+ goto free_mem_table;
}
RTE_LOG(INFO, VHOST_CONFIG,
"\t userfaultfd registered for range : "
@@ -1246,7 +1257,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
(uint64_t)reg_struct.range.start +
(uint64_t)reg_struct.range.len - 1);
#else
- goto err_mmap;
+ goto free_mem_table;
#endif
}
}
@@ -1265,7 +1276,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
dev = translate_ring_addresses(dev, i);
if (!dev) {
dev = *pdev;
- goto err_mmap;
+ goto free_mem_table;
}
*pdev = dev;
@@ -1276,10 +1287,15 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
return RTE_VHOST_MSG_RESULT_OK;
-err_mmap:
+free_mem_table:
free_mem_region(dev);
rte_free(dev->mem);
dev->mem = NULL;
+free_guest_pages:
+ rte_free(dev->guest_pages);
+ dev->guest_pages = NULL;
+close_msg_fds:
+ close_msg_fds(msg);
return RTE_VHOST_MSG_RESULT_ERR;
}
--
2.27.0
---
Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- - 2020-11-17 11:13:14.165782941 +0000
+++ 0028-vhost-fix-error-path-when-setting-memory-tables.patch 2020-11-17 11:13:12.933116851 +0000
@@ -1 +1 @@
-From 726a14eb83a594011aba5e09159b47f12bc1bad0 Mon Sep 17 00:00:00 2001
+From f650e9c222af91a4f639a2b2dd94a1c96700e8f4 Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit 726a14eb83a594011aba5e09159b47f12bc1bad0 ]
+
@@ -17 +18,0 @@
-Cc: stable at dpdk.org
@@ -28 +29 @@
-index 8a8726f8b8..3898c93d1f 100644
+index 647f0e9cde..f8366c9be2 100644
@@ -31 +32 @@
-@@ -99,8 +99,15 @@ close_msg_fds(struct VhostUserMsg *msg)
+@@ -97,8 +97,15 @@ close_msg_fds(struct VhostUserMsg *msg)
@@ -49 +50 @@
-@@ -1004,7 +1011,6 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1032,7 +1039,6 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -57 +58 @@
-@@ -1012,7 +1018,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1040,7 +1046,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -59 +60 @@
- VHOST_LOG_CONFIG(ERR,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -66 +67 @@
-@@ -1054,7 +1060,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1073,7 +1079,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -75,2 +76,2 @@
-@@ -1064,18 +1070,23 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
- VHOST_LOG_CONFIG(ERR,
+@@ -1083,18 +1089,23 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -102 +103 @@
-@@ -1085,7 +1096,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1104,7 +1115,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -111 +112 @@
-@@ -1098,11 +1109,11 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1117,11 +1128,11 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -118 +119 @@
- VHOST_LOG_CONFIG(ERR,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -125,2 +126,2 @@
-@@ -1118,17 +1129,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
- VHOST_LOG_CONFIG(ERR, "mmap size (0x%" PRIx64 ") "
+@@ -1137,17 +1148,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+ RTE_LOG(ERR, VHOST_CONFIG, "mmap size (0x%" PRIx64 ") "
@@ -133 +134 @@
- populate = dev->async_copy ? MAP_POPULATE : 0;
+ populate = (dev->dequeue_zero_copy) ? MAP_POPULATE : 0;
@@ -139 +140 @@
- VHOST_LOG_CONFIG(ERR,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -146,2 +147,2 @@
-@@ -1141,7 +1152,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
- VHOST_LOG_CONFIG(ERR,
+@@ -1160,7 +1171,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -154,2 +155,2 @@
- VHOST_LOG_CONFIG(INFO,
-@@ -1184,17 +1195,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+ RTE_LOG(INFO, VHOST_CONFIG,
+@@ -1203,17 +1214,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -157 +158 @@
- VHOST_LOG_CONFIG(ERR,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -168 +169 @@
- VHOST_LOG_CONFIG(ERR,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -176 +177 @@
-@@ -1218,7 +1229,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1237,7 +1248,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -183 +184 @@
- VHOST_LOG_CONFIG(INFO,
+ RTE_LOG(INFO, VHOST_CONFIG,
@@ -185 +186 @@
-@@ -1227,7 +1238,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1246,7 +1257,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -194 +195 @@
-@@ -1249,7 +1260,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1265,7 +1276,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -203 +204 @@
-@@ -1260,10 +1271,15 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1276,10 +1287,15 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
More information about the stable
mailing list