[Intel-wired-lan] [PATCH v1 net-next 2/3] igb: move initialization of link properties before igb_sw_init

Brown, Aaron F aaron.f.brown at intel.com
Tue Apr 28 03:20:18 UTC 2015


> From: netdev-owner at vger.kernel.org [mailto:netdev-owner at vger.kernel.org]
> On Behalf Of Jonathan Toppins
> Sent: Friday, April 10, 2015 4:04 PM
> To: Kirsher, Jeffrey T
> Cc: Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald
> C; Vick, Matthew; Ronciak, John; Williams, Mitch A; intel-wired-
> lan at lists.osuosl.org; netdev at vger.kernel.org; gospo at cumulusnetworks.com;
> shm at cumulusnetworks.com
> Subject: [PATCH v1 net-next 2/3] igb: move initialization of link
> properties before igb_sw_init
> 
> This is required otherwise the driver may experience a NULL ptr
> dereference if CONFIG_PCI_IOV is set to yes.
> 
> Since the code can follow the flow on init (driver insmod):
> 	hw->mac.autoneg = false; (this is not set it is its default)
> 	igb_probe()
> 	+- igb_sw_init()
> 	    +- igb_probe_vfs()
> 	        +- igb_pci_enable_sriov()
> 	            +- igb_sriov_reinit()
> 	               +- igb_reset()
> 	                  trimmed path is the same see call path for
> 	                  igb_reset below
> 	+- hw->mac.autoneg = true;
> 	+- igb_reset()
> 
> 	/* igb_reset() call chain */
> 	igb_reset()
> 	+- hw->mac.ops.init_hw() == igb_init_hw_82575
> 	   +- igb_setup_link()
> 	      +- hw->mac.ops.setup_physical_interface() ==
> 	           igb_setup_copper_link_82575()
> 	         +- igb_setup_copper_link()
> 	            +- possible NULL dereference
> 
> Pseudo code from igb_setup_copper_link():
> 	if (hw->mac.autoneg) {
> 	    /* setup link */
> 	} else {
> 	    hw->phy.ops.force_speed_duplex(hw);  // <-- NULL deref here
> 	}
> 
> Since the way the current code is designed the driver will call
> igb_setup_copper_link twice if SRIOV is configured to be enabled. The
> call will occur once with autoneg == false and the second
> time autoneg == true. Since the decision to call the function pointer
> (hw->phy.ops.force_speed_duplex) is predicated on autoneg being set
> correctly move the setting of these parameters to as early as possible
> in the probe function.
> 
> Signed-off-by: Jonathan Toppins <jtoppins at cumulusnetworks.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Tested-by: Aaron Brown <aaron.f.brown at intel.com>


More information about the Intel-wired-lan mailing list