Re: [PATCH 5.10 047/126] ath10k: hold RCU lock when calling ieee80211_find_sta_by_ifaddr()

[Pavel Machek]

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:

> +++ 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();
>               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;
>               }
>               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;
>       }
> +
> +exit:
> +     rcu_read_unlock();
>       kfree(tb);
>  }

The switch only takes the lock in 3 branches, but it is released
unconditionally at the end.

Something like this?

Best regards,
                                                                Pavel

Signed-off-by: Pavel Machek (CIP) <pa...@denx.de>

diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c 
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index e7072fc4f487..e03ff56d938b 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -582,20 +582,19 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k *ar, 
struct sk_buff *skb)
                                                       NULL);
                if (!station) {
                        ath10k_warn(ar, "did not find station from tdls peer 
event");
-                       goto exit;
-               }
-               arvif = ath10k_get_arvif(ar, __le32_to_cpu(ev->vdev_id));
-               ieee80211_tdls_oper_request(
+               } else {
+                       arvif = ath10k_get_arvif(ar, 
__le32_to_cpu(ev->vdev_id));
+                       ieee80211_tdls_oper_request(
                                        arvif->vif, station->addr,
                                        NL80211_TDLS_TEARDOWN,
                                        WLAN_REASON_TDLS_TEARDOWN_UNREACHABLE,
                                        GFP_ATOMIC
                                        );
+               }
+               rcu_read_unlock();
                break;
        }
 
-exit:
-       rcu_read_unlock();
        kfree(tb);
 }
 


-- 
http://www.livejournal.com/~pavelmachek

signature.asc
Description: Digital signature