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 7F535A04B1; Thu, 5 Nov 2020 12:46:29 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id ABC61C321; Thu, 5 Nov 2020 12:46:09 +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 7071DBE95 for ; Thu, 5 Nov 2020 12:46:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1604576762; 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=iRDZnN8T99vgAdlEXNqQqz5jk5AqncXosm/UpIDr8AA=; b=bwyHcwo8Vi9fnKZ1HOkp4/pCJ9tsgLezQ8eDU4K+lqIydUwi4uO2/+UmlBNBwPUd+j159c CHcsDR5OPBCc2Fp05aaFH4g5J+VUo13G8HRHcGAuvzNcJ4AdTnQhgy8MpAE4potVJMHEJx vovtEzyahyGGF2CzKlCiWy0pl2tkXjM= 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-222-UGSPUVfSPzSiCpizD9xpDg-1; Thu, 05 Nov 2020 06:46:01 -0500 X-MC-Unique: UGSPUVfSPzSiCpizD9xpDg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 98E621074646; Thu, 5 Nov 2020 11:45:59 +0000 (UTC) Received: from localhost.localdomain (unknown [10.36.110.29]) by smtp.corp.redhat.com (Postfix) with ESMTP id 889F25D9D5; Thu, 5 Nov 2020 11:45:57 +0000 (UTC) From: Maxime Coquelin To: dev@dpdk.org, xuan.ding@intel.com, stephen@networkplumber.org, ferruh.yigit@intel.com, thomas@monjalon.net, chenbo.xia@intel.com Cc: stable@dpdk.org, Maxime Coquelin Date: Thu, 5 Nov 2020 12:45:47 +0100 Message-Id: <20201105114549.97717-2-maxime.coquelin@redhat.com> In-Reply-To: <20201105114549.97717-1-maxime.coquelin@redhat.com> References: <20201105114549.97717-1-maxime.coquelin@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" Subject: [dpdk-dev] [PATCH 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 the an error is encountered before the memory regions are parsed, the file descriptors for these shared buffers is 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 overiden on subsequent function calls. Fixes: 8f972312b8f4 ("vhost: support vhost-user") Cc: stable@dpdk.org Reported-by: Xuan Ding Signed-off-by: Maxime Coquelin --- 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; } -- 2.26.2