From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5ED5FA04DB for ; Tue, 17 Nov 2020 12:15:53 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5403C5916; Tue, 17 Nov 2020 12:15:52 +0100 (CET) Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) by dpdk.org (Postfix) with ESMTP id DF4D34C90 for ; Tue, 17 Nov 2020 12:15:50 +0100 (CET) Received: by mail-wm1-f53.google.com with SMTP id a3so2837833wmb.5 for ; Tue, 17 Nov 2020 03:15:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=A8l9YJSDq+K6e8kI89sjvhzdQxljSPaO6cmrQSx9r2I=; b=HyGQYMTFY/RFvQ/qYe0fDbbD/TAP+MpV4W5Z39t8zaZZqdnOusR06Ey/SSouKQ/49D mxevVtUzfpah7jwMTpvsiq/vL8s2pPghAueuaIftZONEhczrAwAslBncaMCXvmwBKVfP VD62QkbPvp2f3blNbihieh4tGqnVFGLspWEGWvu4M/RHp5BRMylHpASoAs5mvzljB526 t/V6G0CTVwZUGOLMW+ZRcisNavE4bIlles5qfNWM4/u6gDy5XuHE+XFiXuDgskvEkvJE ej37mxi3ctyv75ejOWm/W/HKzWyeBfmFeifxSk0o4DozTUblDMp0SzUH4/DXOlbK7D9v XzQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=A8l9YJSDq+K6e8kI89sjvhzdQxljSPaO6cmrQSx9r2I=; b=hxjYXf5Tl81b9I2DppnnMyoGWs9P6TdNgRZfsas3OmL2MOU8/JBNfNrFbC4Uc7Lfm4 x+P0CIp7dBERMrYn33k+DhrL9brtKZAafLd83XeVACZpVG1y/5lk4hcqk7gsTeD3GEYB lT9YKq6utsKfrJLFayzyHr4IIoB+FPLSPfuMgv8RBHPOzVvVOX9CNFaeoi8lg+SC5K+D I7jPE3bbUEZLkV+M0zfKYMGEGVwWjWqB8hSDjfbcqpGjXFirEoPWo9+fG+7tpcYN4uzL brQuxxPVaJMTn6i8a28tt3DHbV8Hbf3+m6t6qRiRDe9lb3/heeJnf7edNXnm1YtO7Kgt NLXw== X-Gm-Message-State: AOAM532rPW2A46pDvCBrAN9pT+YC2OUSDFJXKYET056FSQOg+NENyBFx Y9KsDH1Ht2X/SZHurCTq+Rw= X-Google-Smtp-Source: ABdhPJysq4WR+yCUuWO/HCo9fYfUC4A/gHNWAppUF5MgPgZHMR8bih3/dr9p8BSUUIkHXLrw9YtpPQ== X-Received: by 2002:a1c:6002:: with SMTP id u2mr3526203wmb.29.1605611749641; Tue, 17 Nov 2020 03:15:49 -0800 (PST) Received: from localhost ([88.98.246.218]) by smtp.gmail.com with ESMTPSA id n10sm28076812wrx.9.2020.11.17.03.15.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 17 Nov 2020 03:15:48 -0800 (PST) From: luca.boccassi@gmail.com To: Maxime Coquelin Cc: Xuan Ding , Chenbo Xia , Xueming Li , dpdk stable Date: Tue, 17 Nov 2020 11:14:02 +0000 Message-Id: <20201117111416.1961049-28-luca.boccassi@gmail.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201117111416.1961049-1-luca.boccassi@gmail.com> References: <20201109185030.3478760-2-luca.boccassi@gmail.com> <20201117111416.1961049-1-luca.boccassi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'vhost: fix error path when setting memory tables' has been queued to stable release 19.11.6 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" 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 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 Signed-off-by: Maxime Coquelin Reviewed-by: Chenbo Xia Reviewed-by: Xueming Li --- 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@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,