netdev - RE: [PATCH 3/9] rtw88: Move rtw_update_sta_info() out of rtw_ra_mask_info_update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Jan 2022 08:42:11 +0000
From:   Pkshih <pkshih@...ltek.com>
To:     Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>
CC:     "tony0620emma@...il.com" <tony0620emma@...il.com>,
        "kvalo@...eaurora.org" <kvalo@...eaurora.org>,
        "johannes@...solutions.net" <johannes@...solutions.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Neo Jou <neojou@...il.com>,
        Jernej Skrabec <jernej.skrabec@...il.com>
Subject: RE: [PATCH 3/9] rtw88: Move rtw_update_sta_info() out of rtw_ra_mask_info_update_iter()


> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
> Sent: Wednesday, December 29, 2021 5:15 AM
> To: linux-wireless@...r.kernel.org
> Cc: tony0620emma@...il.com; kvalo@...eaurora.org; johannes@...solutions.net; netdev@...r.kernel.org;
> linux-kernel@...r.kernel.org; Neo Jou <neojou@...il.com>; Jernej Skrabec <jernej.skrabec@...il.com>;
> Pkshih <pkshih@...ltek.com>; Martin Blumenstingl <martin.blumenstingl@...glemail.com>
> Subject: [PATCH 3/9] rtw88: Move rtw_update_sta_info() out of rtw_ra_mask_info_update_iter()
> 
> rtw_update_sta_info() internally access some registers while being
> called unter an atomic lock acquired by rtw_iterate_vifs_atomic(). Move
> rtw_update_sta_info() call out of (rtw_ra_mask_info_update_iter) in
> preparation for SDIO support where register access may sleep.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
> ---
> v1 -> v2:
> - this patch is new in v2
> - keep rtw_iterate_vifs_atomic() to prevent deadlocks as Johannes
>   suggested. Keep track of all relevant stations inside
>   rtw_ra_mask_info_update_iter() and the iter-data and then call
>   rtw_update_sta_info() while held under rtwdev->mutex instead
> 
>  drivers/net/wireless/realtek/rtw88/mac80211.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c
> b/drivers/net/wireless/realtek/rtw88/mac80211.c
> index ae7d97de5fdf..3bd12354a8a1 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac80211.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac80211.c

[...]

> @@ -699,11 +702,20 @@ static void rtw_ra_mask_info_update(struct rtw_dev *rtwdev,
>  				    const struct cfg80211_bitrate_mask *mask)
>  {
>  	struct rtw_iter_bitrate_mask_data br_data;
> +	unsigned int i;
> +
> +	mutex_lock(&rtwdev->mutex);

I think this lock is used to protect br_data.si[i], right?

And, I prefer to move mutex lock to caller, like:

@@ -734,7 +734,9 @@ static int rtw_ops_set_bitrate_mask(struct ieee80211_hw *hw,
 {
        struct rtw_dev *rtwdev = hw->priv;

+       mutex_lock(&rtwdev->mutex);
        rtw_ra_mask_info_update(rtwdev, vif, mask);
+       mutex_unlock(&rtwdev->mutex);

        return 0;
 }

> 
>  	br_data.rtwdev = rtwdev;
>  	br_data.vif = vif;
>  	br_data.mask = mask;
> +	br_data.num_si = 0;
>  	rtw_iterate_stas_atomic(rtwdev, rtw_ra_mask_info_update_iter, &br_data);
> +
> +	for (i = 0; i < br_data.num_si; i++)
> +		rtw_update_sta_info(rtwdev, br_data.si[i]);
> +
> +	mutex_unlock(&rtwdev->mutex);
>  }
> 

--
Ping-Ke