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 0B50CA0545; Fri, 13 Nov 2020 08:39:40 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D2EED56A3; Fri, 13 Nov 2020 08:39:37 +0100 (CET) Received: from nat-hk.nvidia.com (nat-hk.nvidia.com [203.18.50.4]) by dpdk.org (Postfix) with ESMTP id 78733569B; Fri, 13 Nov 2020 08:39:34 +0100 (CET) Received: from HKMAIL101.nvidia.com (Not Verified[10.18.92.77]) by nat-hk.nvidia.com (using TLS: TLSv1.2, AES256-SHA) id ; Fri, 13 Nov 2020 15:39:32 +0800 Received: from HKMAIL102.nvidia.com (10.18.16.11) by HKMAIL101.nvidia.com (10.18.16.10) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 13 Nov 2020 07:39:29 +0000 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.172) by HKMAIL102.nvidia.com (10.18.16.11) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Fri, 13 Nov 2020 07:39:29 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ephp/Tj6lbNUZDiKzv3sRTrse6/nHusRXXvf9zokoKh8Y2pckcy2jMsBap9lD2gYwF0jxS9IrbKMKje/aYgQ6FtNEIyYoq3cHR/KXQC/5IVf86G91Ri4HR8qiqLTDNozH/pxiWyu8HPiaJXeZwKZcTSVEvBnnjOxSpQ9ub9SSMhFhWksdM7KUlg0kHhSeChbqH7cg2Xd6A3r1NxPWZLlghniSulOMhUlkQdtkt8waGXoKvMl0HeFiSRwOoqqOM25sMzrRUbVUq6b6BVCfgr8HW6oYqCi/Fk8WXNq9D8sEdiPYPzKlDwvt2Y0KbxJiAQC4ieWdJCroDDggln01ZL0gA== 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=4UZbfXgmA+Q696yt5IhFYvWmhud6kg3sZravUFnZcvY=; b=h/CzuRp4tyl8Zr+MgYhqGlRK5bmjq/7TuCZrBNB1EQncCkIZobtpadWOkS3v4J8+SoBpZP5AlSOV0dzTzc9XHTQkr8fXHo7G/vqHOBXA1yX4HXuj0vcVttHAdfZjNEzbCqnH4isOFHuBEu3dLnYWpHavbq79nFoXK91hmcqGbkQ9V8S9sLRiVc6OIfeLi/EwPG7qgEO4PEkyRKNVU0iCTo4xT9SLIToY+qmeBxC+NwCXLLeoSPltdD+5quL9/9+cLQOjETxCnipmFXdV6RbRrSOYy/riODnbJ2jg+/p1kvfyKbaIV76lxMXNhjRiP7FA0hJFbvlKgFRMWm3z/va3mw== 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 BY5PR12MB4869.namprd12.prod.outlook.com (2603:10b6:a03:1d9::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3541.25; Fri, 13 Nov 2020 07:39:27 +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; Fri, 13 Nov 2020 07:39:27 +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: [PATCH v4 1/3] vhost: fix error path when setting memory tables Thread-Index: AQHWuRjW2BkssySdk0GxOyRuqEj82qnFreyQ Date: Fri, 13 Nov 2020 07:39:27 +0000 Message-ID: References: <20201112171029.354593-1-maxime.coquelin@redhat.com> <20201112171029.354593-2-maxime.coquelin@redhat.com> In-Reply-To: <20201112171029.354593-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:46d:a900:5e1e:7896:40fd:fa19:79c8] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 7d1d1b92-8cdd-4445-b63e-08d887a73fb9 x-ms-traffictypediagnostic: BY5PR12MB4869: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:5797; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 1mpEmpVfmGCAuFN8W5tqCfZuLUqRKIkoy4WWxf/kPbz4ycTFKcmudTLbCIQKrGzb91Guu/ZLna5191QvMOWR216hnA3UaRO8dvAk8ZFGo9B1snAargRr0InCcuPT4DNnRjW0HOj27NyLgCfF1oWUre1V2E2KL+Jd+nvyIWZetj5OS1C6yNxLaFPUSMXPKKlUIFNE3qFHAoYn3z7/VZeEaiLTRAX+iP9Q+2CWodjuGneMoY4tdNGsGxRzDpCI/pqr8jh9Y0E+jGNH0R3pHAF6hPMvof1nvEDQEy/tk6vGnVArtpOMFrT51jBpOGDhLWubT1lK4kPC8hP08JdBlzK5oA== 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)(396003)(346002)(39860400002)(136003)(366004)(376002)(2906002)(33656002)(66946007)(8676002)(52536014)(86362001)(186003)(66476007)(64756008)(110136005)(76116006)(71200400001)(83380400001)(316002)(6506007)(8936002)(66446008)(66556008)(55016002)(5660300002)(7696005)(478600001)(9686003); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: QRacqvwv8Y4YuplQQO6ck//gic4/DRLgZ39oqk1BwmVLm1QTBfJi3XlurKykrIvFCfP0mmEI7tgJYBcDM1vK6VdBFMq6LhfYjzD+dsjFfcsCHD61JGu9weTU/vUYQZG2LPnssEfu/Bu+NV/ZCFzTRA4DvV32+fYAxavDUR9DuzQGAD48htzptl8+yWtvdUDaWKXoNZMGOpD4TzX93NH0k2+RbNjWTD88IP4hZIVmwJx2jnQgg2LMYJQQIT9fIKXALF1fyyxZgT9Azolz2/poBJlQt04sNJJ1B8wYKWmwClbZFWhKlpR6Usqbn5Rb5OmzLHCV+/2QAWb8jZGPmYviGEceyBFXofmxzua3jROTnAo//phMKVVLn7mYkSCTMnt0KhmzfbT31ILKCR5kCQjhFN9opOz6hQT1W8f6nGwVSGAgrPQcvqawk02UW/TT71J07hE1mynRxDBJA6NYlrJO4lB5DflB3RGTrq5p2DNLwtdEa+whb5bWWjgUq6DnXQzpIrjGEGsMk0/uXKI0pRrsmJ5gAVUtMgAVS20io9f0YdAO92cwh/vACachhajqjE4mPtIukWyrX0vgflN5aNN6FXdUdRYKma6Rp3ZWUhcrjOxjxXoPwjAa2jfmvVRCmiNHompYCl5ZEayvBXKwyXBlw8/49RAuxv6/J4lFxb7AaAIvhOeFFc41jwKCGUyodLeJp1DGGjpDYI741JFYvCQ4JQ== 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: 7d1d1b92-8cdd-4445-b63e-08d887a73fb9 X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Nov 2020 07:39:27.0691 (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: xapoEHZlCvW8ewfmp05joAFCNSWwjGnqQ9LuHmPT3GlZPpDToNBXzekpqTfpqdcmgqo8uTio6yimJtccHmORlw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB4869 X-OriginatorOrg: Nvidia.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1605253172; bh=4UZbfXgmA+Q696yt5IhFYvWmhud6kg3sZravUFnZcvY=; 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=Wjc7LYKykkUSznnEGypFF8WqJBN0/1qGPJY3/hMIhhtyLoKkkMJzWWlN3WLrnASET kd97oXK4lq95AWjV20kKIuhl8lTgQqqa8hHswfRtCL6JpRXTKnaekcI0FRQHPa6j9H yYvnJ4dqI5J27j6CAJJpyLfkv8diWyyktuqS+sUbBvQdti4zU+vmQFo5Z8wfIqj9jv UTgFUtO9jInKQ6rpGn3G/UGSCyKJMsxWnr23KwrCDGVnCWcO5tkg3MFgXgS2zkj0Yz FTzPgTC4lE116TGv+5xonC244cCf8Vc7Ap51KyQrb39bKRgrMwUC7xSU5srSReCKbG pbCm0DgUxTJDw== Subject: Re: [dpdk-dev] [PATCH v4 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" >-----Original Message----- >From: Maxime Coquelin >Sent: Friday, November 13, 2020 1:10 AM >To: dev@dpdk.org; xuan.ding@intel.com; stephen@networkplumber.org; >NBU-Contact-Thomas Monjalon ; stable@dpdk.org; >chenbo.xia@intel.com; Xueming(Steven) Li >Cc: Maxime Coquelin >Subject: [PATCH v4 1/3] vhost: fix error path when setting memory 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 | 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 >8a8726f8b8..3898c93d1f 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,7 +1018,7 @@ 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)) >{ @@ -1054,7 +1060,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 +1070,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 +1096,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 +1109,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 +1129,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 +1152,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 +1195,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 +1229,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 +1238,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 +1260,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 +1271,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 Reviewed-by: Xueming(Steven) Li