Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
@ 2019-11-27  3:19 Wen Gong
  2019-11-29  7:43 ` Kalle Valo
  0 siblings, 1 reply; 5+ messages in thread
From: Wen Gong @ 2019-11-27  3:19 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless

After the firmware crashes ath10k recovers via ieee80211_reconfig(),
which eventually leads to firmware configuration and including the
encryption keys. However, because there is no new auth/assoc and
4-way-handshake, and firmware set the authorize flag after
4-way-handshake, so the authorize flag in firmware is not set in
firmware without 4-way-handshake. This will lead to a failure of data
transmission after recovery done when using encrypted connections like
WPA-PSK. Set authorize flag after installing keys to firmware will fix
the issue.

This was noticed by testing firmware crashing using simulate_fw_crash
debugfs file.

Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/mac.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index ae84f8e4e7dd..0829582c0d50 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -6329,6 +6329,9 @@ static int ath10k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	if (sta && sta->tdls)
 		ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
 					  ar->wmi.peer_param->authorize, 1);
+	else if (sta && cmd == SET_KEY && (key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
+		ath10k_wmi_peer_set_param(ar, arvif->vdev_id, peer_addr,
+					  ar->wmi.peer_param->authorize, 1);
 
 exit:
 	mutex_unlock(&ar->conf_mutex);
-- 
2.23.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
  2019-11-27  3:19 [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash Wen Gong
@ 2019-11-29  7:43 ` Kalle Valo
  2019-12-02  4:45   ` Justin Capella
  0 siblings, 1 reply; 5+ messages in thread
From: Kalle Valo @ 2019-11-29  7:43 UTC (permalink / raw)
  To: Wen Gong; +Cc: ath10k, linux-wireless

Wen Gong <wgong@codeaurora.org> wrote:

> After the firmware crashes ath10k recovers via ieee80211_reconfig(),
> which eventually leads to firmware configuration and including the
> encryption keys. However, because there is no new auth/assoc and
> 4-way-handshake, and firmware set the authorize flag after
> 4-way-handshake, so the authorize flag in firmware is not set in
> firmware without 4-way-handshake. This will lead to a failure of data
> transmission after recovery done when using encrypted connections like
> WPA-PSK. Set authorize flag after installing keys to firmware will fix
> the issue.
> 
> This was noticed by testing firmware crashing using simulate_fw_crash
> debugfs file.
> 
> Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.
> 
> Signed-off-by: Wen Gong <wgong@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

382e51c139ef ath10k: set WMI_PEER_AUTHORIZE after a firmware crash

-- 
https://patchwork.kernel.org/patch/11263357/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
  2019-11-29  7:43 ` Kalle Valo
@ 2019-12-02  4:45   ` Justin Capella
  2019-12-02 18:17     ` Ben Greear
  0 siblings, 1 reply; 5+ messages in thread
From: Justin Capella @ 2019-12-02  4:45 UTC (permalink / raw)
  To: Kalle Valo; +Cc: Wen Gong, ath10k, linux-wireless

Are there security concerns here? Was the peer known to be authorized
beforehand? Would it be better to just trash the peer in the event of
a fw crash?

On Thu, Nov 28, 2019 at 11:46 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Wen Gong <wgong@codeaurora.org> wrote:
>
> > After the firmware crashes ath10k recovers via ieee80211_reconfig(),
> > which eventually leads to firmware configuration and including the
> > encryption keys. However, because there is no new auth/assoc and
> > 4-way-handshake, and firmware set the authorize flag after
> > 4-way-handshake, so the authorize flag in firmware is not set in
> > firmware without 4-way-handshake. This will lead to a failure of data
> > transmission after recovery done when using encrypted connections like
> > WPA-PSK. Set authorize flag after installing keys to firmware will fix
> > the issue.
> >
> > This was noticed by testing firmware crashing using simulate_fw_crash
> > debugfs file.
> >
> > Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.
> >
> > Signed-off-by: Wen Gong <wgong@codeaurora.org>
> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
>
> Patch applied to ath-next branch of ath.git, thanks.
>
> 382e51c139ef ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
>
> --
> https://patchwork.kernel.org/patch/11263357/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
  2019-12-02  4:45   ` Justin Capella
@ 2019-12-02 18:17     ` Ben Greear
  2019-12-14 16:01       ` Justin Capella
  0 siblings, 1 reply; 5+ messages in thread
From: Ben Greear @ 2019-12-02 18:17 UTC (permalink / raw)
  To: Justin Capella, Kalle Valo; +Cc: Wen Gong, ath10k, linux-wireless

On 12/1/19 8:45 PM, Justin Capella wrote:
> Are there security concerns here? Was the peer known to be authorized
> beforehand? Would it be better to just trash the peer in the event of
> a fw crash?

I think you should completely re-associate the peer(s) when firmware
crashes.  The driver does not cache all possible changes, so it cannot
exactly rebuild the config to the previous state.

Thanks,
Ben

> 
> On Thu, Nov 28, 2019 at 11:46 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>> Wen Gong <wgong@codeaurora.org> wrote:
>>
>>> After the firmware crashes ath10k recovers via ieee80211_reconfig(),
>>> which eventually leads to firmware configuration and including the
>>> encryption keys. However, because there is no new auth/assoc and
>>> 4-way-handshake, and firmware set the authorize flag after
>>> 4-way-handshake, so the authorize flag in firmware is not set in
>>> firmware without 4-way-handshake. This will lead to a failure of data
>>> transmission after recovery done when using encrypted connections like
>>> WPA-PSK. Set authorize flag after installing keys to firmware will fix
>>> the issue.
>>>
>>> This was noticed by testing firmware crashing using simulate_fw_crash
>>> debugfs file.
>>>
>>> Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.
>>>
>>> Signed-off-by: Wen Gong <wgong@codeaurora.org>
>>> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
>>
>> Patch applied to ath-next branch of ath.git, thanks.
>>
>> 382e51c139ef ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
>>
>> --
>> https://patchwork.kernel.org/patch/11263357/
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>>
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
  2019-12-02 18:17     ` Ben Greear
@ 2019-12-14 16:01       ` Justin Capella
  0 siblings, 0 replies; 5+ messages in thread
From: Justin Capella @ 2019-12-14 16:01 UTC (permalink / raw)
  To: Ben Greear, Wen Gong; +Cc: ath10k, linux-wireless

If you have time to spare I'd be interested in hearing a little more
about your stances on this... I'm trying to learn more about this
stuff and not at all qualified to say one way or the other if it is a
good idea, but my intuition is this is going to lead to inconsistent
state/behaviors. I have been wondering if maybe this change may be
related to some of the fw crash reports coming in--- perhaps marking
the station as authorized before the fw is fully started and/or the
device is present

On Mon, Dec 2, 2019 at 10:17 AM Ben Greear <greearb@candelatech.com> wrote:
>
> On 12/1/19 8:45 PM, Justin Capella wrote:
> > Are there security concerns here? Was the peer known to be authorized
> > beforehand? Would it be better to just trash the peer in the event of
> > a fw crash?
>
> I think you should completely re-associate the peer(s) when firmware
> crashes.  The driver does not cache all possible changes, so it cannot
> exactly rebuild the config to the previous state.
>
> Thanks,
> Ben
>
> >
> > On Thu, Nov 28, 2019 at 11:46 PM Kalle Valo <kvalo@codeaurora.org> wrote:
> >>
> >> Wen Gong <wgong@codeaurora.org> wrote:
> >>
> >>> After the firmware crashes ath10k recovers via ieee80211_reconfig(),
> >>> which eventually leads to firmware configuration and including the
> >>> encryption keys. However, because there is no new auth/assoc and
> >>> 4-way-handshake, and firmware set the authorize flag after
> >>> 4-way-handshake, so the authorize flag in firmware is not set in
> >>> firmware without 4-way-handshake. This will lead to a failure of data
> >>> transmission after recovery done when using encrypted connections like
> >>> WPA-PSK. Set authorize flag after installing keys to firmware will fix
> >>> the issue.
> >>>
> >>> This was noticed by testing firmware crashing using simulate_fw_crash
> >>> debugfs file.
> >>>
> >>> Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.
> >>>
> >>> Signed-off-by: Wen Gong <wgong@codeaurora.org>
> >>> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
> >>
> >> Patch applied to ath-next branch of ath.git, thanks.
> >>
> >> 382e51c139ef ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
> >>
> >> --
> >> https://patchwork.kernel.org/patch/11263357/
> >>
> >> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> >>
> >
>
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27  3:19 [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash Wen Gong
2019-11-29  7:43 ` Kalle Valo
2019-12-02  4:45   ` Justin Capella
2019-12-02 18:17     ` Ben Greear
2019-12-14 16:01       ` Justin Capella

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git