Re: [dpdk-dev] [PATCH] net/thunderx: fix access an array out of bounds

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Fri,  7 Apr 2017 16:38:41 +0200
> From: marcin.wilk@caviumnetworks.com
> To: jerin.jacob@caviumnetworks.com, maciej.czekaj@caviumnetworks.com
> Cc: dev@dpdk.org, stable@dpdk.org, Marcin Wilk
>  <marcin.wilk@caviumnetworks.com>
> Subject: [PATCH] net/thunderx: fix access an array out of bounds
> X-Mailer: git-send-email 2.7.4
> 
> From: Marcin Wilk <marcin.wilk@caviumnetworks.com>
> 
> Trying to assign more queues to stats struct break only from one loop
> when the maximum size is reached. Outside loop interation is continued.
> This leads to access an array out of bounds.
> 
> Fixes: 21e3fb0050b9 ("net/thunderx: add final bits for secondary queue support")
> 
> Signed-off-by: Marcin Wilk <marcin.wilk@caviumnetworks.com>
> ---
>  drivers/net/thunderx/nicvf_ethdev.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
> index 6c3670a..40d6671 100644
> --- a/drivers/net/thunderx/nicvf_ethdev.c
> +++ b/drivers/net/thunderx/nicvf_ethdev.c
> @@ -251,6 +251,7 @@ nicvf_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  	uint16_t rx_start, rx_end;
>  	uint16_t tx_start, tx_end;
>  	size_t i;
> +	bool breakout = false;
>  
>  	/* RX queue indices for the first VF */
>  	nicvf_rx_range(dev, nic, &rx_start, &rx_end);
> @@ -289,8 +290,10 @@ nicvf_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  
>  		/* Reading per RX ring stats */
>  		for (qidx = rx_start; qidx <= rx_end; qidx++) {
> -			if (qidx == RTE_ETHDEV_QUEUE_STAT_CNTRS)

IMO, adding the qidx >= RTE_ETHDEV_QUEUE_STAT_CNTRS is much simpler than
breakout logic.

With that change:
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

> +			if (qidx == RTE_ETHDEV_QUEUE_STAT_CNTRS) {
> +				breakout = true;
>  				break;
> +			}
>  
>  			nicvf_hw_get_rx_qstats(snic, &rx_qstats,
>  					       qidx % MAX_RCV_QUEUES_PER_QS);
> @@ -302,14 +305,18 @@ nicvf_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  		nicvf_tx_range(dev, snic, &tx_start, &tx_end);
>  		/* Reading per TX ring stats */
>  		for (qidx = tx_start; qidx <= tx_end; qidx++) {
> -			if (qidx == RTE_ETHDEV_QUEUE_STAT_CNTRS)
> +			if (qidx == RTE_ETHDEV_QUEUE_STAT_CNTRS) {
> +				breakout = true;
>  				break;
> +			}
>  
>  			nicvf_hw_get_tx_qstats(snic, &tx_qstats,
>  					       qidx % MAX_SND_QUEUES_PER_QS);
>  			stats->q_obytes[qidx] = tx_qstats.q_tx_bytes;
>  			stats->q_opackets[qidx] = tx_qstats.q_tx_packets;
>  		}
> +		if (breakout)
> +			break;
>  	}
>  
>  	nicvf_hw_get_stats(nic, &port_stats);
> -- 
> 2.7.4
>