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 5B3FBA04B5; Wed, 23 Sep 2020 03:55:25 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 05BF51DB11; Wed, 23 Sep 2020 03:55:24 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 3F7F61DAD5 for ; Wed, 23 Sep 2020 03:55:23 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20200923015522euoutp0201652be9992fa145b933ef8518119eba~3RuB4jnmY2862128621euoutp02V for ; Wed, 23 Sep 2020 01:55:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20200923015522euoutp0201652be9992fa145b933ef8518119eba~3RuB4jnmY2862128621euoutp02V DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1600826122; bh=4swM0hG8PBa2BhSC+k1qc7lgcaCpQMkFnUe5sxPfX2Y=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=duqH+4Fu3YFUof9J+xGUnM4kb0BgGyrQLZNmMTqdzfClCo8HsIpOeRimdq5ke2oHe qQ/BVPeZEmH8sFQ4z4C9NYJFqvuX2G3MNUFD47i2QlN0lzsWaK8QkdrxYEOKXv1L95 OSvqE6FPTwkArOpcf62AQgz3ojuLuYMgLDLjgnHg= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20200923015522eucas1p245a29e4255500e78faf4e3536261d724~3RuBybfUV0764707647eucas1p2R; Wed, 23 Sep 2020 01:55:22 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id 96.9D.05997.A0BAA6F5; Wed, 23 Sep 2020 02:55:22 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20200923015521eucas1p133d30eff38feeef552c6ba419662811f~3RuBFMG5w2794327943eucas1p1N; Wed, 23 Sep 2020 01:55:21 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20200923015521eusmtrp1fb1584a15d9fd413e00470031504207d~3RuBEpm3l1355413554eusmtrp1z; Wed, 23 Sep 2020 01:55:21 +0000 (GMT) X-AuditID: cbfec7f4-677ff7000000176d-9b-5f6aab0a8c3b Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id DC.9A.06314.90BAA6F5; Wed, 23 Sep 2020 02:55:21 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20200923015520eusmtip26088b67523ff5f46dd23e88266503d32~3RuAe0IV52730627306eusmtip2d; Wed, 23 Sep 2020 01:55:20 +0000 (GMT) To: David Marchand , David Hunt Cc: Bruce Richardson , dev , dpdk stable From: Lukasz Wojciechowski Message-ID: <6f700bea-4f4b-b381-bdb1-ee6b5f956c68@partner.samsung.com> Date: Wed, 23 Sep 2020 03:55:20 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Transfer-Encoding: 8bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprKKsWRmVeSWpSXmKPExsWy7djPc7pcq7PiDT7d5re4screom/SRyaL 7Su62CzefdrOZPGv4w+7A6vHrwVLWT0W73nJ5PF+31W2AOYoLpuU1JzMstQifbsEroy5Dy8x F7wRqdg54xJTA+Mvvi5GTg4JAROJk5cXMnYxcnEICaxglNh39yAbhPOFUWLHrkYo5zOjRNut lewwLdMvrmeBSCxnlDh8eh8ThPOWUeLy0ntMIFXCApESKzqnMILYIgLBEtfvzGYBsZkF0iS6 lt0Em8QmYCtxZOZX1i5GDg5eATeJqQclQEwWAVWJPTe4QCpEBeIkjp16BNbJKyAocXLmEzCb UyBQ4ui1dmaIifISzVtnQ9niEreezGeCuHM6u8TRU9YQtotEw7NrzBC2sMSr41ugfpGROD25 B+wXCYFtjBJXf/9khHD2M0pc710BVWUtcfjfbzaQ45gFNCXW79KHCDtKHN2zkAUkLCHAJ3Hj rSDEDXwSk7ZNZ4YI80p0tAlBVOtJPO2Zygiz9s/aJywTGJVmIflsFpJvZiH5ZhbC3gWMLKsY xVNLi3PTU4uN8lLL9YoTc4tL89L1kvNzNzECE8vpf8e/7GDc9SfpEKMAB6MSD++LJ5nxQqyJ ZcWVuYcYJTiYlUR4nc6ejhPiTUmsrEotyo8vKs1JLT7EKM3BoiTOa7zoZayQQHpiSWp2ampB ahFMlomDU6qBUf9gjcSu3Vc9FhwpOhkXdbt+wroY5qj/vD9lfq98tn3X5B2yHjFFIUzsXfbi lwTM/C4JyNQ8X8Ns/Lr36n8dhmmHP3ILT8z039zbW/tyveklFwvNst3/nj5SfnCa+0vgcicN e82FURun7rfgWzS1+uKv70preeuv3HbafdMww8GtX887XHnLXiWW4oxEQy3mouJEAMnz6YUo AwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrOIsWRmVeSWpSXmKPExsVy+t/xe7qcq7PiDc48k7e4screom/SRyaL 7Su62CzefdrOZPGv4w+7A6vHrwVLWT0W73nJ5PF+31W2AOYoPZui/NKSVIWM/OISW6VoQwsj PUNLCz0jE0s9Q2PzWCsjUyV9O5uU1JzMstQifbsEvYy5Dy8xF7wRqdg54xJTA+Mvvi5GTg4J AROJ6RfXs3QxcnEICSxllPj2vIu5i5EDKCEj8eGSAESNsMSfa11sEDWvGSVmTjjEDJIQFoiU WNE5hRHEFhEIltj4bB+YzSyQJrFw4VF2iIY5TBL/mr+xgCTYBGwljsz8ygqygFfATWLqQQkQ k0VAVWLPDS6QClGBOIkzPS/YQGxeAUGJkzOfgHVyCgRKHL3Wzgwx3kxi3uaHULa8RPPW2VC2 uMStJ/OZJjAKzULSPgtJyywkLbOQtCxgZFnFKJJaWpybnltsqFecmFtcmpeul5yfu4kRGEnb jv3cvIPx0sbgQ4wCHIxKPLwvnmTGC7EmlhVX5h5ilOBgVhLhdTp7Ok6INyWxsiq1KD++qDQn tfgQoynQbxOZpUST84FRnlcSb2hqaG5haWhubG5sZqEkztshcDBGSCA9sSQ1OzW1ILUIpo+J g1OqgbG23/q29s1eozTT3lt2SpU7rzEWOLl8FLJn3yA0fW9rhP6l6n9d08I4vJY/XV299luF onXyMn1H9vdToh/3Lt70yJbn/8G2bwWTnvvMrZFZvNB0p+OFSZ2LI1/2/f/96osV49oVTUkt 8lXT3p/6ePF6HJ/CZo1npYYB2+yOOzHrSvSb3Ofq6FBiKc5INNRiLipOBAAmqE5ougIAAA== X-CMS-MailID: 20200923015521eucas1p133d30eff38feeef552c6ba419662811f X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200915193458eucas1p1d9308e63063eda28f96eedba3a361a2b X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200915193458eucas1p1d9308e63063eda28f96eedba3a361a2b References: <20200915193449.13310-1-l.wojciechow@partner.samsung.com> <20200915193449.13310-4-l.wojciechow@partner.samsung.com> Subject: Re: [dpdk-dev] [PATCH v1 3/6] app/test: fix freeing mbufs in distributor tests 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" Hello David, W dniu 22.09.2020 o 14:42, David Marchand pisze: > Hello Lukasz, David, > > > On Tue, Sep 15, 2020 at 9:35 PM Lukasz Wojciechowski > wrote: >> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c >> index 0e49e3714..da13a9a3f 100644 >> --- a/app/test/test_distributor.c >> +++ b/app/test/test_distributor.c >> @@ -277,24 +277,21 @@ handle_work_with_free_mbufs(void *arg) >> struct rte_mbuf *buf[8] __rte_cache_aligned; >> struct worker_params *wp = arg; >> struct rte_distributor *d = wp->dist; >> - unsigned int count = 0; >> unsigned int i; >> unsigned int num = 0; >> unsigned int id = __atomic_fetch_add(&worker_idx, 1, __ATOMIC_RELAXED); >> >> for (i = 0; i < 8; i++) >> buf[i] = NULL; >> - num = rte_distributor_get_pkt(d, id, buf, buf, num); >> + num = rte_distributor_get_pkt(d, id, buf, buf, 0); > For my understanding, we pass an array even if we return 0 packet. Is > this necessary? The short answer is: yes. That's because in case of using old legacy API (single distributor), it is required to pass a pointer to mbuf (might be NULL however). The new burst API functions call the old API dereferencing the first element of the array passed. So there must be a valid array containing at least 1 elem. I pushed the v2 version of the patchset, which contains 2 new patches. Patch #7 fixes this issue in librte_distributor by passing NULL mbuf pointer to legacy API if number of return buffers is zero. > > >> while (!quit) { >> - count += num; >> __atomic_fetch_add(&worker_stats[id].handled_packets, num, >> __ATOMIC_ACQ_REL); >> for (i = 0; i < num; i++) >> rte_pktmbuf_free(buf[i]); >> num = rte_distributor_get_pkt(d, >> - id, buf, buf, num); >> + id, buf, buf, 0); > Here, it gives the impression we have some potential use-after-free on > buf[] content. Nice catch! I missed it. I fixed it in v2 by assigning NULL values to the bufs, so they won't be used after free. > And trying to pass NULL, I can see the distributor library > dereferences oldpkt[] without checking retcount != 0. That's fixed in new patch v2 7/8 Best regards Lukasz > > -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com