* [PATCH 0/2] wifi: wilc1000: fix RCU warning in connect path @ 2024-01-04 14:39 Alexis Lothoré 2024-01-04 14:39 ` [PATCH 1/2] wifi: wilc1000: fix declarations ordering Alexis Lothoré 2024-01-04 14:39 ` [PATCH 2/2] wifi: wilc1000: fix RCU usage in connect path Alexis Lothoré 0 siblings, 2 replies; 5+ messages in thread From: Alexis Lothoré @ 2024-01-04 14:39 UTC (permalink / raw) To: linux-wireless Cc: Ajay Singh, Claudiu Beznea, Kalle Valo, Thomas Petazzoni, linux-kernel Hello, this small series aims to fix a RCU warning due to RCU pointer dereference outside of a RCU critical read section. Fix is in the second commit, the first one is a mere declarations reordering before touching declared variables in wilc_parse_join_bss_param Alexis Lothoré (2): wifi: wilc1000: fix declarations ordering wifi: wilc1000: fix RCU usage in connect path drivers/net/wireless/microchip/wilc1000/hif.c | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) -- 2.42.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] wifi: wilc1000: fix declarations ordering 2024-01-04 14:39 [PATCH 0/2] wifi: wilc1000: fix RCU warning in connect path Alexis Lothoré @ 2024-01-04 14:39 ` Alexis Lothoré 2024-01-04 14:39 ` [PATCH 2/2] wifi: wilc1000: fix RCU usage in connect path Alexis Lothoré 1 sibling, 0 replies; 5+ messages in thread From: Alexis Lothoré @ 2024-01-04 14:39 UTC (permalink / raw) To: linux-wireless Cc: Ajay Singh, Claudiu Beznea, Kalle Valo, Thomas Petazzoni, linux-kernel From: Alexis Lothoré <alexis.lothore@bootlin.com> Reorder parameters declaration in wilc_parse_join_bss_param to enforce reverse christmas tree Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com> --- drivers/net/wireless/microchip/wilc1000/hif.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/microchip/wilc1000/hif.c b/drivers/net/wireless/microchip/wilc1000/hif.c index 839f142663e8..6f1218a51061 100644 --- a/drivers/net/wireless/microchip/wilc1000/hif.c +++ b/drivers/net/wireless/microchip/wilc1000/hif.c @@ -377,13 +377,13 @@ struct wilc_join_bss_param * wilc_parse_join_bss_param(struct cfg80211_bss *bss, struct cfg80211_crypto_settings *crypto) { - struct wilc_join_bss_param *param; - struct ieee80211_p2p_noa_attr noa_attr; - u8 rates_len = 0; + const struct cfg80211_bss_ies *ies = rcu_dereference(bss->ies); const u8 *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; + struct wilc_join_bss_param *param; + u8 rates_len = 0; int ret; - const struct cfg80211_bss_ies *ies = rcu_dereference(bss->ies); param = kzalloc(sizeof(*param), GFP_KERNEL); if (!param) -- 2.42.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] wifi: wilc1000: fix RCU usage in connect path 2024-01-04 14:39 [PATCH 0/2] wifi: wilc1000: fix RCU warning in connect path Alexis Lothoré 2024-01-04 14:39 ` [PATCH 1/2] wifi: wilc1000: fix declarations ordering Alexis Lothoré @ 2024-01-04 14:39 ` Alexis Lothoré 2024-01-04 15:30 ` Christophe JAILLET 1 sibling, 1 reply; 5+ messages in thread From: Alexis Lothoré @ 2024-01-04 14:39 UTC (permalink / raw) To: linux-wireless Cc: Ajay Singh, Claudiu Beznea, Kalle Valo, Thomas Petazzoni, linux-kernel From: Alexis Lothoré <alexis.lothore@bootlin.com> 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@bootlin.com> --- 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); + if (!ies_data) { + rcu_read_unlock(); + 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; } -- 2.42.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] wifi: wilc1000: fix RCU usage in connect path 2024-01-04 14:39 ` [PATCH 2/2] wifi: wilc1000: fix RCU usage in connect path Alexis Lothoré @ 2024-01-04 15:30 ` Christophe JAILLET 2024-01-04 15:41 ` Alexis Lothoré 0 siblings, 1 reply; 5+ messages in thread From: Christophe JAILLET @ 2024-01-04 15:30 UTC (permalink / raw) To: alexis.lothore Cc: ajay.kathat, claudiu.beznea, kvalo, linux-kernel, linux-wireless, thomas.petazzoni Le 04/01/2024 à 15:39, Alexis Lothoré a écrit : > From: Alexis Lothoré <alexis.lothore-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> > > 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@public.gmane.org> > --- > 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; > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] wifi: wilc1000: fix RCU usage in connect path 2024-01-04 15:30 ` Christophe JAILLET @ 2024-01-04 15:41 ` Alexis Lothoré 0 siblings, 0 replies; 5+ messages in thread From: Alexis Lothoré @ 2024-01-04 15:41 UTC (permalink / raw) To: Christophe JAILLET Cc: ajay.kathat, claudiu.beznea, kvalo, linux-kernel, linux-wireless, thomas.petazzoni On 1/4/24 16:30, Christophe JAILLET wrote: > Le 04/01/2024 à 15:39, Alexis Lothoré a écrit : >> From: Alexis Lothoré <alexis.lothore-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> >> >> 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@public.gmane.org> >> --- >> 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]). From my understanding: yes, because if any sleep occurs in a section protected by rcu_read_lock/rcu_read_unlock, it may mess with RCU grace period wait (and so, rcu reclaim step may occur while this reader is still in the read section) > >> + if (!ies_data) { >> + rcu_read_unlock(); > > 'param' allocated just a few lines above is leaking now. You are right, good catch, thanks. I will update accordingly. > > 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; >> } >> > -- Alexis Lothoré, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-04 15:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-04 14:39 [PATCH 0/2] wifi: wilc1000: fix RCU warning in connect path Alexis Lothoré 2024-01-04 14:39 ` [PATCH 1/2] wifi: wilc1000: fix declarations ordering Alexis Lothoré 2024-01-04 14:39 ` [PATCH 2/2] wifi: wilc1000: fix RCU usage in connect path Alexis Lothoré 2024-01-04 15:30 ` Christophe JAILLET 2024-01-04 15:41 ` Alexis Lothoré
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®