All of lore.kernel.org
 help / color / mirror / Atom feed
From: Justin Capella <justincapella@gmail.com>
To: Ben Greear <greearb@candelatech.com>, Wen Gong <wgong@codeaurora.org>
Cc: ath10k <ath10k@lists.infradead.org>, linux-wireless@vger.kernel.org
Subject: Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
Date: Sat, 14 Dec 2019 08:01:33 -0800	[thread overview]
Message-ID: <CAMrEMU_2D9KzPudqVEMv-JS73JZD=hrmtf4drk41Hd1zOqS2dw@mail.gmail.com> (raw)
In-Reply-To: <b5404ac0-1be1-229f-a9e3-8033cdf7eea9@candelatech.com>

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
>

WARNING: multiple messages have this Message-ID (diff)
From: Justin Capella <justincapella@gmail.com>
To: Ben Greear <greearb@candelatech.com>, Wen Gong <wgong@codeaurora.org>
Cc: linux-wireless@vger.kernel.org, ath10k <ath10k@lists.infradead.org>
Subject: Re: [PATCH] ath10k: set WMI_PEER_AUTHORIZE after a firmware crash
Date: Sat, 14 Dec 2019 08:01:33 -0800	[thread overview]
Message-ID: <CAMrEMU_2D9KzPudqVEMv-JS73JZD=hrmtf4drk41Hd1zOqS2dw@mail.gmail.com> (raw)
In-Reply-To: <b5404ac0-1be1-229f-a9e3-8033cdf7eea9@candelatech.com>

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
>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

  reply	other threads:[~2019-12-14 16:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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-11-29  7:43 ` Kalle Valo
2019-12-02  4:45   ` Justin Capella
2019-12-02  4:45     ` Justin Capella
2019-12-02 18:17     ` Ben Greear
2019-12-02 18:17       ` Ben Greear
2019-12-14 16:01       ` Justin Capella [this message]
2019-12-14 16:01         ` Justin Capella
2019-11-27  3:19 Wen Gong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMrEMU_2D9KzPudqVEMv-JS73JZD=hrmtf4drk41Hd1zOqS2dw@mail.gmail.com' \
    --to=justincapella@gmail.com \
    --cc=ath10k@lists.infradead.org \
    --cc=greearb@candelatech.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=wgong@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.