From: Kevin Traynor <ktraynor@redhat.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>
Cc: Xuan Ding <xuan.ding@intel.com>,
Chenbo Xia <chenbo.xia@intel.com>,
Xueming Li <xuemingl@nvidia.com>, dpdk stable <stable@dpdk.org>
Subject: [dpdk-stable] patch 'vhost: fix error path when setting memory tables' has been queued to LTS release 18.11.11
Date: Mon, 23 Nov 2020 17:12:06 +0000 [thread overview]
Message-ID: <20201123171222.79398-14-ktraynor@redhat.com> (raw)
In-Reply-To: <20201123171222.79398-1-ktraynor@redhat.com>
Hi,
FYI, your patch has been queued to LTS release 18.11.11
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/27/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/kevintraynor/dpdk-stable-queue
This queued commit can be viewed at:
https://github.com/kevintraynor/dpdk-stable-queue/commit/36b1fdd3248fcf1a87eded42f82ca2648fa54293
Thanks.
Kevin.
---
From 36b1fdd3248fcf1a87eded42f82ca2648fa54293 Mon Sep 17 00:00:00 2001
From: Maxime Coquelin <maxime.coquelin@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@intel.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
Reviewed-by: Xueming Li <xuemingl@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 754759f6e0..f35ea97b6a 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -89,6 +89,13 @@ 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);
+ }
}
@@ -1005,5 +1012,4 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
uint32_t i;
int populate;
- int fd;
if (validate_msg_fds(msg, memory->nregions) != 0)
@@ -1013,5 +1019,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
RTE_LOG(ERR, VHOST_CONFIG,
"too many memory regions (%u)\n", memory->nregions);
- return VH_RESULT_ERR;
+ goto close_msg_fds;
}
@@ -1046,5 +1052,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
"for dev->guest_pages\n",
dev->vid);
- return VH_RESULT_ERR;
+ goto close_msg_fds;
}
}
@@ -1056,10 +1062,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
"(%d) failed to allocate memory for dev->mem\n",
dev->vid);
- return VH_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];
@@ -1067,5 +1072,11 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
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;
@@ -1077,5 +1088,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
"(%#"PRIx64") overflow\n",
mmap_offset, reg->size);
- goto err_mmap;
+ goto free_mem_table;
}
@@ -1090,9 +1101,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
* 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);
@@ -1110,15 +1121,15 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
"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;
}
@@ -1133,5 +1144,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
"adding guest pages to region %u failed.\n",
i);
- goto err_mmap;
+ goto free_mem_table;
}
@@ -1176,9 +1187,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
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) {
@@ -1186,5 +1197,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
"Bad qemu ack on postcopy set-mem-table (%d)\n",
ack_msg.request.master);
- goto err_mmap;
+ goto free_mem_table;
}
@@ -1210,5 +1221,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
i, dev->postcopy_ufd,
strerror(errno));
- goto err_mmap;
+ goto free_mem_table;
}
RTE_LOG(INFO, VHOST_CONFIG,
@@ -1219,5 +1230,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
(uint64_t)reg_struct.range.len - 1);
#else
- goto err_mmap;
+ goto free_mem_table;
#endif
}
@@ -1238,5 +1249,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
if (!dev) {
dev = *pdev;
- goto err_mmap;
+ goto free_mem_table;
}
@@ -1249,8 +1260,13 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
return VH_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 VH_RESULT_ERR;
}
--
2.26.2
---
Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- - 2020-11-23 17:10:14.328560137 +0000
+++ 0014-vhost-fix-error-path-when-setting-memory-tables.patch 2020-11-23 17:10:13.993061585 +0000
@@ -1 +1 @@
-From 726a14eb83a594011aba5e09159b47f12bc1bad0 Mon Sep 17 00:00:00 2001
+From 36b1fdd3248fcf1a87eded42f82ca2648fa54293 Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit 726a14eb83a594011aba5e09159b47f12bc1bad0 ]
+
@@ -17 +18,0 @@
-Cc: stable@dpdk.org
@@ -28 +29 @@
-index 8a8726f8b8..3898c93d1f 100644
+index 754759f6e0..f35ea97b6a 100644
@@ -31 +32 @@
-@@ -100,6 +100,13 @@ close_msg_fds(struct VhostUserMsg *msg)
+@@ -89,6 +89,13 @@ close_msg_fds(struct VhostUserMsg *msg)
@@ -54 +55 @@
- VHOST_LOG_CONFIG(ERR,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -56 +57 @@
-- return RTE_VHOST_MSG_RESULT_ERR;
+- return VH_RESULT_ERR;
@@ -60 +61 @@
-@@ -1055,5 +1061,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1046,5 +1052,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -63 +64 @@
-- return RTE_VHOST_MSG_RESULT_ERR;
+- return VH_RESULT_ERR;
@@ -67 +68 @@
-@@ -1065,10 +1071,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1056,10 +1062,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -70 +71 @@
-- return RTE_VHOST_MSG_RESULT_ERR;
+- return VH_RESULT_ERR;
@@ -79 +80 @@
-@@ -1076,5 +1081,11 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1067,5 +1072,11 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -92 +93 @@
-@@ -1086,5 +1097,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1077,5 +1088,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -99 +100 @@
-@@ -1099,9 +1110,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1090,9 +1101,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -105 +106 @@
- VHOST_LOG_CONFIG(ERR,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -111 +112 @@
-@@ -1119,15 +1130,15 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1110,15 +1121,15 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -118 +119 @@
- populate = dev->async_copy ? MAP_POPULATE : 0;
+ populate = (dev->dequeue_zero_copy) ? MAP_POPULATE : 0;
@@ -124 +125 @@
- VHOST_LOG_CONFIG(ERR,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -130 +131 @@
-@@ -1142,5 +1153,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1133,5 +1144,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -137,2 +138,2 @@
-@@ -1185,9 +1196,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
- VHOST_LOG_CONFIG(ERR,
+@@ -1176,9 +1187,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+ RTE_LOG(ERR, VHOST_CONFIG,
@@ -149 +150 @@
-@@ -1195,5 +1206,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1186,5 +1197,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -156 +157 @@
-@@ -1219,5 +1230,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1210,5 +1221,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -162,2 +163,2 @@
- VHOST_LOG_CONFIG(INFO,
-@@ -1228,5 +1239,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+ RTE_LOG(INFO, VHOST_CONFIG,
+@@ -1219,5 +1230,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -170 +171 @@
-@@ -1250,5 +1261,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+@@ -1238,5 +1249,5 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
@@ -177,2 +178,2 @@
-@@ -1261,8 +1272,13 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
- return RTE_VHOST_MSG_RESULT_OK;
+@@ -1249,8 +1260,13 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,
+ return VH_RESULT_OK;
@@ -190 +191 @@
- return RTE_VHOST_MSG_RESULT_ERR;
+ return VH_RESULT_ERR;
next prev parent reply other threads:[~2020-11-23 17:13 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-23 17:11 [dpdk-stable] patch 'examples/l2fwd-keepalive: skip meson build if no librt' " Kevin Traynor
2020-11-23 17:11 ` [dpdk-stable] patch 'devtools: fix directory filter in forbidden token check' " Kevin Traynor
2020-11-23 17:11 ` [dpdk-stable] patch 'examples/qos_sched: fix usage string' " Kevin Traynor
2020-11-23 17:11 ` [dpdk-stable] patch 'usertools: fix CPU layout script to be PEP8 compliant' " Kevin Traynor
2020-11-23 17:11 ` [dpdk-stable] patch 'config: enable packet prefetching with Meson' " Kevin Traynor
2020-11-23 17:11 ` [dpdk-stable] patch 'net/ring: fix typo in log message' " Kevin Traynor
2020-11-23 17:11 ` [dpdk-stable] patch 'examples/vhost_crypto: add new line character in usage' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'vhost: fix virtqueue initialization' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'net/i40e: add C++ include guard' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'net/avf: fix releasing mbufs' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'net/avf: fix performance drop after port reset' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'net/i40e: fix build for log format specifier' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'net/ixgbe: remove redundant MAC flag check' " Kevin Traynor
2020-11-23 17:12 ` Kevin Traynor [this message]
2020-11-23 17:12 ` [dpdk-stable] patch 'vhost: fix fd leak in dirty logging setup' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'vhost: fix fd leak in kick " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'app/testpmd: fix MTU after device configure' " Kevin Traynor
2020-11-23 18:35 ` David Marchand
2020-11-24 9:38 ` Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'app/eventdev: check timer adadpters number' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'examples/kni: fix build with pkg-config' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'examples/l2fwd-crypto: " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'examples/performance-thread: " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'doc: fix rule file parameters in l3fwd-acl guide' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'net/mlx5: fix Rx queue count calculation' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'net/mlx5: fix Rx descriptor status' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'app/testpmd: revert setting MTU explicitly after configure' " Kevin Traynor
2020-11-23 18:36 ` David Marchand
2020-11-23 17:12 ` [dpdk-stable] patch 'compress/isal: check allocation in queue setup' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'examples/l3fwd-power: check packet types after start' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'net/mlx5: fix representor interrupts handler' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'malloc: fix style in free list index computation' " Kevin Traynor
2020-11-23 17:12 ` [dpdk-stable] patch 'usertools: fix pmdinfo parsing' " Kevin Traynor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201123171222.79398-14-ktraynor@redhat.com \
--to=ktraynor@redhat.com \
--cc=chenbo.xia@intel.com \
--cc=maxime.coquelin@redhat.com \
--cc=stable@dpdk.org \
--cc=xuan.ding@intel.com \
--cc=xuemingl@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).