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