On 10/29/24 21:08, Kalle Valo wrote:
+ aditya
Jeff Johnson <quic_jjohnson@xxxxxxxxxxx> writes:
+static int ath12k_mac_station_unauthorize(struct ath12k *ar,
+ struct ath12k_link_vif *arvif,
+ struct ath12k_link_sta *arsta)
+{
+ struct ath12k_peer *peer;
+ int ret;
+
+ lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
+
+ spin_lock_bh(&ar->ab->base_lock);
+
+ peer = ath12k_peer_find(ar->ab, arvif->vdev_id, arsta->addr);
+ if (peer)
+ peer->is_authorized = false;
+
+ spin_unlock_bh(&ar->ab->base_lock);
+
+ /* Driver should clear the peer keys during mac80211's ref ptr
+ * gets cleared in __sta_info_destroy_part2 (trans from
+ * IEEE80211_STA_AUTHORIZED to IEEE80211_STA_ASSOC)
I'm unable to understand this comment
Indeed, that's weird. Aditya, do you have any idea what the comment is
trying to say?
At present, ath12k clear the keys in ath12k_station_disassoc() which
gets executed in state change from IEEE80211_STA_ASSOC to
IEEE80211_STA_AUTH.
However, in mac80211, once the station moves from
IEEE80211_STA_AUTHORIZED to IEEE80211_STA_ASSOC itself, the keys are
deleted. Please see - __sta_info_destroy_part2() ->
ieee80211_free_sta_keys().
Now, ath12k peer object (struct ath12k_peer) holds the key reference
from mac80211 (see ath12k_peer::keys[]). Hence, once mac80211 deletes
the key, driver should not keep a reference to it or else it could lead
to issues.
Therefore, it is important that the driver should clear the peer keys
during transition from IEEE80211_STA_AUTHORIZED to IEEE80211_STA_ASSOC
it self since we know that once we return from here, mac80211 is going
to remove the keys.
ath12k_mac_station_unauthorize() gets called when station moves from
state IEEE80211_STA_AUTHORIZED to IEEE80211_STA_ASSOC hence call to
ath12k_clear_peer_keys() is moved from ath12k_station_disassoc() to
ath12k_mac_station_unauthorize().
Is this clear now?
May be the comment in the code could be re-written as below?
/* Driver must clear the keys during the state change from
* IEEE80211_STA_AUTHORIZED to IEEE80211_STA_ASSOC, since after
* returning from here, mac80211 is going to delete the keys
* in __sta_info_destroy_part2(). This will ensure that the driver does
* not retain stale key references after mac80211 deletes the keys.
*/
--
Aditya
