Re: [PATCH] drivers/wireless: iwlwifi/mvm: Convert timers to use timer_setup()

[Luca Coelho]

On Tue, 2017-10-24 at 02:29 -0700, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list
> pointer to
> all timer callbacks, switch to using the new timer_setup() and
> from_timer()
> to pass the timer pointer explicitly.
> 
> The RCU lifetime on baid_data is unclear, so this adds a direct copy
> of the
> rcu_ptr passed to the original callback. It may be possible to
> improve this
> to just use baid_data->mvm->baid_map[baid_data->baid] instead.
> 
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: Luca Coelho 
> Cc: Intel Linux Wireless 
> Cc: Kalle Valo 
> Cc: Sara Sharon 
> Cc: linux-wireless@vger.kernel.org
> Cc: net...@vger.kernel.org
> Signed-off-by: Kees Cook 
> ---

Thanks, Kees.  I'm taking this for review on our internal tree.  If all
our checks pass, I'll apply it and it will reach the mainline following
our usual upstreaming process.

--
Cheers,
Luca.

[PATCH] drivers/wireless: iwlwifi/mvm: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

The RCU lifetime on baid_data is unclear, so this adds a direct copy of the
rcu_ptr passed to the original callback. It may be possible to improve this
to just use baid_data->mvm->baid_map[baid_data->baid] instead.

Cc: Johannes Berg 
Cc: Emmanuel Grumbach 
Cc: Luca Coelho 
Cc: Intel Linux Wireless 
Cc: Kalle Valo 
Cc: Sara Sharon 
Cc: linux-wireless@vger.kernel.org
Cc: net...@vger.kernel.org
Signed-off-by: Kees Cook 
---
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  3 ++-
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c |  4 ++--
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c  | 18 +-
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h 
b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index bf25c3ce7c95..a22c0ecff324 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -636,6 +636,7 @@ struct iwl_mvm_baid_data {
u16 timeout;
unsigned long last_rx;
struct timer_list session_timer;
+   struct iwl_mvm_baid_data __rcu **rcu_ptr;
struct iwl_mvm *mvm;
struct iwl_mvm_reorder_buffer reorder_buf[];
 };
@@ -1829,7 +1830,7 @@ void iwl_mvm_tdls_ch_switch_work(struct work_struct 
*work);
 void iwl_mvm_sync_rx_queues_internal(struct iwl_mvm *mvm,
 struct iwl_mvm_internal_rxq_notif *notif,
 u32 size);
-void iwl_mvm_reorder_timer_expired(unsigned long data);
+void iwl_mvm_reorder_timer_expired(struct timer_list *t);
 struct ieee80211_vif *iwl_mvm_get_bss_vif(struct iwl_mvm *mvm);
 bool iwl_mvm_is_vif_assoc(struct iwl_mvm *mvm);
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index 5e679859f948..53485dc096b3 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -456,9 +456,9 @@ static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
}
 }
 
-void iwl_mvm_reorder_timer_expired(unsigned long data)
+void iwl_mvm_reorder_timer_expired(struct timer_list *t)
 {
-   struct iwl_mvm_reorder_buffer *buf = (void *)data;
+   struct iwl_mvm_reorder_buffer *buf = from_timer(buf, t, reorder_timer);
int i;
u16 sn = 0, index = 0;
bool expired = false;
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index 282424f40c43..b95b24a03959 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -252,9 +252,11 @@ int iwl_mvm_sta_send_to_fw(struct iwl_mvm *mvm, struct 
ieee80211_sta *sta,
return ret;
 }
 
-static void iwl_mvm_rx_agg_session_expired(unsigned long data)
+static void iwl_mvm_rx_agg_session_expired(struct timer_list *t)
 {
-   struct iwl_mvm_baid_data __rcu **rcu_ptr = (void *)data;
+   struct iwl_mvm_baid_data *data =
+   from_timer(data, t, session_timer);
+   struct iwl_mvm_baid_data __rcu **rcu_ptr = data->rcu_ptr;
struct iwl_mvm_baid_data *ba_data;
struct ieee80211_sta *sta;
struct iwl_mvm_sta *mvm_sta;
@@ -2150,10 +2152,8 @@ static void iwl_mvm_init_reorder_buffer(struct iwl_mvm 
*mvm,
reorder_buf->head_sn = ssn;
reorder_buf->buf_size = buf_size;
/* rx reorder timer */
-   reorder_buf->reorder_timer.function =
-   iwl_mvm_reorder_timer_expired;
-   reorder_buf->reorder_timer.data = (unsigned long)reorder_buf;
-   init_timer(_buf->reorder_timer);
+   timer_setup(_buf->reorder_timer,
+   iwl_mvm_reorder_timer_expired, 0);
spin_lock_init(_buf->lock);
reorder_buf->mvm = mvm;
reorder_buf->queue = i;
@@ -2250,9 +2250,9 @@ int iwl_mvm_sta_rx_agg(struct iwl_mvm *mvm, struct 
ieee80211_sta *sta,
baid_data->baid = baid;
baid_data->timeout = timeout;
baid_data->last_rx = jiffies;
-   setup_timer(_data->session_timer,
-   iwl_mvm_rx_agg_session_expired,
-   (unsigned long)>baid_map[baid]);
+   baid_data->rcu_ptr = >baid_map[baid];
+   timer_setup(_data->session_timer,
+   iwl_mvm_rx_agg_session_expired, 0);
baid_data->mvm = mvm;
baid_data->tid = tid;
baid_data->sta_id = mvm_sta->sta_id;
-- 
2.7.4


-- 
Kees Cook
Pixel Security