From 2096a53f2a5b655e88e75890d77276d6e01909b2 Mon Sep 17 00:00:00 2001 From: Dmitry Tarnyagin Date: Fri, 19 Aug 2011 14:35:55 +0200 Subject: cw1200: Fix incorrect usage of wait_event_interruptible_timeout. In many places in the driver wait_event_interruptible_timeout was wrongly used instead of wait_event_timeout. It caused several problems when upper layer received signals. A typical one was: [ 4553.767944] [BH] wakeup. [ 4553.768035] kernel BUG at drivers/staging/cw1200/wsm.c:1069! [ 4553.768066] Unable to handle kernel NULL pointer dereference at virtual address 00000000 ... [ 4553.770507] [] (__bug+0x1c/0x28) from [] (wsm_cmd_send+0x270/0x2cc [cw1200_core]) [ 4553.770538] [] (wsm_cmd_send+0x270/0x2cc [cw1200_core]) from [] (wsm_remove_key+0xf0/0x114 [cw1200_core]) ... [ 4553.771972] [] (sock_sendmsg+0xa0/0xbc) from [] (sys_sendmsg+0x1b0/0x20c) [ 4553.772003] [] (sys_sendmsg+0x1b0/0x20c) from [] (ret_fast_syscall+0x0/0x30) Change-Id: Ie0c2ded7348379a324f1f23fae4416014c272530 Signed-off-by: Dmitry Tarnyagin Reviewed-on: http://gerrit.lud.stericsson.com/gerrit/29164 Reviewed-by: Bartosz MARKOWSKI Reviewed-on: http://gerrit.lud.stericsson.com/gerrit/33513 Reviewed-by: Philippe LANGLAIS --- drivers/staging/cw1200/bh.c | 34 +++++++++++++------------ drivers/staging/cw1200/bh.h | 4 +-- drivers/staging/cw1200/pm.c | 56 ++++++++++++++++++++++-------------------- drivers/staging/cw1200/queue.c | 4 +-- drivers/staging/cw1200/sta.c | 4 +-- drivers/staging/cw1200/wsm.c | 14 +++++------ 6 files changed, 61 insertions(+), 55 deletions(-) diff --git a/drivers/staging/cw1200/bh.c b/drivers/staging/cw1200/bh.c index 1ecae7466f6..4c4a3bf86ad 100644 --- a/drivers/staging/cw1200/bh.c +++ b/drivers/staging/cw1200/bh.c @@ -89,7 +89,7 @@ void cw1200_unregister_bh(struct cw1200_common *priv) priv->bh_thread = NULL; bh_printk(KERN_DEBUG "[BH] unregister.\n"); atomic_add(1, &priv->bh_term); - wake_up_interruptible(&priv->bh_wq); + wake_up(&priv->bh_wq); kthread_stop(thread); #ifdef HAS_PUT_TASK_STRUCT put_task_struct(thread); @@ -103,7 +103,7 @@ void cw1200_irq_handler(struct cw1200_common *priv) return; if (atomic_add_return(1, &priv->bh_rx) == 1) - wake_up_interruptible(&priv->bh_wq); + wake_up(&priv->bh_wq); } void cw1200_bh_wakeup(struct cw1200_common *priv) @@ -113,31 +113,33 @@ void cw1200_bh_wakeup(struct cw1200_common *priv) return; if (atomic_add_return(1, &priv->bh_tx) == 1) - wake_up_interruptible(&priv->bh_wq); + wake_up(&priv->bh_wq); } -void cw1200_bh_suspend(struct cw1200_common *priv) +int cw1200_bh_suspend(struct cw1200_common *priv) { bh_printk(KERN_DEBUG "[BH] suspend.\n"); if (WARN_ON(priv->bh_error)) - return; + return 0; atomic_set(&priv->bh_suspend, CW1200_BH_SUSPEND); - wake_up_interruptible(&priv->bh_wq); - wait_event_interruptible(priv->bh_evt_wq, priv->bh_error || - (CW1200_BH_SUSPENDED == atomic_read(&priv->bh_suspend))); + wake_up(&priv->bh_wq); + return wait_event_timeout(priv->bh_evt_wq, priv->bh_error || + (CW1200_BH_SUSPENDED == atomic_read(&priv->bh_suspend)), + 1 * HZ) ? 0 : -ETIMEDOUT; } -void cw1200_bh_resume(struct cw1200_common *priv) +int cw1200_bh_resume(struct cw1200_common *priv) { bh_printk(KERN_DEBUG "[BH] resume.\n"); if (WARN_ON(priv->bh_error)) - return; + return 0; atomic_set(&priv->bh_suspend, CW1200_BH_RESUME); - wake_up_interruptible(&priv->bh_wq); - wait_event_interruptible(priv->bh_evt_wq, priv->bh_error || - (CW1200_BH_RESUMED == atomic_read(&priv->bh_suspend))); + wake_up(&priv->bh_wq); + return wait_event_timeout(priv->bh_evt_wq, priv->bh_error || + (CW1200_BH_RESUMED == atomic_read(&priv->bh_suspend)), + 1 * HZ) ? 0 : -ETIMEDOUT; } static inline void wsm_alloc_tx_buffer(struct cw1200_common *priv) @@ -156,7 +158,7 @@ int wsm_release_tx_buffer(struct cw1200_common *priv, int count) else if (hw_bufs_used >= priv->wsm_caps.numInpChBufs) ret = 1; if (!priv->hw_bufs_used) - wake_up_interruptible(&priv->bh_evt_wq); + wake_up(&priv->bh_evt_wq); return ret; } @@ -306,7 +308,7 @@ static int cw1200_bh(void *arg) } atomic_set(&priv->bh_suspend, CW1200_BH_SUSPENDED); - wake_up_interruptible(&priv->bh_evt_wq); + wake_up(&priv->bh_evt_wq); status = wait_event_interruptible(priv->bh_wq, CW1200_BH_RESUME == atomic_read( &priv->bh_suspend)); @@ -318,7 +320,7 @@ static int cw1200_bh(void *arg) } bh_printk(KERN_DEBUG "[BH] Device resume.\n"); atomic_set(&priv->bh_suspend, CW1200_BH_RESUMED); - wake_up_interruptible(&priv->bh_evt_wq); + wake_up(&priv->bh_evt_wq); atomic_add(1, &priv->bh_rx); continue; } diff --git a/drivers/staging/cw1200/bh.h b/drivers/staging/cw1200/bh.h index a19393c61dd..6d4d27b18c5 100644 --- a/drivers/staging/cw1200/bh.h +++ b/drivers/staging/cw1200/bh.h @@ -22,8 +22,8 @@ int cw1200_register_bh(struct cw1200_common *priv); void cw1200_unregister_bh(struct cw1200_common *priv); void cw1200_irq_handler(struct cw1200_common *priv); void cw1200_bh_wakeup(struct cw1200_common *priv); -void cw1200_bh_suspend(struct cw1200_common *priv); -void cw1200_bh_resume(struct cw1200_common *priv); +int cw1200_bh_suspend(struct cw1200_common *priv); +int cw1200_bh_resume(struct cw1200_common *priv); /* Must be called from BH thread. */ void cw1200_enable_powersave(struct cw1200_common *priv, bool enable); diff --git a/drivers/staging/cw1200/pm.c b/drivers/staging/cw1200/pm.c index 1d9207c8761..694f97851ee 100644 --- a/drivers/staging/cw1200/pm.c +++ b/drivers/staging/cw1200/pm.c @@ -173,40 +173,26 @@ int cw1200_wow_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan) /* Ensure pending operations are done. * Note also that wow_suspend must return in ~2.5sec, before * watchdog is triggered. */ - if (priv->channel_switch_in_progress) { - mutex_unlock(&priv->conf_mutex); - return -EBUSY; - } + if (priv->channel_switch_in_progress) + goto revert1; /* Do not suspend when join work is scheduled */ - if (work_pending(&priv->join_work)) { - mutex_unlock(&priv->conf_mutex); - return -EBUSY; - } + if (work_pending(&priv->join_work)) + goto revert1; /* Do not suspend when scanning */ - if (down_trylock(&priv->scan.lock)) { - mutex_unlock(&priv->conf_mutex); - return -EBUSY; - } + if (down_trylock(&priv->scan.lock)) + goto revert1; /* Lock TX. */ wsm_lock_tx_async(priv); - if (priv->hw_bufs_used) { - wsm_unlock_tx(priv); - up(&priv->scan.lock); - mutex_unlock(&priv->conf_mutex); - return -EBUSY; - } + if (priv->hw_bufs_used) + goto revert3; /* Allocate state */ state = kzalloc(sizeof(struct cw1200_suspend_state), GFP_KERNEL); - if (!state) { - wsm_unlock_tx(priv); - up(&priv->scan.lock); - mutex_unlock(&priv->conf_mutex); - return -ENOMEM; - } + if (!state) + goto revert3; /* Store delayed work states. */ state->bss_loss_tmo = @@ -219,7 +205,8 @@ int cw1200_wow_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan) cw1200_suspend_work(&priv->scan.probe_work); /* Stop serving thread */ - cw1200_bh_suspend(priv); + if (cw1200_bh_suspend(priv)) + goto revert4; /* Store suspend state */ pm_state->suspend_state = state; @@ -241,6 +228,23 @@ int cw1200_wow_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan) } return 0; + +revert4: + cw1200_resume_work(priv, &priv->bss_loss_work, + state->bss_loss_tmo); + cw1200_resume_work(priv, &priv->connection_loss_work, + state->connection_loss_tmo); + cw1200_resume_work(priv, &priv->join_timeout, + state->join_tmo); + cw1200_resume_work(priv, &priv->scan.probe_work, + state->direct_probe); + kfree(state); +revert3: + wsm_unlock_tx(priv); + up(&priv->scan.lock); +revert1: + mutex_unlock(&priv->conf_mutex); + return -EBUSY; } int cw1200_wow_resume(struct ieee80211_hw *hw) @@ -256,7 +260,7 @@ int cw1200_wow_resume(struct ieee80211_hw *hw) priv->sbus_ops->power_mgmt(priv->sbus_priv, false); /* Resume BH thread */ - cw1200_bh_resume(priv); + WARN_ON(cw1200_bh_resume(priv)); /* Resume delayed work */ cw1200_resume_work(priv, &priv->bss_loss_work, diff --git a/drivers/staging/cw1200/queue.c b/drivers/staging/cw1200/queue.c index 4140981eed1..a81442f8f4f 100644 --- a/drivers/staging/cw1200/queue.c +++ b/drivers/staging/cw1200/queue.c @@ -153,7 +153,7 @@ int cw1200_queue_clear(struct cw1200_queue *queue) } spin_unlock_bh(&stats->lock); spin_unlock_bh(&queue->lock); - wake_up_interruptible(&stats->wait_link_id_empty); + wake_up(&stats->wait_link_id_empty); return 0; } @@ -279,7 +279,7 @@ int cw1200_queue_get(struct cw1200_queue *queue, } spin_unlock_bh(&queue->lock); if (wakeup_stats) - wake_up_interruptible(&stats->wait_link_id_empty); + wake_up(&stats->wait_link_id_empty); return ret; } diff --git a/drivers/staging/cw1200/sta.c b/drivers/staging/cw1200/sta.c index 47608bb5ed8..b35dbca7ba0 100644 --- a/drivers/staging/cw1200/sta.c +++ b/drivers/staging/cw1200/sta.c @@ -289,7 +289,7 @@ int cw1200_config(struct ieee80211_hw *dev, u32 changed) cw1200_cancel_scan(priv); sta_printk(KERN_DEBUG "[STA] Freq %d (wsm ch: %d).\n", ch->center_freq, ch->hw_value); - WARN_ON(wait_event_interruptible_timeout( + WARN_ON(wait_event_timeout( priv->channel_switch_done, !priv->channel_switch_in_progress, 3 * HZ) <= 0); @@ -762,7 +762,7 @@ int __cw1200_flush(struct cw1200_common *priv, bool drop) } for (;;) { - ret = wait_event_interruptible_timeout( + ret = wait_event_timeout( priv->tx_queue_stats.wait_link_id_empty, cw1200_queue_stats_is_empty( &priv->tx_queue_stats, -1), diff --git a/drivers/staging/cw1200/wsm.c b/drivers/staging/cw1200/wsm.c index 37194870c56..2ad3eceaf69 100644 --- a/drivers/staging/cw1200/wsm.c +++ b/drivers/staging/cw1200/wsm.c @@ -842,7 +842,7 @@ static int wsm_startup_indication(struct cw1200_common *priv, priv->wsm_caps.firmwareReady = 1; - wake_up_interruptible(&priv->wsm_startup_done); + wake_up(&priv->wsm_startup_done); return 0; underflow: @@ -929,7 +929,7 @@ static int wsm_channel_switch_indication(struct cw1200_common *priv, WARN_ON(WSM_GET32(buf)); priv->channel_switch_in_progress = 0; - wake_up_interruptible(&priv->channel_switch_done); + wake_up(&priv->channel_switch_done); if (priv->wsm_cbc.channel_switch) priv->wsm_cbc.channel_switch(priv); @@ -1035,7 +1035,7 @@ int wsm_cmd_send(struct cw1200_common *priv, do { /* It's safe to use unprotected access to * wsm_cmd.done here */ - ret = wait_event_interruptible_timeout( + ret = wait_event_timeout( priv->wsm_cmd_wq, priv->wsm_cmd.done, tmo); rx_timestamp = jiffies - priv->rx_timestamp; @@ -1059,7 +1059,7 @@ int wsm_cmd_send(struct cw1200_common *priv, /* If wsm_handle_rx got stuck in _confirm we will hang * system there. It's better than silently currupt * stack or heap, isn't it? */ - BUG_ON(wait_event_interruptible_timeout( + BUG_ON(wait_event_timeout( priv->wsm_cmd_wq, priv->wsm_cmd.done, WSM_CMD_LAST_CHANCE_TIMEOUT) <= 0); } @@ -1081,7 +1081,7 @@ void wsm_lock_tx(struct cw1200_common *priv) { wsm_cmd_lock(priv); if (atomic_add_return(1, &priv->tx_lock) == 1) { - WARN_ON(wait_event_interruptible_timeout(priv->bh_evt_wq, + WARN_ON(wait_event_timeout(priv->bh_evt_wq, !priv->hw_bufs_used, WSM_CMD_LAST_CHANCE_TIMEOUT) <= 0); wsm_printk(KERN_DEBUG "[WSM] TX is locked.\n"); } @@ -1097,7 +1097,7 @@ void wsm_lock_tx_async(struct cw1200_common *priv) void wsm_flush_tx(struct cw1200_common *priv) { BUG_ON(!atomic_read(&priv->tx_lock)); - WARN_ON(wait_event_interruptible_timeout(priv->bh_evt_wq, + WARN_ON(wait_event_timeout(priv->bh_evt_wq, !priv->hw_bufs_used, WSM_CMD_LAST_CHANCE_TIMEOUT) <= 0); } @@ -1269,7 +1269,7 @@ int wsm_handle_rx(struct cw1200_common *priv, int id, spin_unlock(&priv->wsm_cmd.lock); ret = 0; /* Error response from device should ne stop BH. */ - wake_up_interruptible(&priv->wsm_cmd_wq); + wake_up(&priv->wsm_cmd_wq); } else if (id & 0x0800) { switch (id) { case 0x0801: -- cgit v1.2.3