[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