Skip to content

Commit

Permalink
wil6210: fix race between disconnect and Tx NAPI
Browse files Browse the repository at this point in the history
When disconnecting some CID, corresponded Tx vring get released. During vring
release, all descriptors get freed. It is possible that Tx NAPI working on the same
vring simultaneously. If it happens, descriptor may be double freed.

To protect from the race above, make sure NAPI won't process the same vring.
Introduce 'enabled' flag in the struct vring_tx_data. Proceed with Tx NAPI only if
'enabled' flag set. Prior to Tx vring release, clear this flag and make sure NAPI
get synchronized.

NAPI enablement status protected by wil->mutex, add protection where it was
missing and check for it.

During reset, disconnect all peers first, then proceed with the Rx vring. It allows for
the disconnect flow to observe proper 'wil->status' and correctly notify cfg80211 about
connection status change

Signed-off-by: Vladimir Kondratiev <qca_vkondrat@qca.qualcomm.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
  • Loading branch information
Vladimir Kondratiev authored and John W. Linville committed Mar 17, 2014
1 parent 260e695 commit 097638a
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 4 deletions.
4 changes: 4 additions & 0 deletions drivers/net/wireless/ath/wil6210/cfg80211.c
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,11 @@ static int wil_cfg80211_del_station(struct wiphy *wiphy,
struct net_device *dev, u8 *mac)
{
struct wil6210_priv *wil = wiphy_to_wil(wiphy);

mutex_lock(&wil->mutex);
wil6210_disconnect(wil, mac);
mutex_unlock(&wil->mutex);

return 0;
}

Expand Down
17 changes: 13 additions & 4 deletions drivers/net/wireless/ath/wil6210/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ static void wil_disconnect_worker(struct work_struct *work)
struct wil6210_priv *wil = container_of(work,
struct wil6210_priv, disconnect_worker);

mutex_lock(&wil->mutex);
_wil6210_disconnect(wil, NULL);
mutex_unlock(&wil->mutex);
}

static void wil_connect_timer_fn(ulong x)
Expand Down Expand Up @@ -260,7 +262,9 @@ void wil_priv_deinit(struct wil6210_priv *wil)
{
cancel_work_sync(&wil->disconnect_worker);
cancel_work_sync(&wil->fw_error_worker);
mutex_lock(&wil->mutex);
wil6210_disconnect(wil, NULL);
mutex_unlock(&wil->mutex);
wmi_event_flush(wil);
destroy_workqueue(wil->wmi_wq_conn);
destroy_workqueue(wil->wmi_wq);
Expand Down Expand Up @@ -374,10 +378,14 @@ int wil_reset(struct wil6210_priv *wil)
{
int rc;

WARN_ON(!mutex_is_locked(&wil->mutex));

cancel_work_sync(&wil->disconnect_worker);
wil6210_disconnect(wil, NULL);

wil->status = 0; /* prevent NAPI from being scheduled */
if (test_bit(wil_status_napi_en, &wil->status)) {
napi_synchronize(&wil->napi_rx);
napi_synchronize(&wil->napi_tx);
}

if (wil->scan_request) {
Expand All @@ -387,9 +395,6 @@ int wil_reset(struct wil6210_priv *wil)
wil->scan_request = NULL;
}

cancel_work_sync(&wil->disconnect_worker);
wil6210_disconnect(wil, NULL);

wil6210_disable_irq(wil);

wmi_event_flush(wil);
Expand Down Expand Up @@ -447,6 +452,8 @@ static int __wil_up(struct wil6210_priv *wil)
struct wireless_dev *wdev = wil->wdev;
int rc;

WARN_ON(!mutex_is_locked(&wil->mutex));

rc = wil_reset(wil);
if (rc)
return rc;
Expand Down Expand Up @@ -506,6 +513,8 @@ int wil_up(struct wil6210_priv *wil)

static int __wil_down(struct wil6210_priv *wil)
{
WARN_ON(!mutex_is_locked(&wil->mutex));

clear_bit(wil_status_napi_en, &wil->status);
napi_disable(&wil->napi_rx);
napi_disable(&wil->napi_tx);
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/wireless/ath/wil6210/pcie_bus.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ static int wil_if_pcie_enable(struct wil6210_priv *wil)
goto stop_master;

/* need reset here to obtain MAC */
mutex_lock(&wil->mutex);
rc = wil_reset(wil);
mutex_unlock(&wil->mutex);
if (rc)
goto release_irq;

Expand Down
17 changes: 17 additions & 0 deletions drivers/net/wireless/ath/wil6210/txrx.c
Original file line number Diff line number Diff line change
Expand Up @@ -618,13 +618,15 @@ int wil_vring_init_tx(struct wil6210_priv *wil, int id, int size,
struct wmi_vring_cfg_done_event cmd;
} __packed reply;
struct vring *vring = &wil->vring_tx[id];
struct vring_tx_data *txdata = &wil->vring_tx_data[id];

if (vring->va) {
wil_err(wil, "Tx ring [%d] already allocated\n", id);
rc = -EINVAL;
goto out;
}

memset(txdata, 0, sizeof(*txdata));
vring->size = size;
rc = wil_vring_alloc(wil, vring);
if (rc)
Expand All @@ -648,6 +650,8 @@ int wil_vring_init_tx(struct wil6210_priv *wil, int id, int size,
}
vring->hwtail = le32_to_cpu(reply.cmd.tx_vring_tail_ptr);

txdata->enabled = 1;

return 0;
out_free:
wil_vring_free(wil, vring, 1);
Expand All @@ -660,9 +664,16 @@ void wil_vring_fini_tx(struct wil6210_priv *wil, int id)
{
struct vring *vring = &wil->vring_tx[id];

WARN_ON(!mutex_is_locked(&wil->mutex));

if (!vring->va)
return;

/* make sure NAPI won't touch this vring */
wil->vring_tx_data[id].enabled = 0;
if (test_bit(wil_status_napi_en, &wil->status))
napi_synchronize(&wil->napi_tx);

wil_vring_free(wil, vring, 1);
}

Expand Down Expand Up @@ -1028,6 +1039,7 @@ int wil_tx_complete(struct wil6210_priv *wil, int ringid)
struct net_device *ndev = wil_to_ndev(wil);
struct device *dev = wil_to_dev(wil);
struct vring *vring = &wil->vring_tx[ringid];
struct vring_tx_data *txdata = &wil->vring_tx_data[ringid];
int done = 0;
int cid = wil->vring2cid_tid[ringid][0];
struct wil_net_stats *stats = &wil->sta[cid].stats;
Expand All @@ -1038,6 +1050,11 @@ int wil_tx_complete(struct wil6210_priv *wil, int ringid)
return 0;
}

if (!txdata->enabled) {
wil_info(wil, "Tx irq[%d]: vring disabled\n", ringid);
return 0;
}

wil_dbg_txrx(wil, "%s(%d)\n", __func__, ringid);

while (!wil_vring_is_empty(vring)) {
Expand Down
9 changes: 9 additions & 0 deletions drivers/net/wireless/ath/wil6210/wil6210.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,14 @@ struct vring {
struct wil_ctx *ctx; /* ctx[size] - software context */
};

/**
* Additional data for Tx Vring
*/
struct vring_tx_data {
int enabled;

};

enum { /* for wil6210_priv.status */
wil_status_fwready = 0,
wil_status_fwconnecting,
Expand Down Expand Up @@ -386,6 +394,7 @@ struct wil6210_priv {
/* DMA related */
struct vring vring_rx;
struct vring vring_tx[WIL6210_MAX_TX_RINGS];
struct vring_tx_data vring_tx_data[WIL6210_MAX_TX_RINGS];
u8 vring2cid_tid[WIL6210_MAX_TX_RINGS][2]; /* [0] - CID, [1] - TID */
struct wil_sta_info sta[WIL6210_MAX_CID];
/* scan */
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/wireless/ath/wil6210/wmi.c
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,9 @@ static void wmi_evt_disconnect(struct wil6210_priv *wil, int id,

wil->sinfo_gen++;

mutex_lock(&wil->mutex);
wil6210_disconnect(wil, evt->bssid);
mutex_unlock(&wil->mutex);
}

static void wmi_evt_notify(struct wil6210_priv *wil, int id, void *d, int len)
Expand Down

0 comments on commit 097638a

Please sign in to comment.