Search Linux Wireless

Re: [PATCH 2/2] wifi: wilc1000: fix RCU usage in connect path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 04/01/2024 à 15:39, Alexis Lothoré a écrit :
From: Alexis Lothoré <alexis.lothore-LDxbnhwyfcJBDgjK7y7TUQ@xxxxxxxxxxxxxxxx>

With lockdep enabled, calls to the connect function from cfg802.11 layer
lead to the following warning:

=============================
WARNING: suspicious RCU usage
6.7.0-rc1-wt+ #333 Not tainted
-----------------------------
drivers/net/wireless/microchip/wilc1000/hif.c:386
suspicious rcu_dereference_check() usage!
[...]
stack backtrace:
CPU: 0 PID: 100 Comm: wpa_supplicant Not tainted 6.7.0-rc1-wt+ #333
Hardware name: Atmel SAMA5
  unwind_backtrace from show_stack+0x18/0x1c
  show_stack from dump_stack_lvl+0x34/0x48
  dump_stack_lvl from wilc_parse_join_bss_param+0x7dc/0x7f4
  wilc_parse_join_bss_param from connect+0x2c4/0x648
  connect from cfg80211_connect+0x30c/0xb74
  cfg80211_connect from nl80211_connect+0x860/0xa94
  nl80211_connect from genl_rcv_msg+0x3fc/0x59c
  genl_rcv_msg from netlink_rcv_skb+0xd0/0x1f8
  netlink_rcv_skb from genl_rcv+0x2c/0x3c
  genl_rcv from netlink_unicast+0x3b0/0x550
  netlink_unicast from netlink_sendmsg+0x368/0x688
  netlink_sendmsg from ____sys_sendmsg+0x190/0x430
  ____sys_sendmsg from ___sys_sendmsg+0x110/0x158
  ___sys_sendmsg from sys_sendmsg+0xe8/0x150
  sys_sendmsg from ret_fast_syscall+0x0/0x1c

This warning is emitted because in the connect path, when trying to parse
target BSS parameters, we dereference a RCU pointer whithout being in RCU
critical section.
Fix RCU dereference usage by moving it to a RCU read critical section. To
avoid wrapping the whole wilc_parse_join_bss_param under the critical
section, just use the critical section to copy ies data

Fixes: c460495ee072 ("staging: wilc1000: fix incorrent type in initializer")
Signed-off-by: Alexis Lothoré <alexis.lothore-LDxbnhwyfcJBDgjK7y7TUQ@xxxxxxxxxxxxxxxx>
---
  drivers/net/wireless/microchip/wilc1000/hif.c | 35 ++++++++++++-------
  1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/hif.c b/drivers/net/wireless/microchip/wilc1000/hif.c
index 6f1218a51061..806b7337b5ae 100644
--- a/drivers/net/wireless/microchip/wilc1000/hif.c
+++ b/drivers/net/wireless/microchip/wilc1000/hif.c
@@ -377,38 +377,48 @@ struct wilc_join_bss_param *
  wilc_parse_join_bss_param(struct cfg80211_bss *bss,
  			  struct cfg80211_crypto_settings *crypto)
  {
-	const struct cfg80211_bss_ies *ies = rcu_dereference(bss->ies);
-	const u8 *tim_elm, *ssid_elm, *rates_ie, *supp_rates_ie;
+	const u8 *ies_data, *tim_elm, *ssid_elm, *rates_ie, *supp_rates_ie;
  	const u8 *ht_ie, *wpa_ie, *wmm_ie, *rsn_ie;
  	struct ieee80211_p2p_noa_attr noa_attr;
+	const struct cfg80211_bss_ies *ies;
  	struct wilc_join_bss_param *param;
-	u8 rates_len = 0;
+	u8 rates_len = 0, ies_len;
  	int ret;
param = kzalloc(sizeof(*param), GFP_KERNEL);
  	if (!param)
  		return NULL;
+ rcu_read_lock();
+	ies = rcu_dereference(bss->ies);
+	ies_data = kmemdup(ies->data, ies->len, GFP_ATOMIC);

Is it mandatory to have GFP_ATOMIC here?
If yes, then some other drivers may need to be fixed (at least [1]).

+	if (!ies_data) {
+		rcu_read_unlock();

'param' allocated just a few lines above is leaking now.

CJ


[1]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/p2pdma.c#n660


+		return NULL;
+	}
+	ies_len = ies->len;
+	rcu_read_unlock();
+
  	param->beacon_period = cpu_to_le16(bss->beacon_interval);
  	param->cap_info = cpu_to_le16(bss->capability);
  	param->bss_type = WILC_FW_BSS_TYPE_INFRA;
  	param->ch = ieee80211_frequency_to_channel(bss->channel->center_freq);
  	ether_addr_copy(param->bssid, bss->bssid);
- ssid_elm = cfg80211_find_ie(WLAN_EID_SSID, ies->data, ies->len);
+	ssid_elm = cfg80211_find_ie(WLAN_EID_SSID, ies_data, ies_len);
  	if (ssid_elm) {
  		if (ssid_elm[1] <= IEEE80211_MAX_SSID_LEN)
  			memcpy(param->ssid, ssid_elm + 2, ssid_elm[1]);
  	}
- tim_elm = cfg80211_find_ie(WLAN_EID_TIM, ies->data, ies->len);
+	tim_elm = cfg80211_find_ie(WLAN_EID_TIM, ies_data, ies_len);
  	if (tim_elm && tim_elm[1] >= 2)
  		param->dtim_period = tim_elm[3];
memset(param->p_suites, 0xFF, 3);
  	memset(param->akm_suites, 0xFF, 3);
- rates_ie = cfg80211_find_ie(WLAN_EID_SUPP_RATES, ies->data, ies->len);
+	rates_ie = cfg80211_find_ie(WLAN_EID_SUPP_RATES, ies_data, ies_len);
  	if (rates_ie) {
  		rates_len = rates_ie[1];
  		if (rates_len > WILC_MAX_RATES_SUPPORTED)
@@ -419,7 +429,7 @@ wilc_parse_join_bss_param(struct cfg80211_bss *bss,
if (rates_len < WILC_MAX_RATES_SUPPORTED) {
  		supp_rates_ie = cfg80211_find_ie(WLAN_EID_EXT_SUPP_RATES,
-						 ies->data, ies->len);
+						 ies_data, ies_len);
  		if (supp_rates_ie) {
  			u8 ext_rates = supp_rates_ie[1];
@@ -434,11 +444,11 @@ wilc_parse_join_bss_param(struct cfg80211_bss *bss,
  		}
  	}
- ht_ie = cfg80211_find_ie(WLAN_EID_HT_CAPABILITY, ies->data, ies->len);
+	ht_ie = cfg80211_find_ie(WLAN_EID_HT_CAPABILITY, ies_data, ies_len);
  	if (ht_ie)
  		param->ht_capable = true;
- ret = cfg80211_get_p2p_attr(ies->data, ies->len,
+	ret = cfg80211_get_p2p_attr(ies_data, ies_len,
  				    IEEE80211_P2P_ATTR_ABSENCE_NOTICE,
  				    (u8 *)&noa_attr, sizeof(noa_attr));
  	if (ret > 0) {
@@ -462,7 +472,7 @@ wilc_parse_join_bss_param(struct cfg80211_bss *bss,
  	}
  	wmm_ie = cfg80211_find_vendor_ie(WLAN_OUI_MICROSOFT,
  					 WLAN_OUI_TYPE_MICROSOFT_WMM,
-					 ies->data, ies->len);
+					 ies_data, ies_len);
  	if (wmm_ie) {
  		struct ieee80211_wmm_param_ie *ie;
@@ -477,13 +487,13 @@ wilc_parse_join_bss_param(struct cfg80211_bss *bss, wpa_ie = cfg80211_find_vendor_ie(WLAN_OUI_MICROSOFT,
  					 WLAN_OUI_TYPE_MICROSOFT_WPA,
-					 ies->data, ies->len);
+					 ies_data, ies_len);
  	if (wpa_ie) {
  		param->mode_802_11i = 1;
  		param->rsn_found = true;
  	}
- rsn_ie = cfg80211_find_ie(WLAN_EID_RSN, ies->data, ies->len);
+	rsn_ie = cfg80211_find_ie(WLAN_EID_RSN, ies_data, ies_len);
  	if (rsn_ie) {
  		int rsn_ie_len = sizeof(struct element) + rsn_ie[1];
  		int offset = 8;
@@ -517,6 +527,7 @@ wilc_parse_join_bss_param(struct cfg80211_bss *bss,
  			param->akm_suites[i] = crypto->akm_suites[i] & 0xFF;
  	}
+ kfree(ies_data);
  	return (void *)param;
  }





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux