examples/qos_sched: fix buffer overflow on mbuf free

Message ID 20230308140902.269982-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series examples/qos_sched: fix buffer overflow on mbuf free |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

Bruce Richardson March 8, 2023, 2:09 p.m. UTC
  When running the qos_sched app with separated worker and Tx threads, the
app would seg-fault after a short time of handling packets. The root
cause of this turns out to be an incorrect array index when freeing
unsent packets post-Tx. Rather than freeing packets using the "nb_tx"
value i.e. where transmission failed, the function was freeing packets
using the "nb_pkts" value, i.e. going beyond the number of packets
previously received into the buffer.

Fixes: 39b25117c40b ("examples/qos_sched: remove Tx buffering")

Reported-by: Megha Ajmera <megha.ajmera@intel.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/qos_sched/app_thread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Cristian Dumitrescu March 8, 2023, 2:29 p.m. UTC | #1
> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Wednesday, March 8, 2023 2:09 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Ajmera, Megha
> <megha.ajmera@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>
> Subject: [PATCH] examples/qos_sched: fix buffer overflow on mbuf free
> 
> When running the qos_sched app with separated worker and Tx threads, the
> app would seg-fault after a short time of handling packets. The root
> cause of this turns out to be an incorrect array index when freeing
> unsent packets post-Tx. Rather than freeing packets using the "nb_tx"
> value i.e. where transmission failed, the function was freeing packets
> using the "nb_pkts" value, i.e. going beyond the number of packets
> previously received into the buffer.
> 
> Fixes: 39b25117c40b ("examples/qos_sched: remove Tx buffering")
> 
> Reported-by: Megha Ajmera <megha.ajmera@intel.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  examples/qos_sched/app_thread.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/qos_sched/app_thread.c
> b/examples/qos_sched/app_thread.c
> index 1ea732aa91..059c470afb 100644
> --- a/examples/qos_sched/app_thread.c
> +++ b/examples/qos_sched/app_thread.c

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
  
Thomas Monjalon March 12, 2023, 2:34 p.m. UTC | #2
> > When running the qos_sched app with separated worker and Tx threads, the
> > app would seg-fault after a short time of handling packets. The root
> > cause of this turns out to be an incorrect array index when freeing
> > unsent packets post-Tx. Rather than freeing packets using the "nb_tx"
> > value i.e. where transmission failed, the function was freeing packets
> > using the "nb_pkts" value, i.e. going beyond the number of packets
> > previously received into the buffer.
> > 
> > Fixes: 39b25117c40b ("examples/qos_sched: remove Tx buffering")
> > 
> > Reported-by: Megha Ajmera <megha.ajmera@intel.com>
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Applied, thanks.
  

Patch

diff --git a/examples/qos_sched/app_thread.c b/examples/qos_sched/app_thread.c
index 1ea732aa91..059c470afb 100644
--- a/examples/qos_sched/app_thread.c
+++ b/examples/qos_sched/app_thread.c
@@ -118,7 +118,7 @@  app_tx_thread(struct thread_conf **confs)
 		if (likely(nb_pkts != 0)) {
 			uint16_t nb_tx = rte_eth_tx_burst(conf->tx_port, 0, mbufs, nb_pkts);
 			if (nb_pkts != nb_tx)
-				rte_pktmbuf_free_bulk(&mbufs[nb_pkts], nb_pkts - nb_tx);
+				rte_pktmbuf_free_bulk(&mbufs[nb_tx], nb_pkts - nb_tx);
 		}
 
 		conf_idx++;