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 E7F43A09D2 for ; Wed, 11 Nov 2020 07:06:45 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C5E8E37B4; Wed, 11 Nov 2020 07:06:44 +0100 (CET) Received: from hqnvemgate26.nvidia.com (hqnvemgate26.nvidia.com [216.228.121.65]) by dpdk.org (Postfix) with ESMTP id EFBCBF64; Wed, 11 Nov 2020 07:06:39 +0100 (CET) Received: from hqmail.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate26.nvidia.com (using TLS: TLSv1.2, AES256-SHA) id ; Tue, 10 Nov 2020 22:06:41 -0800 Received: from HQMAIL111.nvidia.com (172.20.187.18) by HQMAIL111.nvidia.com (172.20.187.18) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Wed, 11 Nov 2020 06:06:35 +0000 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.176) by HQMAIL111.nvidia.com (172.20.187.18) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Wed, 11 Nov 2020 06:06:34 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jSx7j8KI9Jijgdxay12uHxiwnJI7UXRwW2Z5Oo4OV2xAqhax7LEg58sFW7HHgPjurV/LfWc6a+fvVBuk2o93rX/e7BiKCBwgpDZ5hOLXjet9+GgCWhwWUFhfx9WmitoXSWFxvGfmYTSTJvkFOz8ydxSRr7pRkZaQ/QkH7s69XE9oarvD2MgixDftWg/wDlclo6Oahty+3iSkvKUhxTQKn4S2ikvAr2m4l54+tZ/gjHAjSNkbD3cgHZooS7m77Gc765R5sweRylZJL8OO5gSY7DOrmPLqASATRKhqKrhGVhnJsgEQOGofaeEIKQS1LsT1TmYAi+xTVF/Xx3/G5yUMUw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=yhywu/V4B3H0qCzRs/bBZee1rD9L5hxkXrHANXp/Lsw=; b=baKP/TawlAQPN6lYnXf4afCC2fPh0KJ1yYeJy6vuqBvqB58P4z/eDHswyXVkecAO5HHFv3/0mOiCq6iIoYWWV3NxnJhavICZL1KnPj2NCDjhJP5FgsooNDamcXXe8Xla10WJIviQ8CsBCpkoXPjaCYtjIonwGGERmxPZi+tWiF/VCWu9Pm9ue10JTGepdtq0pTGby4MGZn6eRmdEfYzjR4Lq9eT0gtpn/ztouddUpmZwGsgtleF1rb0FiBJi4qh9K3mRPKD/xQqJ6oYYJ5VOG3+b5LC/23BalHCayEUPzGR9KRjl5ZwJvQvcZSKrlc9PigOvg9+GqRtp9GAOkq26Ng== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none Received: from BY5PR12MB4324.namprd12.prod.outlook.com (2603:10b6:a03:209::10) by BYAPR12MB4760.namprd12.prod.outlook.com (2603:10b6:a03:9c::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.28; Wed, 11 Nov 2020 06:06:33 +0000 Received: from BY5PR12MB4324.namprd12.prod.outlook.com ([fe80::3420:8913:2256:fa6f]) by BY5PR12MB4324.namprd12.prod.outlook.com ([fe80::3420:8913:2256:fa6f%2]) with mapi id 15.20.3541.025; Wed, 11 Nov 2020 06:06:33 +0000 From: "Xueming(Steven) Li" To: Maxime Coquelin , "dev@dpdk.org" , "xuan.ding@intel.com" , "stephen@networkplumber.org" , "NBU-Contact-Thomas Monjalon" , "stable@dpdk.org" , "chenbo.xia@intel.com" Thread-Topic: [dpdk-dev] [PATCH v3 1/3] vhost: fix error path when setting memory tables Thread-Index: AQHWtpJUGY/AtFXFHkWURVagQ386U6nCbIig Date: Wed, 11 Nov 2020 06:06:33 +0000 Message-ID: References: <20201109121630.251603-1-maxime.coquelin@redhat.com> <20201109121630.251603-2-maxime.coquelin@redhat.com> In-Reply-To: <20201109121630.251603-2-maxime.coquelin@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [240e:46c:b580:11b6:1873:8290:65b9:2693] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: f3bd0fce-1809-4b25-aad1-08d88607f0c0 x-ms-traffictypediagnostic: BYAPR12MB4760: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: D+c4+mfpd5Lr59LrbRUOD1VKfNbRjN6SO9x3u6eSRRNODhUCPO6mEq9XgTeEdwJpiCMbQWaSe5YE9vhELCZbPzjJHgGffu8hq3uUSrKdZQggawCrsnfPpsVAICpbvF5WVGi9wZEjgonC4aQArY7R38CRoVFQ+d14QeOzqFzVWskYl/Ykjo613bLAtFYWk27xjsC4CduIbI/rvmSziaHqdO0ZAPOPPHmQdPMx1SKO1dw5OLyZujaS5qAxh2MPL9psyuY7Q+e2yoSYEWpc7uxYoTNteELuELqFSghVbcT07cBptoSgwv/d+oDU72VMahGjWCmuviXTWX0tKEcSZC2PTw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BY5PR12MB4324.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(346002)(376002)(366004)(136003)(396003)(39860400002)(6506007)(110136005)(5660300002)(186003)(52536014)(2906002)(55016002)(83380400001)(478600001)(7696005)(316002)(64756008)(66446008)(9686003)(86362001)(66556008)(33656002)(8936002)(76116006)(71200400001)(66946007)(66476007); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: I3XfsgpxWeRYK8jXGGaAdVO/V6pRXyp5owaWRQlp49SIN0LcJkaW6fDfMtHcXoc/pV8IBHY7WD8qn7xR0hwsmkQBwoctb0hNU7rp8E9yp7VFosHtADvsfGdEZiL9ItXUmwgALncI0uWZT4z6pTSZBDavw1wYtWHKJBCa+F0Ac8dtm1aNfv1hvDyU83L3q2vRdMMdQyJjBLzsgaGXi6/JKEdQA3pIj435yM7A3wyUvUGugI67ztvp3UINdA0o0FO8W+X5X/IizDTz6dPxqVh33kCHvchg40nyYIKWh43xqwg7DK9lS8OPywVOSOybpjhHh8y4mSmggT5Edan02N8cTzmvcOWWjNKcJ2RRPlp4J00vJx72D01FO2CUQDMrCBOIbkswLvPjMMtu2MxZjjqhfV/++P9pq3GF5Ggj8UOUQ9s0PuUv2EgspZa6ZHrvNnYLvmnJWLWyGYfHnbGKCJE9rc9yeUwB0sniDaqhmbMovEPbLv1Ilkut0cvj22tdqojFqEtsAyVCrl5lmnE0GoDZqSQvSh9f4z38+6EKIgyApN+6FLORrn2FJFB7IzJkjSzMER/ksLTkOCFdZebGUeCg2v9PuBLyP/gP++tXSDW4fNCrUbzspp7xcWHTgXXt1Zbr5PNP5xGAo2lXBwZs2JA1tMtTJXbqZLpOcG5RxBMeSYYnv5Hgp+Ey0F+jBl1bQscObsq8LV9YvkTBIuGvFB2JIA== x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB4324.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: f3bd0fce-1809-4b25-aad1-08d88607f0c0 X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Nov 2020 06:06:33.4528 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: +a9OFk25qWdNxm4HZN7R7lAqZfO3VIkHLskvaLwY+VLC64jspXK4pAZGz7ci/k5Sv/U9eFONkldeqihBBV7a3w== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR12MB4760 X-OriginatorOrg: Nvidia.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1605074801; bh=yhywu/V4B3H0qCzRs/bBZee1rD9L5hxkXrHANXp/Lsw=; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:From:To: Subject:Thread-Topic:Thread-Index:Date:Message-ID:References: In-Reply-To:Accept-Language:Content-Language:X-MS-Has-Attach: X-MS-TNEF-Correlator:authentication-results:x-originating-ip: x-ms-publictraffictype:x-ms-office365-filtering-correlation-id: x-ms-traffictypediagnostic:x-ld-processed: x-microsoft-antispam-prvs:x-ms-oob-tlc-oobclassifiers: x-ms-exchange-senderadcheck:x-microsoft-antispam: x-microsoft-antispam-message-info:x-forefront-antispam-report: x-ms-exchange-antispam-messagedata:x-ms-exchange-transport-forked: Content-Type:Content-Transfer-Encoding:MIME-Version: X-MS-Exchange-CrossTenant-AuthAs: X-MS-Exchange-CrossTenant-AuthSource: X-MS-Exchange-CrossTenant-Network-Message-Id: X-MS-Exchange-CrossTenant-originalarrivaltime: X-MS-Exchange-CrossTenant-fromentityheader: X-MS-Exchange-CrossTenant-id:X-MS-Exchange-CrossTenant-mailboxtype: X-MS-Exchange-CrossTenant-userprincipalname: X-MS-Exchange-Transport-CrossTenantHeadersStamped:X-OriginatorOrg; b=f7VzlxfNPgeFFPC8P3r9D45/j8VYdlpT1zd9GNJMCNCVu7tpRWnOSAQR9KTBuT5+t lsDjStr5bTlpwbMJLsAYbQf+5MjP+wDqNDYIj4njxr5EIyNafoVcBYG2eHOsmRbFI/ c3ES9V+XoKLE9FWtA2zTEHks2mouFhx450iKyFVTMZYQyH5G8zJ9Uf4Uzzg8nf1p6U inq/AzexcMzglnRpXH7kGaO+WRRF5o64ntFMIXeWg5WQmybc0RiQ+Dn0l49RaK87aE KCJY3HNWArvQYcVLzArQ7DGHtViWGh35uzF0Ty/LacDvko7qIHw0gJDwwx9jtYNLGZ ghfJ3cm/Ml/aA== Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v3 1/3] vhost: fix error path when setting memory tables 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 Maxime, >-----Original Message----- >From: dev On Behalf Of Maxime Coquelin >Sent: Monday, November 9, 2020 8:16 PM >To: dev@dpdk.org; xuan.ding@intel.com; stephen@networkplumber.org; >NBU-Contact-Thomas Monjalon ; stable@dpdk.org; >chenbo.xia@intel.com >Cc: Maxime Coquelin >Subject: [dpdk-dev] [PATCH v3 1/3] vhost: fix error path when setting memo= ry >tables > >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, ta= king >care of avoiding double closing of the file descriptors. guest_pages is al= so >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 =3D 0; i < msg->fd_num; i++) >- close(msg->fds[i]); >+ for (i =3D 0; i < msg->fd_num; i++) { >+ int fd =3D msg->fds[i]; >+ >+ if (fd =3D=3D -1) >+ continue; >+ >+ msg->fds[i] =3D -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) !=3D 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; Return code will be changed to RTE_VHOST_MSG_RESULT_ERR, is this ok? > } > > 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 =3D memory->nregions; > > for (i =3D 0; i < memory->nregions; i++) { >- fd =3D msg->fds[i]; > reg =3D &dev->mem->regions[i]; > > reg->guest_phys_addr =3D memory->regions[i].guest_phys_addr; > reg->guest_user_addr =3D memory->regions[i].userspace_addr; > reg->size =3D memory->regions[i].memory_size; >- reg->fd =3D fd; >+ reg->fd =3D msg->fds[i]; >+ >+ /* >+ * Assign invalid file descriptor value to avoid double >+ * closing on error path. >+ */ >+ msg->fds[i] =3D -1; > > mmap_offset =3D 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 =3D 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 =3D get_blk_size(fd); >+ alignment =3D get_blk_size(reg->fd); > if (alignment =3D=3D (uint64_t)-1) { > VHOST_LOG_CONFIG(ERR, > "couldn't get hugepage size through fstat\n"); >- goto err_mmap; >+ goto free_mem_table; > } > mmap_size =3D RTE_ALIGN_CEIL(mmap_size, alignment); > if (mmap_size =3D=3D 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 =3D dev->async_copy ? MAP_POPULATE : 0; > mmap_addr =3D mmap(NULL, mmap_size, PROT_READ | >PROT_WRITE, >- MAP_SHARED | populate, fd, 0); >+ MAP_SHARED | populate, reg->fd, 0); > > if (mmap_addr =3D=3D MAP_FAILED) { > VHOST_LOG_CONFIG(ERR, > "mmap region %u failed.\n", i); >- goto err_mmap; >+ goto free_mem_table; > } > > reg->mmap_addr =3D 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) <=3D 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) !=3D 0) >- goto err_mmap; >+ goto free_mem_table; > > if (ack_msg.request.master !=3D 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 =3D %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 =3D translate_ring_addresses(dev, i); > if (!dev) { > dev =3D *pdev; >- goto err_mmap; >+ goto free_mem_table; > } > > *pdev =3D 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 =3D NULL; >+free_guest_pages: >+ rte_free(dev->guest_pages); >+ dev->guest_pages =3D NULL; >+close_msg_fds: >+ close_msg_fds(msg); > return RTE_VHOST_MSG_RESULT_ERR; > } > >-- >2.26.2