> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@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-
> l...@lists.osuosl.org; netdev@vger.kernel.org; go...@cumulusnetworks.com;
> s...@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 <jtopp...@cumulusnetworks.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Tested-by: Aaron Brown <aaron.f.br...@intel.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to