From patchwork Mon Nov 9 12:16:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxime Coquelin X-Patchwork-Id: 83843 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0CAD2A0527; Mon, 9 Nov 2020 13:17:07 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DED7F6A1B; Mon, 9 Nov 2020 13:16:46 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by dpdk.org (Postfix) with ESMTP id 91B346883 for ; Mon, 9 Nov 2020 13:16:44 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1604924203; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+dj4pk0M/XfD4ryywtAIkfewrrXOGTCa+kKn+KHtGr8=; b=DzpJB4lW4uyw8Prryyzi0kc7+FFWjIiCJd2uQ1yohf+hEAktqEhpDGbQASYCUnKGTSz71V kkcbRpo5GPis2XjUvzxUDYWla3UpSWg+XnzHKjgE10Enh9DHhCz6/8Tqjr7FqzqApMHcH1 gmXbwu5mPacdCXM5nCLyyuIhq7Eu5ds= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-460-EmD_X06ZNvGasGeiJgxi5g-1; Mon, 09 Nov 2020 07:16:41 -0500 X-MC-Unique: EmD_X06ZNvGasGeiJgxi5g-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2DF691007467; Mon, 9 Nov 2020 12:16:40 +0000 (UTC) Received: from localhost.localdomain (unknown [10.36.110.32]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2FC786EF5C; Mon, 9 Nov 2020 12:16:37 +0000 (UTC) From: Maxime Coquelin To: dev@dpdk.org, xuan.ding@intel.com, stephen@networkplumber.org, thomas@monjalon.net, stable@dpdk.org, chenbo.xia@intel.com Cc: Maxime Coquelin Date: Mon, 9 Nov 2020 13:16:28 +0100 Message-Id: <20201109121630.251603-2-maxime.coquelin@redhat.com> In-Reply-To: <20201109121630.251603-1-maxime.coquelin@redhat.com> References: <20201109121630.251603-1-maxime.coquelin@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [dpdk-dev] [PATCH v3 1/3] vhost: fix error path when setting memory tables X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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") Cc: stable@dpdk.org Reported-by: Xuan Ding Signed-off-by: Maxime Coquelin Reviewed-by: Chenbo Xia --- lib/librte_vhost/vhost_user.c | 65 +++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 8a8726f8b8..473fd778ca 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -99,8 +99,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); + } } /* @@ -1004,7 +1011,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; @@ -1012,16 +1018,13 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg, if (memory->nregions > VHOST_MEMORY_MAX_NREGIONS) { VHOST_LOG_CONFIG(ERR, "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)) { VHOST_LOG_CONFIG(INFO, "(%d) memory regions not changed\n", dev->vid); - - close_msg_fds(msg); - - return RTE_VHOST_MSG_RESULT_OK; + goto close_msg_fds; } if (dev->mem) { @@ -1054,7 +1057,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; } } @@ -1064,18 +1067,23 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg, VHOST_LOG_CONFIG(ERR, "(%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; @@ -1085,7 +1093,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; @@ -1098,11 +1106,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) { VHOST_LOG_CONFIG(ERR, "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) { @@ -1118,17 +1126,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg, VHOST_LOG_CONFIG(ERR, "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->async_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) { VHOST_LOG_CONFIG(ERR, "mmap region %u failed.\n", i); - goto err_mmap; + goto free_mem_table; } reg->mmap_addr = mmap_addr; @@ -1141,7 +1149,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg, VHOST_LOG_CONFIG(ERR, "adding guest pages to region %u failed.\n", i); - goto err_mmap; + goto free_mem_table; } VHOST_LOG_CONFIG(INFO, @@ -1184,17 +1192,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg, if (read_vhost_message(main_fd, &ack_msg) <= 0) { VHOST_LOG_CONFIG(ERR, "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) { VHOST_LOG_CONFIG(ERR, "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 */ @@ -1218,7 +1226,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; } VHOST_LOG_CONFIG(INFO, "\t userfaultfd registered for range : " @@ -1227,7 +1235,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 } } @@ -1249,7 +1257,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; @@ -1260,10 +1268,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; } From patchwork Mon Nov 9 12:16:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxime Coquelin X-Patchwork-Id: 83844 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 563C8A0527; Mon, 9 Nov 2020 13:17:35 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 074587CC8; Mon, 9 Nov 2020 13:16:50 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by dpdk.org (Postfix) with ESMTP id C35296CA9 for ; Mon, 9 Nov 2020 13:16:46 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1604924205; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZO7vHp0RgtBAH669CQ1G2/wdl+9SOwwOtctncF6EPsg=; b=JHifnuQMD3+WQbdFzgbado4FqCGiv2WRBUTPkwEBxVD4lNqn2Vgadx7STjF0Y67Nc678nD X91Ua/EvvWa8I4Uu92xSqntoqWV7+AOXHyv1KJbGYagqGcsd2f3XOYsKu/wNXk4I6X8z3v 2fxqt9KHZVGXB+qhkirLYAJAXKhoylA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-473-ECa9rXBvMC-DraRS8-Wvew-1; Mon, 09 Nov 2020 07:16:43 -0500 X-MC-Unique: ECa9rXBvMC-DraRS8-Wvew-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2E98F1007464; Mon, 9 Nov 2020 12:16:42 +0000 (UTC) Received: from localhost.localdomain (unknown [10.36.110.32]) by smtp.corp.redhat.com (Postfix) with ESMTP id 85FF36EF5C; Mon, 9 Nov 2020 12:16:40 +0000 (UTC) From: Maxime Coquelin To: dev@dpdk.org, xuan.ding@intel.com, stephen@networkplumber.org, thomas@monjalon.net, stable@dpdk.org, chenbo.xia@intel.com Cc: Maxime Coquelin Date: Mon, 9 Nov 2020 13:16:29 +0100 Message-Id: <20201109121630.251603-3-maxime.coquelin@redhat.com> In-Reply-To: <20201109121630.251603-1-maxime.coquelin@redhat.com> References: <20201109121630.251603-1-maxime.coquelin@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [dpdk-dev] [PATCH v3 2/3] vhost: fix fd leak in dirty logging setup X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This patch fixes a file descriptor leak which happens in the error path of vhost_user_set_log_base(). Fixes: 4796ad63ba1f ("examples/vhost: import userspace vhost application") Cc: stable@dpdk.org Reported-by: Xuan Ding Signed-off-by: Maxime Coquelin Reviewed-by: Chenbo Xia Reviewed-by: Xueming(Steven) Li --- lib/librte_vhost/vhost_user.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 473fd778ca..94b066f0b9 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -2083,7 +2083,7 @@ vhost_user_set_log_base(struct virtio_net **pdev, struct VhostUserMsg *msg, VHOST_LOG_CONFIG(ERR, "invalid log base msg size: %"PRId32" != %d\n", msg->size, (int)sizeof(VhostUserLog)); - return RTE_VHOST_MSG_RESULT_ERR; + goto close_msg_fds; } size = msg->payload.log.mmap_size; @@ -2094,7 +2094,7 @@ vhost_user_set_log_base(struct virtio_net **pdev, struct VhostUserMsg *msg, VHOST_LOG_CONFIG(ERR, "log offset %#"PRIx64" and log size %#"PRIx64" overflow\n", off, size); - return RTE_VHOST_MSG_RESULT_ERR; + goto close_msg_fds; } VHOST_LOG_CONFIG(INFO, @@ -2131,6 +2131,10 @@ vhost_user_set_log_base(struct virtio_net **pdev, struct VhostUserMsg *msg, msg->fd_num = 0; return RTE_VHOST_MSG_RESULT_REPLY; + +close_msg_fds: + close_msg_fds(msg); + return RTE_VHOST_MSG_RESULT_ERR; } static int vhost_user_set_log_fd(struct virtio_net **pdev __rte_unused, From patchwork Mon Nov 9 12:16:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxime Coquelin X-Patchwork-Id: 83845 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 41454A0527; Mon, 9 Nov 2020 13:17:56 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8811A9B02; Mon, 9 Nov 2020 13:16:52 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by dpdk.org (Postfix) with ESMTP id C623C9B02 for ; Mon, 9 Nov 2020 13:16:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1604924209; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=M+1gGtWVubbP/kqqdRwNDPVmOvVJjMSslC9hKN0Pe58=; b=blMs9sxycY2SjIW2KCtaTYRsDY8TPP3Og32W/dpX5/E4qKWz/fOgcbd8FVQ6N7JvTN7ez+ eN169pQEVc0kNFFCl4XgM62WA09LhL28fWTg3ikxLmJQvAYomFf0pYTuHH0Kc1FAZo0JO+ A16kHkxLy2I8sKPxhigJDqrW5e+tv+A= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-278-AvbPx8ebO521IQQsVNaVow-1; Mon, 09 Nov 2020 07:16:45 -0500 X-MC-Unique: AvbPx8ebO521IQQsVNaVow-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3AECECE642; Mon, 9 Nov 2020 12:16:44 +0000 (UTC) Received: from localhost.localdomain (unknown [10.36.110.32]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7D4156EF5C; Mon, 9 Nov 2020 12:16:42 +0000 (UTC) From: Maxime Coquelin To: dev@dpdk.org, xuan.ding@intel.com, stephen@networkplumber.org, thomas@monjalon.net, stable@dpdk.org, chenbo.xia@intel.com Cc: Maxime Coquelin Date: Mon, 9 Nov 2020 13:16:30 +0100 Message-Id: <20201109121630.251603-4-maxime.coquelin@redhat.com> In-Reply-To: <20201109121630.251603-1-maxime.coquelin@redhat.com> References: <20201109121630.251603-1-maxime.coquelin@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [dpdk-dev] [PATCH v3 3/3] vhost: fix fd leak in kick setup X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This patch fixes a file descriptor leak which happens in the error path of vhost_user_set_vring_kick(). Fixes: 4796ad63ba1f ("examples/vhost: import userspace vhost application") Cc: stable@dpdk.org Signed-off-by: Maxime Coquelin Reviewed-by: Chenbo Xia --- lib/librte_vhost/vhost_user.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 94b066f0b9..f3b2adabac 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -1855,8 +1855,12 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg, /* Interpret ring addresses only when ring is started. */ dev = translate_ring_addresses(dev, file.index); - if (!dev) + if (!dev) { + if (file.fd != VIRTIO_INVALID_EVENTFD) + close(file.fd); + return RTE_VHOST_MSG_RESULT_ERR; + } *pdev = dev;