Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/83836/?format=api
{ "id": 83836, "url": "http://patches.dpdk.org/api/patches/83836/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/20201109112600.250779-2-maxime.coquelin@redhat.com/", "project": { "id": 1, "url": "http://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<20201109112600.250779-2-maxime.coquelin@redhat.com>", "list_archive_url": "https://inbox.dpdk.org/dev/20201109112600.250779-2-maxime.coquelin@redhat.com", "date": "2020-11-09T11:25:58", "name": "[v2,1/3] vhost: fix error path when setting memory tables", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "48cef2c077a8a44e6fb8a0790bd230a3cdbdf69a", "submitter": { "id": 512, "url": "http://patches.dpdk.org/api/people/512/?format=api", "name": "Maxime Coquelin", "email": "maxime.coquelin@redhat.com" }, "delegate": { "id": 2642, "url": "http://patches.dpdk.org/api/users/2642/?format=api", "username": "mcoquelin", "first_name": "Maxime", "last_name": "Coquelin", "email": "maxime.coquelin@redhat.com" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/20201109112600.250779-2-maxime.coquelin@redhat.com/mbox/", "series": [ { "id": 13740, "url": "http://patches.dpdk.org/api/series/13740/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=13740", "date": "2020-11-09T11:25:57", "name": "vhost: fix fd an memory leaks", "version": 2, "mbox": "http://patches.dpdk.org/series/13740/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/83836/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/83836/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from dpdk.org (dpdk.org [92.243.14.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 83854A0527;\n\tMon, 9 Nov 2020 12:26:37 +0100 (CET)", "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 98E1C697B;\n\tMon, 9 Nov 2020 12:26:19 +0100 (CET)", "from us-smtp-delivery-124.mimecast.com\n (us-smtp-delivery-124.mimecast.com [63.128.21.124])\n by dpdk.org (Postfix) with ESMTP id 27D506947\n for <dev@dpdk.org>; Mon, 9 Nov 2020 12:26:17 +0100 (CET)", "from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com\n [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id\n us-mta-468-6l57dB5UO6O7STnzzsoFPg-1; Mon, 09 Nov 2020 06:26:11 -0500", "from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com\n [10.5.11.13])\n (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n (No client certificate requested)\n by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 943691891E87;\n Mon, 9 Nov 2020 11:26:10 +0000 (UTC)", "from localhost.localdomain (unknown [10.36.110.32])\n by smtp.corp.redhat.com (Postfix) with ESMTP id A3B516EF51;\n Mon, 9 Nov 2020 11:26:08 +0000 (UTC)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n s=mimecast20190719; t=1604921175;\n h=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n to:to:cc:cc:mime-version:mime-version:content-type:content-type:\n content-transfer-encoding:content-transfer-encoding:\n in-reply-to:in-reply-to:references:references;\n bh=+dj4pk0M/XfD4ryywtAIkfewrrXOGTCa+kKn+KHtGr8=;\n b=VRhP7kd/q10HnCYO1xgwaZ6ngvb/MEGmvKT2oWahf53oKjKhC0MH80Gub2VYDj/AolDSNQ\n 1vfqg6mz1dqowqVsXR2iSwspJCWXBJmZbQaOMi9f5nZUXsYHw0T0grG5eNs9tvPb92+rPL\n mNC8QKXE+1tVDf0QN5S/oMtPWnEwpus=", "X-MC-Unique": "6l57dB5UO6O7STnzzsoFPg-1", "From": "Maxime Coquelin <maxime.coquelin@redhat.com>", "To": "dev@dpdk.org, xuan.ding@intel.com, stephen@networkplumber.org,\n thomas@monjalon.net, stable@dpdk.org, chenbo.xia@intel.com", "Cc": "Maxime Coquelin <maxime.coquelin@redhat.com>", "Date": "Mon, 9 Nov 2020 12:25:58 +0100", "Message-Id": "<20201109112600.250779-2-maxime.coquelin@redhat.com>", "In-Reply-To": "<20201109112600.250779-1-maxime.coquelin@redhat.com>", "References": "<20201109112600.250779-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;\n 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 v2 1/3] vhost: fix error path when setting memory\n\ttables", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "If an error is encountered before the memory regions are\nparsed, the file descriptors for these shared buffers are\nleaked.\n\nThis patch fixes this by closing the message file descriptors\non error, taking care of avoiding double closing of the file\ndescriptors. guest_pages is also freed, even though it was not\nleaked as its pointer was not overridden on subsequent function\ncalls.\n\nFixes: 8f972312b8f4 (\"vhost: support vhost-user\")\nCc: stable@dpdk.org\n\nReported-by: Xuan Ding <xuan.ding@intel.com>\nSigned-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>\nReviewed-by: Chenbo Xia <chenbo.xia@intel.com>\n---\n lib/librte_vhost/vhost_user.c | 65 +++++++++++++++++++++--------------\n 1 file changed, 39 insertions(+), 26 deletions(-)", "diff": "diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c\nindex 8a8726f8b8..473fd778ca 100644\n--- a/lib/librte_vhost/vhost_user.c\n+++ b/lib/librte_vhost/vhost_user.c\n@@ -99,8 +99,15 @@ close_msg_fds(struct VhostUserMsg *msg)\n {\n \tint i;\n \n-\tfor (i = 0; i < msg->fd_num; i++)\n-\t\tclose(msg->fds[i]);\n+\tfor (i = 0; i < msg->fd_num; i++) {\n+\t\tint fd = msg->fds[i];\n+\n+\t\tif (fd == -1)\n+\t\t\tcontinue;\n+\n+\t\tmsg->fds[i] = -1;\n+\t\tclose(fd);\n+\t}\n }\n \n /*\n@@ -1004,7 +1011,6 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \tuint64_t alignment;\n \tuint32_t i;\n \tint populate;\n-\tint fd;\n \n \tif (validate_msg_fds(msg, memory->nregions) != 0)\n \t\treturn RTE_VHOST_MSG_RESULT_ERR;\n@@ -1012,16 +1018,13 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \tif (memory->nregions > VHOST_MEMORY_MAX_NREGIONS) {\n \t\tVHOST_LOG_CONFIG(ERR,\n \t\t\t\"too many memory regions (%u)\\n\", memory->nregions);\n-\t\treturn RTE_VHOST_MSG_RESULT_ERR;\n+\t\tgoto close_msg_fds;\n \t}\n \n \tif (dev->mem && !vhost_memory_changed(memory, dev->mem)) {\n \t\tVHOST_LOG_CONFIG(INFO,\n \t\t\t\"(%d) memory regions not changed\\n\", dev->vid);\n-\n-\t\tclose_msg_fds(msg);\n-\n-\t\treturn RTE_VHOST_MSG_RESULT_OK;\n+\t\tgoto close_msg_fds;\n \t}\n \n \tif (dev->mem) {\n@@ -1054,7 +1057,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\t\t\t\"(%d) failed to allocate memory \"\n \t\t\t\t\"for dev->guest_pages\\n\",\n \t\t\t\tdev->vid);\n-\t\t\treturn RTE_VHOST_MSG_RESULT_ERR;\n+\t\t\tgoto close_msg_fds;\n \t\t}\n \t}\n \n@@ -1064,18 +1067,23 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\tVHOST_LOG_CONFIG(ERR,\n \t\t\t\"(%d) failed to allocate memory for dev->mem\\n\",\n \t\t\tdev->vid);\n-\t\treturn RTE_VHOST_MSG_RESULT_ERR;\n+\t\tgoto free_guest_pages;\n \t}\n \tdev->mem->nregions = memory->nregions;\n \n \tfor (i = 0; i < memory->nregions; i++) {\n-\t\tfd = msg->fds[i];\n \t\treg = &dev->mem->regions[i];\n \n \t\treg->guest_phys_addr = memory->regions[i].guest_phys_addr;\n \t\treg->guest_user_addr = memory->regions[i].userspace_addr;\n \t\treg->size = memory->regions[i].memory_size;\n-\t\treg->fd = fd;\n+\t\treg->fd = msg->fds[i];\n+\n+\t\t/*\n+\t\t * Assign invalid file descriptor value to avoid double\n+\t\t * closing on error path.\n+\t\t */\n+\t\tmsg->fds[i] = -1;\n \n \t\tmmap_offset = memory->regions[i].mmap_offset;\n \n@@ -1085,7 +1093,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\t\t\t\"mmap_offset (%#\"PRIx64\") and memory_size \"\n \t\t\t\t\"(%#\"PRIx64\") overflow\\n\",\n \t\t\t\tmmap_offset, reg->size);\n-\t\t\tgoto err_mmap;\n+\t\t\tgoto free_mem_table;\n \t\t}\n \n \t\tmmap_size = reg->size + mmap_offset;\n@@ -1098,11 +1106,11 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\t * to avoid failure, make sure in caller to keep length\n \t\t * aligned.\n \t\t */\n-\t\talignment = get_blk_size(fd);\n+\t\talignment = get_blk_size(reg->fd);\n \t\tif (alignment == (uint64_t)-1) {\n \t\t\tVHOST_LOG_CONFIG(ERR,\n \t\t\t\t\"couldn't get hugepage size through fstat\\n\");\n-\t\t\tgoto err_mmap;\n+\t\t\tgoto free_mem_table;\n \t\t}\n \t\tmmap_size = RTE_ALIGN_CEIL(mmap_size, alignment);\n \t\tif (mmap_size == 0) {\n@@ -1118,17 +1126,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\t\tVHOST_LOG_CONFIG(ERR, \"mmap size (0x%\" PRIx64 \") \"\n \t\t\t\t\t\"or alignment (0x%\" PRIx64 \") is invalid\\n\",\n \t\t\t\t\treg->size + mmap_offset, alignment);\n-\t\t\tgoto err_mmap;\n+\t\t\tgoto free_mem_table;\n \t\t}\n \n \t\tpopulate = dev->async_copy ? MAP_POPULATE : 0;\n \t\tmmap_addr = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,\n-\t\t\t\t MAP_SHARED | populate, fd, 0);\n+\t\t\t\t MAP_SHARED | populate, reg->fd, 0);\n \n \t\tif (mmap_addr == MAP_FAILED) {\n \t\t\tVHOST_LOG_CONFIG(ERR,\n \t\t\t\t\"mmap region %u failed.\\n\", i);\n-\t\t\tgoto err_mmap;\n+\t\t\tgoto free_mem_table;\n \t\t}\n \n \t\treg->mmap_addr = mmap_addr;\n@@ -1141,7 +1149,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\t\t\tVHOST_LOG_CONFIG(ERR,\n \t\t\t\t\t\"adding guest pages to region %u failed.\\n\",\n \t\t\t\t\ti);\n-\t\t\t\tgoto err_mmap;\n+\t\t\t\tgoto free_mem_table;\n \t\t\t}\n \n \t\tVHOST_LOG_CONFIG(INFO,\n@@ -1184,17 +1192,17 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\tif (read_vhost_message(main_fd, &ack_msg) <= 0) {\n \t\t\tVHOST_LOG_CONFIG(ERR,\n \t\t\t\t\"Failed to read qemu ack on postcopy set-mem-table\\n\");\n-\t\t\tgoto err_mmap;\n+\t\t\tgoto free_mem_table;\n \t\t}\n \n \t\tif (validate_msg_fds(&ack_msg, 0) != 0)\n-\t\t\tgoto err_mmap;\n+\t\t\tgoto free_mem_table;\n \n \t\tif (ack_msg.request.master != VHOST_USER_SET_MEM_TABLE) {\n \t\t\tVHOST_LOG_CONFIG(ERR,\n \t\t\t\t\"Bad qemu ack on postcopy set-mem-table (%d)\\n\",\n \t\t\t\tack_msg.request.master);\n-\t\t\tgoto err_mmap;\n+\t\t\tgoto free_mem_table;\n \t\t}\n \n \t\t/* Now userfault register and we can use the memory */\n@@ -1218,7 +1226,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\t\t\t\t\"Failed to register ufd for region %d: (ufd = %d) %s\\n\",\n \t\t\t\t\ti, dev->postcopy_ufd,\n \t\t\t\t\tstrerror(errno));\n-\t\t\t\tgoto err_mmap;\n+\t\t\t\tgoto free_mem_table;\n \t\t\t}\n \t\t\tVHOST_LOG_CONFIG(INFO,\n \t\t\t\t\"\\t userfaultfd registered for range : \"\n@@ -1227,7 +1235,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\t\t\t(uint64_t)reg_struct.range.start +\n \t\t\t\t(uint64_t)reg_struct.range.len - 1);\n #else\n-\t\t\tgoto err_mmap;\n+\t\t\tgoto free_mem_table;\n #endif\n \t\t}\n \t}\n@@ -1249,7 +1257,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \t\t\tdev = translate_ring_addresses(dev, i);\n \t\t\tif (!dev) {\n \t\t\t\tdev = *pdev;\n-\t\t\t\tgoto err_mmap;\n+\t\t\t\tgoto free_mem_table;\n \t\t\t}\n \n \t\t\t*pdev = dev;\n@@ -1260,10 +1268,15 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg,\n \n \treturn RTE_VHOST_MSG_RESULT_OK;\n \n-err_mmap:\n+free_mem_table:\n \tfree_mem_region(dev);\n \trte_free(dev->mem);\n \tdev->mem = NULL;\n+free_guest_pages:\n+\trte_free(dev->guest_pages);\n+\tdev->guest_pages = NULL;\n+close_msg_fds:\n+\tclose_msg_fds(msg);\n \treturn RTE_VHOST_MSG_RESULT_ERR;\n }\n \n", "prefixes": [ "v2", "1/3" ] }