Commit 1a51502b authored by Eric Dumazet's avatar Eric Dumazet Committed by Jeff Kirsher
Browse files

ixgbe: delay rx_ring freeing 



"cat /proc/net/dev" uses RCU protection only.

Its quite possible we call a driver get_stats() method while device is
dismantling and freeing its data structures.

So get_stats() methods must be very careful not accessing driver private
data without appropriate locking.

In ixgbe case, we access rx_ring pointers. These pointers are freed in
ixgbe_clear_interrupt_scheme() and set to NULL, this can trigger NULL
dereference in ixgbe_get_stats64()

A possible fix is to use RCU locking in ixgbe_get_stats64() and defer
rx_ring freeing after a grace period in ixgbe_clear_interrupt_scheme()

Signed-off-by: default avatarEric Dumazet <eric.dumazet@gmail.com>
Reported-by: default avatarTantilov, Emil S <emil.s.tantilov@intel.com>
Tested-by: default avatarRoss Brattain <ross.b.brattain@intel.com>
Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
parent b178bb3d

drivers/net/ixgbe/ixgbe.h

+1 −0
Original line number Diff line number Diff line
@@ -192,6 +192,7 @@ struct ixgbe_ring { 


	unsigned int size;		/* length in bytes */

	dma_addr_t dma;			/* phys. address of descriptor ring */

	struct rcu_head rcu;

} ____cacheline_internodealigned_in_smp;



enum ixgbe_ring_f_enum {

drivers/net/ixgbe/ixgbe_main.c

+24 −10
Original line number Diff line number Diff line
@@ -4751,6 +4751,11 @@ err_set_interrupt: 
	return err;

}



static void ring_free_rcu(struct rcu_head *head)

{

	kfree(container_of(head, struct ixgbe_ring, rcu));

}



/**

 * ixgbe_clear_interrupt_scheme - Clear the current interrupt scheme settings

 * @adapter: board private structure to clear interrupt scheme on
@@ -4767,7 +4772,12 @@ void ixgbe_clear_interrupt_scheme(struct ixgbe_adapter *adapter) 
		adapter->tx_ring[i] = NULL;

	}

	for (i = 0; i < adapter->num_rx_queues; i++) {

		kfree(adapter->rx_ring[i]);

		struct ixgbe_ring *ring = adapter->rx_ring[i];



		/* ixgbe_get_stats64() might access this ring, we must wait

		 * a grace period before freeing it.

		 */

		call_rcu(&ring->rcu, ring_free_rcu);

		adapter->rx_ring[i] = NULL;

	}
@@ -6563,11 +6573,13 @@ static struct rtnl_link_stats64 *ixgbe_get_stats64(struct net_device *netdev, 


	/* accurate rx/tx bytes/packets stats */

	dev_txq_stats_fold(netdev, stats);

	rcu_read_lock();

	for (i = 0; i < adapter->num_rx_queues; i++) {

		struct ixgbe_ring *ring = adapter->rx_ring[i];

		struct ixgbe_ring *ring = ACCESS_ONCE(adapter->rx_ring[i]);

		u64 bytes, packets;

		unsigned int start;



		if (ring) {

			do {

				start = u64_stats_fetch_begin_bh(&ring->syncp);

				packets = ring->stats.packets;
@@ -6576,7 +6588,8 @@ static struct rtnl_link_stats64 *ixgbe_get_stats64(struct net_device *netdev, 
			stats->rx_packets += packets;

			stats->rx_bytes   += bytes;

		}



	}

	rcu_read_unlock();

	/* following stats updated by ixgbe_watchdog_task() */

	stats->multicast	= netdev->stats.multicast;

	stats->rx_errors	= netdev->stats.rx_errors;
@@ -7282,6 +7295,7 @@ static void __exit ixgbe_exit_module(void) 
	dca_unregister_notify(&dca_notifier);

#endif

	pci_unregister_driver(&ixgbe_driver);

	rcu_barrier(); /* Wait for completion of call_rcu()'s */

}



#ifdef CONFIG_IXGBE_DCA

CRA Git | Maintained and supported by SUSTech CRA and CCSE