diff mbox series

[wireless] wifi: mac80211: skip non-uploaded keys in ieee80211_iter_keys

Message ID 20241006153630.87885-1-nbd@nbd.name
State New
Headers show
Series [wireless] wifi: mac80211: skip non-uploaded keys in ieee80211_iter_keys | expand

Commit Message

Felix Fietkau Oct. 6, 2024, 3:36 p.m. UTC
Sync iterator conditions with ieee80211_iter_keys_rcu.

Fixes: 830af02f24fb ("mac80211: allow driver to iterate keys")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/mac80211/key.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

Comments

Kalle Valo Oct. 15, 2024, 9:13 a.m. UTC | #1
Felix Fietkau <nbd@nbd.name> writes:

> Sync iterator conditions with ieee80211_iter_keys_rcu.
>
> Fixes: 830af02f24fb ("mac80211: allow driver to iterate keys")
> Signed-off-by: Felix Fietkau <nbd@nbd.name>

I see this is already applied but why we need this? Please include a
description of the bug in the commit message. That's too late now but if
you provide one via email I can include it in the pull request.
Felix Fietkau Oct. 15, 2024, 9:42 a.m. UTC | #2
On 15.10.24 11:13, Kalle Valo wrote:
> Felix Fietkau <nbd@nbd.name> writes:
> 
>> Sync iterator conditions with ieee80211_iter_keys_rcu.
>>
>> Fixes: 830af02f24fb ("mac80211: allow driver to iterate keys")
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> 
> I see this is already applied but why we need this? Please include a
> description of the bug in the commit message. That's too late now but if
> you provide one via email I can include it in the pull request.

I needed the key iterator for not yet published mt7996 work, and while 
reading the code found the inconsistency, that (unlike the RCU version) 
it could pass keys that weren't added to the driver yet.
I didn't see any specific driver bugs, but the mac80211 code didn't make 
sense to me during review, so I fixed it.

- Felix
Kalle Valo Oct. 16, 2024, 9:41 a.m. UTC | #3
Felix Fietkau <nbd@nbd.name> writes:

> On 15.10.24 11:13, Kalle Valo wrote:
>> Felix Fietkau <nbd@nbd.name> writes:
>> 
>>> Sync iterator conditions with ieee80211_iter_keys_rcu.
>>>
>>> Fixes: 830af02f24fb ("mac80211: allow driver to iterate keys")
>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>> I see this is already applied but why we need this? Please include a
>> description of the bug in the commit message. That's too late now but if
>> you provide one via email I can include it in the pull request.
>
> I needed the key iterator for not yet published mt7996 work, and while
> reading the code found the inconsistency, that (unlike the RCU
> version) it could pass keys that weren't added to the driver yet.
> I didn't see any specific driver bugs, but the mac80211 code didn't
> make sense to me during review, so I fixed it.

Ah, if this is just a theoretical fix I don't mention anything about
this in the pull request. Thanks.
diff mbox series

Patch

diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index eecdd2265eaa..e45b5f56c405 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -987,6 +987,26 @@  void ieee80211_reenable_keys(struct ieee80211_sub_if_data *sdata)
 	}
 }
 
+static void
+ieee80211_key_iter(struct ieee80211_hw *hw,
+		   struct ieee80211_vif *vif,
+		   struct ieee80211_key *key,
+		   void (*iter)(struct ieee80211_hw *hw,
+				struct ieee80211_vif *vif,
+				struct ieee80211_sta *sta,
+				struct ieee80211_key_conf *key,
+				void *data),
+		   void *iter_data)
+{
+	/* skip keys of station in removal process */
+	if (key->sta && key->sta->removed)
+		return;
+	if (!(key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE))
+		return;
+	iter(hw, vif, key->sta ? &key->sta->sta : NULL,
+	     &key->conf, iter_data);
+}
+
 void ieee80211_iter_keys(struct ieee80211_hw *hw,
 			 struct ieee80211_vif *vif,
 			 void (*iter)(struct ieee80211_hw *hw,
@@ -1005,16 +1025,13 @@  void ieee80211_iter_keys(struct ieee80211_hw *hw,
 	if (vif) {
 		sdata = vif_to_sdata(vif);
 		list_for_each_entry_safe(key, tmp, &sdata->key_list, list)
-			iter(hw, &sdata->vif,
-			     key->sta ? &key->sta->sta : NULL,
-			     &key->conf, iter_data);
+			ieee80211_key_iter(hw, vif, key, iter, iter_data);
 	} else {
 		list_for_each_entry(sdata, &local->interfaces, list)
 			list_for_each_entry_safe(key, tmp,
 						 &sdata->key_list, list)
-				iter(hw, &sdata->vif,
-				     key->sta ? &key->sta->sta : NULL,
-				     &key->conf, iter_data);
+				ieee80211_key_iter(hw, &sdata->vif, key,
+						   iter, iter_data);
 	}
 }
 EXPORT_SYMBOL(ieee80211_iter_keys);
@@ -1031,17 +1048,8 @@  _ieee80211_iter_keys_rcu(struct ieee80211_hw *hw,
 {
 	struct ieee80211_key *key;
 
-	list_for_each_entry_rcu(key, &sdata->key_list, list) {
-		/* skip keys of station in removal process */
-		if (key->sta && key->sta->removed)
-			continue;
-		if (!(key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE))
-			continue;
-
-		iter(hw, &sdata->vif,
-		     key->sta ? &key->sta->sta : NULL,
-		     &key->conf, iter_data);
-	}
+	list_for_each_entry_rcu(key, &sdata->key_list, list)
+		ieee80211_key_iter(hw, &sdata->vif, key, iter, iter_data);
 }
 
 void ieee80211_iter_keys_rcu(struct ieee80211_hw *hw,