linux-kernel - Re: [PATCH 02/31] staging: wfx: do not send CAB while scanning

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 10 Sep 2021 19:31:00 +0300
From:   Kari Argillander <kari.argillander@...il.com>
To:     Jerome Pouiller <Jerome.Pouiller@...abs.com>
Cc:     devel@...verdev.osuosl.org, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH 02/31] staging: wfx: do not send CAB while scanning

On Fri, Sep 10, 2021 at 06:04:35PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@...abs.com>
> 
> During the scan requests, the Tx traffic is suspended. This lock is
> shared by all the network interfaces. So, a scan request on one
> interface will block the traffic on a second interface. This causes
> trouble when the queued traffic contains CAB (Content After DTIM Beacon)
> since this traffic cannot be delayed.
> 
> It could be possible to make the lock local to each interface. But It
> would only push the problem further. The device won't be able to send
> the CAB before the end of the scan.
> 
> So, this patch just ignore the DTIM indication when a scan is in
> progress. The firmware will send another indication on the next DTIM and
> this time the system will be able to send the traffic just behind the
> beacon.
> 
> The only drawback of this solution is that the stations connected to
> the AP will wait for traffic after the DTIM for nothing. But since the
> case is really rare it is not a big deal.
> 
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@...abs.com>
> ---
>  drivers/staging/wfx/sta.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> index a236e5bb6914..d901588237a4 100644
> --- a/drivers/staging/wfx/sta.c
> +++ b/drivers/staging/wfx/sta.c
> @@ -629,8 +629,18 @@ int wfx_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta, bool set)
>  
>  void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd notify_cmd)
>  {
> +	struct wfx_vif *wvif_it;
> +
>  	if (notify_cmd != STA_NOTIFY_AWAKE)
>  		return;
> +
> +	// Device won't be able to honor CAB if a scan is in progress on any
> +	// interface. Prefer to skip this DTIM and wait for the next one.

In one patch you drop // comments but you introduce some of your self.

> +	wvif_it = NULL;
> +	while ((wvif_it = wvif_iterate(wvif->wdev, wvif_it)) != NULL)
> +		if (mutex_is_locked(&wvif_it->scan_lock))
> +			return;
> +
>  	if (!wfx_tx_queues_has_cab(wvif) || wvif->after_dtim_tx_allowed)
>  		dev_warn(wvif->wdev->dev, "incorrect sequence (%d CAB in queue)",
>  			 wfx_tx_queues_has_cab(wvif));
> -- 
> 2.33.0
>