2019-11-27 03:22:54

by Wen Gong

[permalink] [raw]
Subject: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash

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 <[email protected]>
---
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


2019-11-29 07:46:04

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash

Wen Gong <[email protected]> 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 <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

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

2019-12-02 04:48:14

by Justin Capella

[permalink] [raw]
Subject: Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash

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 <[email protected]> wrote:
>
> Wen Gong <[email protected]> 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 <[email protected]>
> > Signed-off-by: Kalle Valo <[email protected]>
>
> 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
>

2019-12-02 18:22:00

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash

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 <[email protected]> wrote:
>>
>> Wen Gong <[email protected]> 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 <[email protected]>
>>> Signed-off-by: Kalle Valo <[email protected]>
>>
>> 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 <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2019-12-14 16:02:02

by Justin Capella

[permalink] [raw]
Subject: Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash

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 <[email protected]> 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 <[email protected]> wrote:
> >>
> >> Wen Gong <[email protected]> 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 <[email protected]>
> >>> Signed-off-by: Kalle Valo <[email protected]>
> >>
> >> 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 <[email protected]>
> Candela Technologies Inc http://www.candelatech.com
>