lkml.org 
[lkml]   [2018]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 172/173] e1000e: Fix link check race condition
    Date
    4.14-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Benjamin Poirier <bpoirier@suse.com>

    commit e2710dbf0dc1e37d85368e2404049dadda848d5a upstream.

    Alex reported the following race condition:

    /* link goes up... interrupt... schedule watchdog */
    \ e1000_watchdog_task
    \ e1000e_has_link
    \ hw->mac.ops.check_for_link() === e1000e_check_for_copper_link
    \ e1000e_phy_has_link_generic(..., &link)
    link = true

    /* link goes down... interrupt */
    \ e1000_msix_other
    hw->mac.get_link_status = true

    /* link is up */
    mac->get_link_status = false

    link_active = true
    /* link_active is true, wrongly, and stays so because
    * get_link_status is false */

    Avoid this problem by making sure that we don't set get_link_status = false
    after having checked the link.

    It seems this problem has been present since the introduction of e1000e.

    Link: https://lkml.org/lkml/2018/1/29/338
    Reported-by: Alexander Duyck <alexander.duyck@gmail.com>
    Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
    Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
    Tested-by: Aaron Brown <aaron.f.brown@intel.com>
    Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
    Cc: Ben Hutchings <ben@decadent.org.uk>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/net/ethernet/intel/e1000e/ich8lan.c | 31 +++++++++++++++-------------
    drivers/net/ethernet/intel/e1000e/mac.c | 14 ++++++------
    2 files changed, 24 insertions(+), 21 deletions(-)

    --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
    +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
    @@ -1383,6 +1383,7 @@ static s32 e1000_check_for_copper_link_i
    */
    if (!mac->get_link_status)
    return 0;
    + mac->get_link_status = false;

    /* First we want to see if the MII Status Register reports
    * link. If so, then we want to get the current speed/duplex
    @@ -1390,12 +1391,12 @@ static s32 e1000_check_for_copper_link_i
    */
    ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
    if (ret_val)
    - return ret_val;
    + goto out;

    if (hw->mac.type == e1000_pchlan) {
    ret_val = e1000_k1_gig_workaround_hv(hw, link);
    if (ret_val)
    - return ret_val;
    + goto out;
    }

    /* When connected at 10Mbps half-duplex, some parts are excessively
    @@ -1428,7 +1429,7 @@ static s32 e1000_check_for_copper_link_i

    ret_val = hw->phy.ops.acquire(hw);
    if (ret_val)
    - return ret_val;
    + goto out;

    if (hw->mac.type == e1000_pch2lan)
    emi_addr = I82579_RX_CONFIG;
    @@ -1450,7 +1451,7 @@ static s32 e1000_check_for_copper_link_i
    hw->phy.ops.release(hw);

    if (ret_val)
    - return ret_val;
    + goto out;

    if (hw->mac.type >= e1000_pch_spt) {
    u16 data;
    @@ -1459,14 +1460,14 @@ static s32 e1000_check_for_copper_link_i
    if (speed == SPEED_1000) {
    ret_val = hw->phy.ops.acquire(hw);
    if (ret_val)
    - return ret_val;
    + goto out;

    ret_val = e1e_rphy_locked(hw,
    PHY_REG(776, 20),
    &data);
    if (ret_val) {
    hw->phy.ops.release(hw);
    - return ret_val;
    + goto out;
    }

    ptr_gap = (data & (0x3FF << 2)) >> 2;
    @@ -1480,18 +1481,18 @@ static s32 e1000_check_for_copper_link_i
    }
    hw->phy.ops.release(hw);
    if (ret_val)
    - return ret_val;
    + goto out;
    } else {
    ret_val = hw->phy.ops.acquire(hw);
    if (ret_val)
    - return ret_val;
    + goto out;

    ret_val = e1e_wphy_locked(hw,
    PHY_REG(776, 20),
    0xC023);
    hw->phy.ops.release(hw);
    if (ret_val)
    - return ret_val;
    + goto out;

    }
    }
    @@ -1518,7 +1519,7 @@ static s32 e1000_check_for_copper_link_i
    (hw->adapter->pdev->device == E1000_DEV_ID_PCH_I218_V3)) {
    ret_val = e1000_k1_workaround_lpt_lp(hw, link);
    if (ret_val)
    - return ret_val;
    + goto out;
    }
    if (hw->mac.type >= e1000_pch_lpt) {
    /* Set platform power management values for
    @@ -1526,7 +1527,7 @@ static s32 e1000_check_for_copper_link_i
    */
    ret_val = e1000_platform_pm_pch_lpt(hw, link);
    if (ret_val)
    - return ret_val;
    + goto out;
    }

    /* Clear link partner's EEE ability */
    @@ -1549,9 +1550,7 @@ static s32 e1000_check_for_copper_link_i
    }

    if (!link)
    - return 0; /* No link detected */
    -
    - mac->get_link_status = false;
    + goto out;

    switch (hw->mac.type) {
    case e1000_pch2lan:
    @@ -1617,6 +1616,10 @@ static s32 e1000_check_for_copper_link_i
    e_dbg("Error configuring flow control\n");

    return ret_val;
    +
    +out:
    + mac->get_link_status = true;
    + return ret_val;
    }

    static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter)
    --- a/drivers/net/ethernet/intel/e1000e/mac.c
    +++ b/drivers/net/ethernet/intel/e1000e/mac.c
    @@ -424,19 +424,15 @@ s32 e1000e_check_for_copper_link(struct
    */
    if (!mac->get_link_status)
    return 0;
    + mac->get_link_status = false;

    /* First we want to see if the MII Status Register reports
    * link. If so, then we want to get the current speed/duplex
    * of the PHY.
    */
    ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
    - if (ret_val)
    - return ret_val;
    -
    - if (!link)
    - return 0; /* No link detected */
    -
    - mac->get_link_status = false;
    + if (ret_val || !link)
    + goto out;

    /* Check if there was DownShift, must be checked
    * immediately after link-up
    @@ -465,6 +461,10 @@ s32 e1000e_check_for_copper_link(struct
    e_dbg("Error configuring flow control\n");

    return ret_val;
    +
    +out:
    + mac->get_link_status = true;
    + return ret_val;
    }

    /**

    \
     
     \ /
      Last update: 2018-09-24 14:26    [W:2.592 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site