| Message ID | 0101016eaadee57a-54500c6d-4751-423f-8bab-5acd8fad2175-000000@us-west-2.amazonses.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 382e51c139ef9d0b8e65ff5c249771f864b411fd |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath10k: set WMI_PEER_AUTHORIZE after a firmware crash | expand |
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
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 >
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 >> >
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 >
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);
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(+)