Re: [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats()

[Kohei Enju <kohei@enjuk.jp>]

On Mon, 16 Feb 2026 13:19:09 +0000, Simon Horman wrote:

> > @@ -345,19 +344,19 @@ static void iavf_get_ethtool_stats(struct net_device *netdev,
> >  	iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
> >  
> >  	rcu_read_lock();
> > -	/* As num_active_queues describe both tx and rx queues, we can use
> > -	 * it to iterate over rings' stats.
> > +	/* Use num_tx_queues to report stats for the maximum number of queues.
> > +	 * Queues beyond num_active_queues will report zero.
> >  	 */
> > -	for (i = 0; i < adapter->num_active_queues; i++) {
> > -		struct iavf_ring *ring;
> > +	for (i = 0; i < netdev->num_tx_queues; i++) {
> > +		struct iavf_ring *tx_ring = NULL, *rx_ring = NULL;
> >  
> > -		/* Tx rings stats */
> > -		ring = &adapter->tx_rings[i];
> > -		iavf_add_queue_stats(&data, ring);
> > +		if (i < adapter->num_active_queues) {
> 
> Hi Enju-san,

Hi Horman-san, thank you for reviewing!

> 
> If I understand things correctly, in the scenario described in the patch
> description, num_active_queues will be 8 here.

Yes.

> 
> Won't that result in an overflow?

I think it won't overflow.

In Thread 1, iavf_set_channels(), which allocates {tx,rx}_rings and
updates num_active_queues, is executed under netdev lock. Therefore
Thread 3, which is also executed under the netdev lock, sees updated
num_active_queues and {tx,rx}_rings.

The scenario flow lacked netdev_(un)lock, my bad.

Thread 1 (ethtool -L)       Thread 2 (work)        Thread 3 (ethtool -S)
netdev_lock()
iavf_set_channels()
...
iavf_alloc_queues()
-> alloc {tx,rx}_rings
-> num_active_queues = 8
iavf_schedule_finish_config()
netdev_unlock()
                                                   netdev_lock()
                                                   iavf_get_sset_count()
                                                   real_num_tx_queues: 1
                                                   -> buffer for 1 queue
                                                   iavf_get_ethtool_stats()
                                                   num_active_queues: 8
                                                   -> out-of-bounds!
                                                   netdev_unlock()
                            iavf_finish_config()
                            netdev_lock()
                            -> real_num_tx_queues = 8
                            netdev_unlock()

> 
> > +			tx_ring = &adapter->tx_rings[i];
> > +			rx_ring = &adapter->rx_rings[i];
> > +		}
> >  
> > -		/* Rx rings stats */
> > -		ring = &adapter->rx_rings[i];
> > -		iavf_add_queue_stats(&data, ring);
> > +		iavf_add_queue_stats(&data, tx_ring);
> > +		iavf_add_queue_stats(&data, rx_ring);
> >  	}
> >  	rcu_read_unlock();
> >  }
> 
> ...