From 6bd951b48222caaa10a796057f617cab04f928b0 Mon Sep 17 00:00:00 2001 From: Lukasz Wojciechowski Date: Sat, 17 Oct 2020 05:06:47 +0200 Subject: [PATCH] distributor: fix buffer use after free rte_distributor_request_pkt and rte_distributor_get_pkt dereferenced oldpkt parameter when in RTE_DIST_ALG_SINGLE even if number of returned buffers from worker to distributor was 0. This patch passes NULL to the legacy API when number of returned buffers is 0. This allows passing NULL as oldpkt parameter. Distributor tests are also updated passing NULL as oldpkt and 0 as number of returned packets, where packets are not returned. Fixes: 775003ad2f96 ("distributor: add new burst-capable library") Cc: stable@dpdk.org Signed-off-by: Lukasz Wojciechowski Acked-by: David Hunt --- app/test/test_distributor.c | 28 +++++++++--------------- lib/librte_distributor/rte_distributor.c | 4 ++-- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c index ba1f81cf8d..52230d2504 100644 --- a/app/test/test_distributor.c +++ b/app/test/test_distributor.c @@ -62,13 +62,10 @@ handle_work(void *arg) struct rte_mbuf *buf[8] __rte_cache_aligned; struct worker_params *wp = arg; struct rte_distributor *db = wp->dist; - unsigned int count = 0, num = 0; + unsigned int count = 0, num; unsigned int id = __atomic_fetch_add(&worker_idx, 1, __ATOMIC_RELAXED); - int i; - for (i = 0; i < 8; i++) - buf[i] = NULL; - num = rte_distributor_get_pkt(db, id, buf, buf, num); + num = rte_distributor_get_pkt(db, id, buf, NULL, 0); while (!quit) { __atomic_fetch_add(&worker_stats[id].handled_packets, num, __ATOMIC_RELAXED); @@ -272,19 +269,16 @@ handle_work_with_free_mbufs(void *arg) struct rte_distributor *d = wp->dist; unsigned int count = 0; unsigned int i; - unsigned int num = 0; + unsigned int num; 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, NULL, 0); while (!quit) { worker_stats[id].handled_packets += num; count += num; for (i = 0; i < num; i++) rte_pktmbuf_free(buf[i]); - num = rte_distributor_get_pkt(d, - id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); } worker_stats[id].handled_packets += num; count += num; @@ -342,14 +336,14 @@ handle_work_for_shutdown_test(void *arg) struct worker_params *wp = arg; struct rte_distributor *d = wp->dist; unsigned int count = 0; - unsigned int num = 0; + unsigned int num; unsigned int total = 0; unsigned int i; unsigned int returned = 0; const unsigned int id = __atomic_fetch_add(&worker_idx, 1, __ATOMIC_RELAXED); - num = rte_distributor_get_pkt(d, id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); /* wait for quit single globally, or for worker zero, wait * for zero_quit */ @@ -358,8 +352,7 @@ handle_work_for_shutdown_test(void *arg) count += num; for (i = 0; i < num; i++) rte_pktmbuf_free(buf[i]); - num = rte_distributor_get_pkt(d, - id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); total += num; } worker_stats[id].handled_packets += num; @@ -373,14 +366,13 @@ handle_work_for_shutdown_test(void *arg) while (zero_quit) usleep(100); - num = rte_distributor_get_pkt(d, - id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); while (!quit) { worker_stats[id].handled_packets += num; count += num; rte_pktmbuf_free(pkt); - num = rte_distributor_get_pkt(d, id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); } returned = rte_distributor_return_pkt(d, id, buf, num); diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_distributor/rte_distributor.c index d6d4350a28..93c90cf543 100644 --- a/lib/librte_distributor/rte_distributor.c +++ b/lib/librte_distributor/rte_distributor.c @@ -42,7 +42,7 @@ rte_distributor_request_pkt(struct rte_distributor *d, if (unlikely(d->alg_type == RTE_DIST_ALG_SINGLE)) { rte_distributor_request_pkt_single(d->d_single, - worker_id, oldpkt[0]); + worker_id, count ? oldpkt[0] : NULL); return; } @@ -134,7 +134,7 @@ rte_distributor_get_pkt(struct rte_distributor *d, if (unlikely(d->alg_type == RTE_DIST_ALG_SINGLE)) { if (return_count <= 1) { pkts[0] = rte_distributor_get_pkt_single(d->d_single, - worker_id, oldpkt[0]); + worker_id, return_count ? oldpkt[0] : NULL); return (pkts[0]) ? 1 : 0; } else return -EINVAL; -- 2.20.1