From patchwork Fri Apr 10 23:03:37 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Toppins X-Patchwork-Id: 460266 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 6D3B71402EC for ; Sat, 11 Apr 2015 09:03:58 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="verification failed; unprotected key" header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.b=VOtfCujc; dkim-adsp=none (unprotected policy); dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933135AbbDJXDx (ORCPT ); Fri, 10 Apr 2015 19:03:53 -0400 Received: from mail-pd0-f174.google.com ([209.85.192.174]:33899 "EHLO mail-pd0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933101AbbDJXDs (ORCPT ); Fri, 10 Apr 2015 19:03:48 -0400 Received: by pdbqa5 with SMTP id qa5so36628297pdb.1 for ; Fri, 10 Apr 2015 16:03:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=wYBvTEWj/AA8BQtzv3m5rJO0JWT/I1ln4WVn9RsTfiY=; b=VOtfCujcuEVeo5yWOlt8L9x62iZp73GzIZohWeefOsMbZm1cd72CtoZGpZT9Dmrrrz ubCUoBtTODh3sgS+H3HOSB/zODSc6KYpMq2e0PTl8C+ALkGda0UhFo/PTp/D2e39uJxj PXb/gYeXBxDqPJfZ0z0Q1+TXIAKn5vzRwMaRE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=wYBvTEWj/AA8BQtzv3m5rJO0JWT/I1ln4WVn9RsTfiY=; b=mx202xmB+zZfTj9MW+GgRPGvWXS/PD4aaeu7tKGo7OYF+a+b5R05E4JBvh+Cj+MO1f amLlp7y4c/uh6F5mS8N3IygaX275aNHSj3eNk1Hq+DSBQeBoQm0Fws26kCzMZShK91j3 kanFNZlQfxE49ON84xP6NhCnuaw+qiY0CUi30H2kP0/Kt9KlJMnat4jUx/zVogt7mFMv Svo+Qz2v2ze3FnMI86aWd587S3G2BhkMFaa+cceP3ne0LzSw5untKMZQcoc3LZOu/Yis S3OWgIFWGquDDEbMPj69Vd7nlhWtZiCXq4YAVi8u9n3q685EBv1OsI9QWXv9WuRt6EbC YwVg== X-Gm-Message-State: ALoCoQnSt63EuP7gAeKwGvVbwIL9ivgsGN+TgS9eLbRyf/sutHlq1bU66xVA/qhXSTHgJ/JqBvF8 X-Received: by 10.70.90.233 with SMTP id bz9mr6447072pdb.47.1428707027558; Fri, 10 Apr 2015 16:03:47 -0700 (PDT) Received: from monster-01.cumulusnetworks.com ([216.129.126.126]) by mx.google.com with ESMTPSA id ul8sm99132pab.36.2015.04.10.16.03.46 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Apr 2015 16:03:47 -0700 (PDT) From: Jonathan Toppins To: jeffrey.t.kirsher@intel.com Cc: jesse.brandeburg@intel.com, shannon.nelson@intel.com, carolyn.wyborny@intel.com, donald.c.skidmore@intel.com, matthew.vick@intel.com, john.ronciak@intel.com, mitch.a.williams@intel.com, intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org, gospo@cumulusnetworks.com, shm@cumulusnetworks.com Subject: [PATCH v1 net-next 2/3] igb: move initialization of link properties before igb_sw_init Date: Fri, 10 Apr 2015 16:03:37 -0700 Message-Id: <1428707018-6405-2-git-send-email-jtoppins@cumulusnetworks.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1428707018-6405-1-git-send-email-jtoppins@cumulusnetworks.com> References: <1428707018-6405-1-git-send-email-jtoppins@cumulusnetworks.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Tested-by: Aaron Brown --- 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 */