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

PATCH In review Kees Cook 2 participants 2 messages Oct 24, 2017 – Oct 25, 2017

Subsystems (3)

Networking Network drivers Intel wireless wifi link (iwlwifi)

lore

Share & tools

lore

Kees Cook

author

2017-10-24 09:29 UTC #0 lore raw

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

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 <[email protected]>

Cc Emmanuel Grumbach <[email protected]>

Cc Luca Coelho <[email protected]>

Cc Intel Linux Wireless <[email protected]>

Cc Kalle Valo <[email protected]>

Cc Sara Sharon <[email protected]>

Cc [email protected]

Signed-off-by Kees Cook <[email protected]>

---
 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(-)

drivers/net/wireless/intel/iwlwifi/mvm/mvm.h +2 -1

index bf25c3ce7c95..a22c0ecff324 100644
@@ -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);

drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c +2 -2

index 5e679859f948..53485dc096b3 100644
@@ -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;

drivers/net/wireless/intel/iwlwifi/mvm/sta.c +9 -9

index 282424f40c43..b95b24a03959 100644
@@ -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(&reorder_buf->reorder_timer);
+		timer_setup(&reorder_buf->reorder_timer,
+			    iwl_mvm_reorder_timer_expired, 0);
 		spin_lock_init(&reorder_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(&baid_data->session_timer,
-			    iwl_mvm_rx_agg_session_expired,
-			    (unsigned long)&mvm->baid_map[baid]);
+		baid_data->rcu_ptr = &mvm->baid_map[baid];
+		timer_setup(&baid_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;

Signature · 6 lines

2.7.4


-- 
Kees Cook
Pixel Security

Luca Coelho

2017-10-25 05:33 UTC #1 lore raw

Quoted 21 lines · 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 <[email protected]>
Cc Emmanuel Grumbach <[email protected]>
Cc Luca Coelho <[email protected]>
Cc Intel Linux Wireless <[email protected]>
Cc Kalle Valo <[email protected]>
Cc Sara Sharon <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>

Quoted 1 line

---

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.