Skip to content

Commit

Permalink
net: ena: fix race condition between device reset and link up setup
Browse files Browse the repository at this point in the history
In rare cases, ena driver would reset and re-start the device,
for example, in case of misbehaving application that causes
transmit timeout

The first step in the reset procedure is to stop the Tx traffic by
calling ena_carrier_off().

After the driver have just started the device reset procedure, device
happens to send an asynchronous notification (via AENQ) to the driver
than there was a link change (to link-up state).
This link change is mapped to a call to netif_carrier_on() which
re-activates the Tx queues, violating the assumption of no tx traffic
until device reset is completed, as the reset task might still be in
the process of queues initialization, leading to an access to
uninitialized memory.

Signed-off-by: Netanel Belgazal <netanel@amazon.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Netanel Belgazal authored and David S. Miller committed Nov 20, 2017
1 parent b399a39 commit d18e4f6
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
11 changes: 9 additions & 2 deletions drivers/net/ethernet/amazon/ena/ena_netdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -2579,6 +2579,7 @@ static int ena_restore_device(struct ena_adapter *adapter)
bool wd_state;
int rc;

set_bit(ENA_FLAG_ONGOING_RESET, &adapter->flags);
rc = ena_device_init(ena_dev, adapter->pdev, &get_feat_ctx, &wd_state);
if (rc) {
dev_err(&pdev->dev, "Can not initialize device\n");
Expand All @@ -2592,6 +2593,11 @@ static int ena_restore_device(struct ena_adapter *adapter)
goto err_device_destroy;
}

clear_bit(ENA_FLAG_ONGOING_RESET, &adapter->flags);
/* Make sure we don't have a race with AENQ Links state handler */
if (test_bit(ENA_FLAG_LINK_UP, &adapter->flags))
netif_carrier_on(adapter->netdev);

rc = ena_enable_msix_and_set_admin_interrupts(adapter,
adapter->num_queues);
if (rc) {
Expand All @@ -2618,7 +2624,7 @@ static int ena_restore_device(struct ena_adapter *adapter)
ena_com_admin_destroy(ena_dev);
err:
clear_bit(ENA_FLAG_DEVICE_RUNNING, &adapter->flags);

clear_bit(ENA_FLAG_ONGOING_RESET, &adapter->flags);
dev_err(&pdev->dev,
"Reset attempt failed. Can not reset the device\n");

Expand Down Expand Up @@ -3495,7 +3501,8 @@ static void ena_update_on_link_change(void *adapter_data,
if (status) {
netdev_dbg(adapter->netdev, "%s\n", __func__);
set_bit(ENA_FLAG_LINK_UP, &adapter->flags);
netif_carrier_on(adapter->netdev);
if (!test_bit(ENA_FLAG_ONGOING_RESET, &adapter->flags))
netif_carrier_on(adapter->netdev);
} else {
clear_bit(ENA_FLAG_LINK_UP, &adapter->flags);
netif_carrier_off(adapter->netdev);
Expand Down
3 changes: 2 additions & 1 deletion drivers/net/ethernet/amazon/ena/ena_netdev.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,8 @@ enum ena_flags_t {
ENA_FLAG_DEV_UP,
ENA_FLAG_LINK_UP,
ENA_FLAG_MSIX_ENABLED,
ENA_FLAG_TRIGGER_RESET
ENA_FLAG_TRIGGER_RESET,
ENA_FLAG_ONGOING_RESET
};

/* adapter specific private data structure */
Expand Down

0 comments on commit d18e4f6

Please sign in to comment.