From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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 <dev@dpdk.org>; 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.marchand@redhat.com>, David Hunt
 <david.hunt@intel.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>, dev <dev@dpdk.org>, dpdk
 stable <stable@dpdk.org>
From: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
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: <CAJFAV8ycSMJe5PXy1wVc=Bd2k_Yh9=n9S+nu7BWkrzBJA_ZcaQ@mail.gmail.com>
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: <CGME20200915193458eucas1p1d9308e63063eda28f96eedba3a361a2b@eucas1p1.samsung.com>
 <20200915193449.13310-1-l.wojciechow@partner.samsung.com>
 <20200915193449.13310-4-l.wojciechow@partner.samsung.com>
 <CAJFAV8ycSMJe5PXy1wVc=Bd2k_Yh9=n9S+nu7BWkrzBJA_ZcaQ@mail.gmail.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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <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>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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
> <l.wojciechow@partner.samsung.com> 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