lkml.org 
[lkml]   [2018]   [Apr]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 124/190] e1000e: fix race condition around skb_tstamp_tx()
    Date
    4.4-stable review patch.  If anyone has any objections, please let me know.

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

    From: Jacob Keller <jacob.e.keller@intel.com>


    [ Upstream commit 5012863b7347866764c4a4e58b62fb05346b0d06 ]

    The e1000e driver and related hardware has a limitation on Tx PTP
    packets which requires we limit to timestamping a single packet at once.
    We do this by verifying that we never request a new Tx timestamp while
    we still have a tx_hwtstamp_skb pointer.

    Unfortunately the driver suffers from a race condition around this. The
    tx_hwtstamp_skb pointer is not set to NULL until after skb_tstamp_tx()
    is called. This function notifies the stack and applications of a new
    timestamp. Even a well behaved application that only sends a new request
    when the first one is finished might be woken up and possibly send
    a packet before we can free the timestamp in the driver again. The
    result is that we needlessly ignore some Tx timestamp requests in this
    corner case.

    Fix this by assigning the tx_hwtstamp_skb pointer prior to calling
    skb_tstamp_tx() and use a temporary pointer to hold the timestamped skb
    until that function finishes. This ensures that the application is not
    woken up until the driver is ready to begin timestamping a new packet.

    This ensures that well behaved applications do not accidentally race
    with condition to skip Tx timestamps. Obviously an application which
    sends multiple Tx timestamp requests at once will still only timestamp
    one packet at a time. Unfortunately there is nothing we can do about
    this.

    Reported-by: David Mirabito <davidm@metamako.com>
    Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
    Tested-by: Aaron Brown <aaron.f.brown@intel.com>
    Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
    Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/net/ethernet/intel/e1000e/netdev.c | 10 ++++++++--
    1 file changed, 8 insertions(+), 2 deletions(-)

    --- a/drivers/net/ethernet/intel/e1000e/netdev.c
    +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
    @@ -1182,6 +1182,7 @@ static void e1000e_tx_hwtstamp_work(stru
    struct e1000_hw *hw = &adapter->hw;

    if (er32(TSYNCTXCTL) & E1000_TSYNCTXCTL_VALID) {
    + struct sk_buff *skb = adapter->tx_hwtstamp_skb;
    struct skb_shared_hwtstamps shhwtstamps;
    u64 txstmp;

    @@ -1190,9 +1191,14 @@ static void e1000e_tx_hwtstamp_work(stru

    e1000e_systim_to_hwtstamp(adapter, &shhwtstamps, txstmp);

    - skb_tstamp_tx(adapter->tx_hwtstamp_skb, &shhwtstamps);
    - dev_kfree_skb_any(adapter->tx_hwtstamp_skb);
    + /* Clear the global tx_hwtstamp_skb pointer and force writes
    + * prior to notifying the stack of a Tx timestamp.
    + */
    adapter->tx_hwtstamp_skb = NULL;
    + wmb(); /* force write prior to skb_tstamp_tx */
    +
    + skb_tstamp_tx(skb, &shhwtstamps);
    + dev_kfree_skb_any(skb);
    } else if (time_after(jiffies, adapter->tx_hwtstamp_start
    + adapter->tx_timeout_factor * HZ)) {
    dev_kfree_skb_any(adapter->tx_hwtstamp_skb);

    \
     
     \ /
      Last update: 2018-04-11 22:34    [W:4.127 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site