Skip to content

Commit

Permalink
iwlwifi: fix and add missing sta_lock usage
Browse files Browse the repository at this point in the history
There are a few places where sta_lock is used, but the
station information protected by it is accessed outside
of the lock. Address this in two ways, if the access
won't sleep then just move the access into the lock, if
the access can sleep then copy the needed station
information to the stack to be accessed without risk of
it changing while access in progress.

Additionally, a number of other places access station
station information without holding the sta_lock, fix
those as well.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
  • Loading branch information
Reinette Chatre committed May 13, 2010
1 parent 94adfaa commit 9c5ac09
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 42 deletions.
9 changes: 3 additions & 6 deletions drivers/net/wireless/iwlwifi/iwl-3945.c
Original file line number Diff line number Diff line change
Expand Up @@ -946,8 +946,7 @@ void iwl3945_hw_build_tx_cmd_rate(struct iwl_priv *priv,
tx_cmd->supp_rates[1], tx_cmd->supp_rates[0]);
}

static u8 iwl3945_sync_sta(struct iwl_priv *priv, int sta_id,
u16 tx_rate, u8 flags)
static u8 iwl3945_sync_sta(struct iwl_priv *priv, int sta_id, u16 tx_rate)
{
unsigned long flags_spin;
struct iwl_station_entry *station;
Expand All @@ -961,10 +960,9 @@ static u8 iwl3945_sync_sta(struct iwl_priv *priv, int sta_id,
station->sta.sta.modify_mask = STA_MODIFY_TX_RATE_MSK;
station->sta.rate_n_flags = cpu_to_le16(tx_rate);
station->sta.mode = STA_CONTROL_MODIFY_MSK;

iwl_send_add_sta(priv, &station->sta, CMD_ASYNC);
spin_unlock_irqrestore(&priv->sta_lock, flags_spin);

iwl_send_add_sta(priv, &station->sta, flags);
IWL_DEBUG_RATE(priv, "SCALE sync station %d to rate %d\n",
sta_id, tx_rate);
return sta_id;
Expand Down Expand Up @@ -2472,8 +2470,7 @@ static int iwl3945_manage_ibss_station(struct iwl_priv *priv,

iwl3945_sync_sta(priv, vif_priv->ibss_bssid_sta_id,
(priv->band == IEEE80211_BAND_5GHZ) ?
IWL_RATE_6M_PLCP : IWL_RATE_1M_PLCP,
CMD_ASYNC);
IWL_RATE_6M_PLCP : IWL_RATE_1M_PLCP);
iwl3945_rate_scale_init(priv->hw, vif_priv->ibss_bssid_sta_id);

return 0;
Expand Down
5 changes: 4 additions & 1 deletion drivers/net/wireless/iwlwifi/iwl-4965.c
Original file line number Diff line number Diff line change
Expand Up @@ -2026,6 +2026,7 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
int sta_id;
int freed;
u8 *qc = NULL;
unsigned long flags;

if ((index >= txq->q.n_bd) || (iwl_queue_used(&txq->q, index) == 0)) {
IWL_ERR(priv, "Read index for DMA queue txq_id (%d) index %d "
Expand All @@ -2050,10 +2051,10 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
return;
}

spin_lock_irqsave(&priv->sta_lock, flags);
if (txq->sched_retry) {
const u32 scd_ssn = iwl4965_get_scd_ssn(tx_resp);
struct iwl_ht_agg *agg = NULL;

WARN_ON(!qc);

agg = &priv->stations[sta_id].tid[tid].agg;
Expand Down Expand Up @@ -2110,6 +2111,8 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
iwlagn_txq_check_empty(priv, sta_id, tid, txq_id);

iwl_check_abort_status(priv, tx_resp->frame_count, status);

spin_unlock_irqrestore(&priv->sta_lock, flags);
}

static int iwl4965_calc_rssi(struct iwl_priv *priv,
Expand Down
7 changes: 6 additions & 1 deletion drivers/net/wireless/iwlwifi/iwl-agn-lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ static void iwlagn_rx_reply_tx(struct iwl_priv *priv,
int tid;
int sta_id;
int freed;
unsigned long flags;

if ((index >= txq->q.n_bd) || (iwl_queue_used(&txq->q, index) == 0)) {
IWL_ERR(priv, "Read index for DMA queue txq_id (%d) index %d "
Expand All @@ -199,9 +200,10 @@ static void iwlagn_rx_reply_tx(struct iwl_priv *priv,
tid = (tx_resp->ra_tid & IWL50_TX_RES_TID_MSK) >> IWL50_TX_RES_TID_POS;
sta_id = (tx_resp->ra_tid & IWL50_TX_RES_RA_MSK) >> IWL50_TX_RES_RA_POS;

spin_lock_irqsave(&priv->sta_lock, flags);
if (txq->sched_retry) {
const u32 scd_ssn = iwlagn_get_scd_ssn(tx_resp);
struct iwl_ht_agg *agg = NULL;
struct iwl_ht_agg *agg;

agg = &priv->stations[sta_id].tid[tid].agg;

Expand Down Expand Up @@ -256,6 +258,7 @@ static void iwlagn_rx_reply_tx(struct iwl_priv *priv,
iwlagn_txq_check_empty(priv, sta_id, tid, txq_id);

iwl_check_abort_status(priv, tx_resp->frame_count, status);
spin_unlock_irqrestore(&priv->sta_lock, flags);
}

void iwlagn_rx_handler_setup(struct iwl_priv *priv)
Expand Down Expand Up @@ -1533,6 +1536,8 @@ int iwlagn_manage_ibss_station(struct iwl_priv *priv,
void iwl_free_tfds_in_queue(struct iwl_priv *priv,
int sta_id, int tid, int freed)
{
WARN_ON(!spin_is_locked(&priv->sta_lock));

if (priv->stations[sta_id].tid[tid].tfds_in_queue >= freed)
priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
else {
Expand Down
41 changes: 34 additions & 7 deletions drivers/net/wireless/iwlwifi/iwl-agn-tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -595,11 +595,17 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
}

txq_id = get_queue_from_ac(skb_get_queue_mapping(skb));

/* irqs already disabled/saved above when locking priv->lock */
spin_lock(&priv->sta_lock);

if (ieee80211_is_data_qos(fc)) {
qc = ieee80211_get_qos_ctl(hdr);
tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
if (unlikely(tid >= MAX_TID_COUNT))
if (WARN_ON_ONCE(tid >= MAX_TID_COUNT)) {
spin_unlock(&priv->sta_lock);
goto drop_unlock;
}
seq_number = priv->stations[sta_id].tid[tid].seq_number;
seq_number &= IEEE80211_SCTL_SEQ;
hdr->seq_ctrl = hdr->seq_ctrl &
Expand All @@ -617,11 +623,18 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
swq_id = txq->swq_id;
q = &txq->q;

if (unlikely(iwl_queue_space(q) < q->high_mark))
if (unlikely(iwl_queue_space(q) < q->high_mark)) {
spin_unlock(&priv->sta_lock);
goto drop_unlock;
}

if (ieee80211_is_data_qos(fc))
if (ieee80211_is_data_qos(fc)) {
priv->stations[sta_id].tid[tid].tfds_in_queue++;
if (!ieee80211_has_morefrags(fc))
priv->stations[sta_id].tid[tid].seq_number = seq_number;
}

spin_unlock(&priv->sta_lock);

/* Set up driver data for this TFD */
memset(&(txq->txb[q->write_ptr]), 0, sizeof(struct iwl_tx_info));
Expand Down Expand Up @@ -700,8 +713,6 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)

if (!ieee80211_has_morefrags(hdr->frame_control)) {
txq->need_update = 1;
if (qc)
priv->stations[sta_id].tid[tid].seq_number = seq_number;
} else {
wait_write_ptr = 1;
txq->need_update = 0;
Expand Down Expand Up @@ -1006,6 +1017,8 @@ int iwlagn_tx_agg_start(struct iwl_priv *priv, struct ieee80211_vif *vif,
if (ret)
return ret;

spin_lock_irqsave(&priv->sta_lock, flags);
tid_data = &priv->stations[sta_id].tid[tid];
if (tid_data->tfds_in_queue == 0) {
IWL_DEBUG_HT(priv, "HW queue is empty\n");
tid_data->agg.state = IWL_AGG_ON;
Expand All @@ -1015,6 +1028,7 @@ int iwlagn_tx_agg_start(struct iwl_priv *priv, struct ieee80211_vif *vif,
tid_data->tfds_in_queue);
tid_data->agg.state = IWL_EMPTYING_HW_QUEUE_ADDBA;
}
spin_unlock_irqrestore(&priv->sta_lock, flags);
return ret;
}

Expand All @@ -1037,11 +1051,14 @@ int iwlagn_tx_agg_stop(struct iwl_priv *priv, struct ieee80211_vif *vif,
return -ENXIO;
}

spin_lock_irqsave(&priv->sta_lock, flags);

if (priv->stations[sta_id].tid[tid].agg.state ==
IWL_EMPTYING_HW_QUEUE_ADDBA) {
IWL_DEBUG_HT(priv, "AGG stop before setup done\n");
ieee80211_stop_tx_ba_cb_irqsafe(vif, sta->addr, tid);
priv->stations[sta_id].tid[tid].agg.state = IWL_AGG_OFF;
spin_unlock_irqrestore(&priv->sta_lock, flags);
return 0;
}

Expand All @@ -1059,13 +1076,17 @@ int iwlagn_tx_agg_stop(struct iwl_priv *priv, struct ieee80211_vif *vif,
IWL_DEBUG_HT(priv, "Stopping a non empty AGG HW QUEUE\n");
priv->stations[sta_id].tid[tid].agg.state =
IWL_EMPTYING_HW_QUEUE_DELBA;
spin_unlock_irqrestore(&priv->sta_lock, flags);
return 0;
}

IWL_DEBUG_HT(priv, "HW queue is empty\n");
priv->stations[sta_id].tid[tid].agg.state = IWL_AGG_OFF;

spin_lock_irqsave(&priv->lock, flags);
/* do not restore/save irqs */
spin_unlock(&priv->sta_lock);
spin_lock(&priv->lock);

/*
* the only reason this call can fail is queue number out of range,
* which can happen if uCode is reloaded and all the station
Expand All @@ -1089,6 +1110,8 @@ int iwlagn_txq_check_empty(struct iwl_priv *priv,
u8 *addr = priv->stations[sta_id].sta.sta.addr;
struct iwl_tid_data *tid_data = &priv->stations[sta_id].tid[tid];

WARN_ON(!spin_is_locked(&priv->sta_lock));

switch (priv->stations[sta_id].tid[tid].agg.state) {
case IWL_EMPTYING_HW_QUEUE_DELBA:
/* We are reclaiming the last packet of the */
Expand All @@ -1113,6 +1136,7 @@ int iwlagn_txq_check_empty(struct iwl_priv *priv,
}
break;
}

return 0;
}

Expand Down Expand Up @@ -1276,6 +1300,7 @@ void iwlagn_rx_reply_compressed_ba(struct iwl_priv *priv,
int index;
int sta_id;
int tid;
unsigned long flags;

/* "flow" corresponds to Tx queue */
u16 scd_flow = le16_to_cpu(ba_resp->scd_flow);
Expand All @@ -1298,7 +1323,7 @@ void iwlagn_rx_reply_compressed_ba(struct iwl_priv *priv,
/* Find index just before block-ack window */
index = iwl_queue_dec_wrap(ba_resp_scd_ssn & 0xff, txq->q.n_bd);

/* TODO: Need to get this copy more safely - now good for debug */
spin_lock_irqsave(&priv->sta_lock, flags);

IWL_DEBUG_TX_REPLY(priv, "REPLY_COMPRESSED_BA [%d] Received from %pM, "
"sta_id = %d\n",
Expand Down Expand Up @@ -1334,4 +1359,6 @@ void iwlagn_rx_reply_compressed_ba(struct iwl_priv *priv,

iwlagn_txq_check_empty(priv, sta_id, tid, scd_flow);
}

spin_unlock_irqrestore(&priv->sta_lock, flags);
}
4 changes: 3 additions & 1 deletion drivers/net/wireless/iwlwifi/iwl-dev.h
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,9 @@ struct iwl_priv {
u8 bssid[ETH_ALEN]; /* used only on 3945 but filled by core */
u8 mac_addr[ETH_ALEN];

/*station table variables */
/* station table variables */

/* Note: if lock and sta_lock are needed, lock must be acquired first */
spinlock_t sta_lock;
int num_stations;
struct iwl_station_entry stations[IWL_STATION_COUNT];
Expand Down
Loading

0 comments on commit 9c5ac09

Please sign in to comment.