| Message ID | 20241006153630.87885-1-nbd@nbd.name (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [wireless] wifi: mac80211: skip non-uploaded keys in ieee80211_iter_keys | expand |
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.
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
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 --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,
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(-)