[dpdk-stable] [dpdk-dev] [PATCH v1 3/6] app/test: fix freeing mbufs in distributor tests

[David Marchand]

Hello Lukasz, David,


On Tue, Sep 15, 2020 at 9:35 PM Lukasz Wojciechowski
<l.wojciechow at 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?


>         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.
And trying to pass NULL, I can see the distributor library
dereferences oldpkt[] without checking retcount != 0.


-- 
David Marchand