Messages in this thread Patch in this message | | | Subject | Re: [PATCH 5.10 047/126] ath10k: hold RCU lock when calling ieee80211_find_sta_by_ifaddr() | From | Shuah Khan <> | Date | Mon, 5 Apr 2021 18:27:15 -0600 |
| |
On 4/5/21 9:34 AM, Pavel Machek wrote: > Hi! > >> Fix ath10k_wmi_tlv_op_pull_peer_stats_info() to hold RCU lock before it >> calls ieee80211_find_sta_by_ifaddr() and release it when the resulting >> pointer is no longer needed. > > It does that. But is also does the unlock even if it did not take the > lock: >
There is only one path after it takes the lock.
>> +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c >> @@ -576,13 +576,13 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k *ar, struct sk_buff *skb) >> case WMI_TDLS_TEARDOWN_REASON_TX: >> case WMI_TDLS_TEARDOWN_REASON_RSSI: >> case WMI_TDLS_TEARDOWN_REASON_PTR_TIMEOUT: >> + rcu_read_lock();
Takes the lock here:
>> station = ieee80211_find_sta_by_ifaddr(ar->hw, >> ev->peer_macaddr.addr, >> NULL); >> if (!station) { >> ath10k_warn(ar, "did not find station from tdls peer event"); >> - kfree(tb); >> - return; >> + goto exit;
Releases it if something fails
>> } >> arvif = ath10k_get_arvif(ar, __le32_to_cpu(ev->vdev_id)); >> ieee80211_tdls_oper_request( >> @@ -593,6 +593,9 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k *ar, struct sk_buff *skb) >> ); >> break; >> } >> +
falls through here.
>> +exit: >> + rcu_read_unlock(); >> kfree(tb); >> } > > The switch only takes the lock in 3 branches, but it is released > unconditionally at the end. >
I don't see any other switch cases. However, default is missing:
It could be done this way perhaps: (simpler than what you proposed)
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c index d97b33f789e4..7efbe03fbca8 100644 --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c @@ -592,6 +592,9 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k *ar, struct sk_buff *skb) GFP_ATOMIC ); break; + default: + kfree(tb); + return; }
exit: thanks, -- Shuah
| |