[Intel-wired-lan] [PATCH v1 net-next 2/3] igb: move initialization of link properties before igb_sw_init
Jonathan Toppins
jtoppins at cumulusnetworks.com
Fri Apr 10 23:03:37 UTC 2015
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(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index bb467bb..720b785 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2328,6 +2328,14 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (err)
goto err_sw_init;
+ /* Initialize link properties that are user-changeable */
+ adapter->fc_autoneg = true;
+ hw->mac.autoneg = true;
+ hw->phy.autoneg_advertised = 0x2f;
+
+ hw->fc.requested_mode = e1000_fc_default;
+ hw->fc.current_mode = e1000_fc_default;
+
/* setup the private structure */
err = igb_sw_init(adapter);
if (err)
@@ -2449,14 +2457,6 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
INIT_WORK(&adapter->reset_task, igb_reset_task);
INIT_WORK(&adapter->watchdog_task, igb_watchdog_task);
- /* Initialize link properties that are user-changeable */
- adapter->fc_autoneg = true;
- hw->mac.autoneg = true;
- hw->phy.autoneg_advertised = 0x2f;
-
- hw->fc.requested_mode = e1000_fc_default;
- hw->fc.current_mode = e1000_fc_default;
-
igb_validate_mdi_setting(hw);
/* By default, support wake on port A */
--
1.7.10.4
More information about the Intel-wired-lan
mailing list