From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8271842993; Thu, 20 Apr 2023 08:48:34 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 57EBA40A4B; Thu, 20 Apr 2023 08:48:34 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 6EB7240687; Thu, 20 Apr 2023 08:48:32 +0200 (CEST) Received: from dggpemm500009.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Q27Vp08CZzsRBp; Thu, 20 Apr 2023 14:46:58 +0800 (CST) Received: from dggpemm500008.china.huawei.com (7.185.36.136) by dggpemm500009.china.huawei.com (7.185.36.225) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Thu, 20 Apr 2023 14:48:29 +0800 Received: from dggpemm500008.china.huawei.com ([7.185.36.136]) by dggpemm500008.china.huawei.com ([7.185.36.136]) with mapi id 15.01.2507.023; Thu, 20 Apr 2023 14:48:29 +0800 From: wangyunjian To: Honnappa Nagarahalli , "dev@dpdk.org" CC: "konstantin.v.ananyev@yandex.ru" , luyicai , "stable@dpdk.org" , nd , nd Subject: RE: [dpdk-dev] [PATCH] ring: fix use after free in ring release Thread-Topic: [dpdk-dev] [PATCH] ring: fix use after free in ring release Thread-Index: AQHZcS47OX0uLMlsYEKKoGyfmc1woK8xOXEAgAD8iJCAAHG2AIAAyK/w Date: Thu, 20 Apr 2023 06:48:29 +0000 Message-ID: References: In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.174.242.157] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > -----Original Message----- > From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com] > Sent: Thursday, April 20, 2023 5:44 AM > To: wangyunjian ; dev@dpdk.org > Cc: konstantin.v.ananyev@yandex.ru; luyicai ; > stable@dpdk.org; nd ; nd > Subject: RE: [dpdk-dev] [PATCH] ring: fix use after free in ring release >=20 > >=20 > > > > > > > -----Original Message----- > > > > From: Yunjian Wang > > > > Sent: Monday, April 17, 2023 8:12 AM > > > > To: dev@dpdk.org > > > > Cc: Honnappa Nagarahalli ; > > > > konstantin.v.ananyev@yandex.ru; luyicai@huawei.com; Yunjian Wang > > > > ; stable@dpdk.org > > > > Subject: [dpdk-dev] [PATCH] ring: fix use after free in ring > > > > release > > > > > > > > When using the ring to find out tailq entry, however it had been > > > > freed by rte_memzone_free function. This change prevents that from > > happening. > > > I am unable to follow the problem you are describing. > > > After the memzone for the ring is released, the contents of the > > > memzone are not being used. I understand that the variable 'r' is > > > being used, but that should not cause any issues. > > > > > > > > > > > Fixes: 4e32101f9b01 ("ring: support freeing") > > > > Cc: stable@dpdk.org > > > > > > > > Signed-off-by: Yunjian Wang > > > > --- > > > > lib/ring/rte_ring.c | 11 +++++------ > > > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c index > > > > 8ed455043d..17d2d7f8a8 100644 > > > > --- a/lib/ring/rte_ring.c > > > > +++ b/lib/ring/rte_ring.c > > > > @@ -333,11 +333,6 @@ rte_ring_free(struct rte_ring *r) > > > > return; > > > > } > > > > > > > > - if (rte_memzone_free(r->memzone) !=3D 0) { > > > > - RTE_LOG(ERR, RING, "Cannot free memory\n"); > > > > - return; > > > > - } > > > Why do we need to free the memzone later? > > > > After the memzone is freed, it is not removed from the 'rte_ring_tailq'= . > > If rte_ring_lookup is called at this time, it will cause a use-after-fr= ee problem. > Thanks, understood >=20 > > > > Thanks, > > Yunjian > > > > > > > - > > > > ring_list =3D RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list); > > > > rte_mcfg_tailq_write_lock(); > > > > > > > > @@ -349,7 +344,7 @@ rte_ring_free(struct rte_ring *r) > > > > > > > > if (te =3D=3D NULL) { > > > > rte_mcfg_tailq_write_unlock(); > > > > - return; > > > > + goto free_memzone; > We do not need this. If 'te =3D=3D NULL' is true, then the ring was not f= ound or > possibly already freed. OK >=20 > > > > } > > > > > > > > TAILQ_REMOVE(ring_list, te, next); @@ -357,6 +352,10 @@ > > > > rte_ring_free(struct rte_ring *r) > We should free the memzone here while holding the lock OK, You are right. I fix it on your suggestions. https://patchwork.dpdk.org/project/dpdk/patch/c23b1135e1b0676ef7d82969b39a2= 1df992d418f.1681972694.git.wangyunjian@huawei.com/ Thanks, Yunjian >=20 > > > > rte_mcfg_tailq_write_unlock(); > > > > > > > > rte_free(te); > > > > + > > > > +free_memzone: > > > > + if (rte_memzone_free(r->memzone) !=3D 0) > > > > + RTE_LOG(ERR, RING, "Cannot free memory\n"); > > > > } > Should be moved up as mentioned above >=20 > > > > > > > > /* dump the status of the ring on the console */ > > > > -- > > > > 2.33.0