mwifiex: fix race when queuing commands

Running the following script repeatedly on XO-4 with SD8787
produces command timeout and system lockup.

insmod mwifiex_sdio.ko
sleep 1
ifconfig eth0 up
iwlist eth0 scan &
sleep 0.5
rmmod mwifiex_sdio

mwifiex_send_cmd_async() is called for sync as well as async
commands. (mwifiex_send_cmd_sync() internally calls it for
sync command.)

"adapter->cmd_queued" gets filled inside mwifiex_send_cmd_async()
routine for both types of commands. But it is used only for sync
commands in mwifiex_wait_queue_complete(). This could lead to a
race when two threads try to queue a sync command with another
sync/async command simultaneously.

Get rid of global variable and pass command node as a parameter
to mwifiex_wait_queue_complete() to fix the problem.

Cc: <stable@vger.kernel.org> # 3.8
Reported-by: Daniel Drake <dsd@laptop.org>
Tested-by: Daniel Drake <dsd@laptop.org>
Tested-by: Marco Cesarano <marco@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
This commit is contained in:
Amitkumar Karwar 2013-03-15 18:47:05 -07:00 committed by John W. Linville
parent 01d4ab96d2
commit 00d7ea11ff
4 changed files with 10 additions and 17 deletions

View File

@ -484,8 +484,6 @@ int mwifiex_send_cmd_sync(struct mwifiex_private *priv, uint16_t cmd_no,
ret = mwifiex_send_cmd_async(priv, cmd_no, cmd_action, cmd_oid, ret = mwifiex_send_cmd_async(priv, cmd_no, cmd_action, cmd_oid,
data_buf); data_buf);
if (!ret)
ret = mwifiex_wait_queue_complete(adapter);
return ret; return ret;
} }
@ -588,9 +586,10 @@ int mwifiex_send_cmd_async(struct mwifiex_private *priv, uint16_t cmd_no,
if (cmd_no == HostCmd_CMD_802_11_SCAN) { if (cmd_no == HostCmd_CMD_802_11_SCAN) {
mwifiex_queue_scan_cmd(priv, cmd_node); mwifiex_queue_scan_cmd(priv, cmd_node);
} else { } else {
adapter->cmd_queued = cmd_node;
mwifiex_insert_cmd_to_pending_q(adapter, cmd_node, true); mwifiex_insert_cmd_to_pending_q(adapter, cmd_node, true);
queue_work(adapter->workqueue, &adapter->main_work); queue_work(adapter->workqueue, &adapter->main_work);
if (cmd_node->wait_q_enabled)
ret = mwifiex_wait_queue_complete(adapter, cmd_node);
} }
return ret; return ret;

View File

@ -723,7 +723,6 @@ struct mwifiex_adapter {
u16 cmd_wait_q_required; u16 cmd_wait_q_required;
struct mwifiex_wait_queue cmd_wait_q; struct mwifiex_wait_queue cmd_wait_q;
u8 scan_wait_q_woken; u8 scan_wait_q_woken;
struct cmd_ctrl_node *cmd_queued;
spinlock_t queue_lock; /* lock for tx queues */ spinlock_t queue_lock; /* lock for tx queues */
struct completion fw_load; struct completion fw_load;
u8 country_code[IEEE80211_COUNTRY_STRING_LEN]; u8 country_code[IEEE80211_COUNTRY_STRING_LEN];
@ -1018,7 +1017,8 @@ int mwifiex_request_set_multicast_list(struct mwifiex_private *priv,
struct mwifiex_multicast_list *mcast_list); struct mwifiex_multicast_list *mcast_list);
int mwifiex_copy_mcast_addr(struct mwifiex_multicast_list *mlist, int mwifiex_copy_mcast_addr(struct mwifiex_multicast_list *mlist,
struct net_device *dev); struct net_device *dev);
int mwifiex_wait_queue_complete(struct mwifiex_adapter *adapter); int mwifiex_wait_queue_complete(struct mwifiex_adapter *adapter,
struct cmd_ctrl_node *cmd_queued);
int mwifiex_bss_start(struct mwifiex_private *priv, struct cfg80211_bss *bss, int mwifiex_bss_start(struct mwifiex_private *priv, struct cfg80211_bss *bss,
struct cfg80211_ssid *req_ssid); struct cfg80211_ssid *req_ssid);
int mwifiex_cancel_hs(struct mwifiex_private *priv, int cmd_type); int mwifiex_cancel_hs(struct mwifiex_private *priv, int cmd_type);

View File

@ -1388,10 +1388,13 @@ int mwifiex_scan_networks(struct mwifiex_private *priv,
list_del(&cmd_node->list); list_del(&cmd_node->list);
spin_unlock_irqrestore(&adapter->scan_pending_q_lock, spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
flags); flags);
adapter->cmd_queued = cmd_node;
mwifiex_insert_cmd_to_pending_q(adapter, cmd_node, mwifiex_insert_cmd_to_pending_q(adapter, cmd_node,
true); true);
queue_work(adapter->workqueue, &adapter->main_work); queue_work(adapter->workqueue, &adapter->main_work);
/* Perform internal scan synchronously */
if (!priv->scan_request)
mwifiex_wait_queue_complete(adapter, cmd_node);
} else { } else {
spin_unlock_irqrestore(&adapter->scan_pending_q_lock, spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
flags); flags);
@ -1946,9 +1949,6 @@ int mwifiex_request_scan(struct mwifiex_private *priv,
/* Normal scan */ /* Normal scan */
ret = mwifiex_scan_networks(priv, NULL); ret = mwifiex_scan_networks(priv, NULL);
if (!ret)
ret = mwifiex_wait_queue_complete(priv->adapter);
up(&priv->async_sem); up(&priv->async_sem);
return ret; return ret;

View File

@ -54,16 +54,10 @@ int mwifiex_copy_mcast_addr(struct mwifiex_multicast_list *mlist,
* This function waits on a cmd wait queue. It also cancels the pending * This function waits on a cmd wait queue. It also cancels the pending
* request after waking up, in case of errors. * request after waking up, in case of errors.
*/ */
int mwifiex_wait_queue_complete(struct mwifiex_adapter *adapter) int mwifiex_wait_queue_complete(struct mwifiex_adapter *adapter,
struct cmd_ctrl_node *cmd_queued)
{ {
int status; int status;
struct cmd_ctrl_node *cmd_queued;
if (!adapter->cmd_queued)
return 0;
cmd_queued = adapter->cmd_queued;
adapter->cmd_queued = NULL;
dev_dbg(adapter->dev, "cmd pending\n"); dev_dbg(adapter->dev, "cmd pending\n");
atomic_inc(&adapter->cmd_pending); atomic_inc(&adapter->cmd_pending);