Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
From: Alexander Wetzel <alexander@wetzel-home.de>
To: Denis Kenzior <denkenz@gmail.com>,
	Kalle Valo <kvalo@codeaurora.org>,
	Maya Erez <merez@codeaurora.org>
Cc: Ahmad Masri <amasri@codeaurora.org>,
	linux-wireless@vger.kernel.org, wil6210@qti.qualcomm.com
Subject: Re: [PATCH 04/11] wil6210: fix PTK re-key race
Date: Thu, 12 Sep 2019 23:04:14 +0200
Message-ID: <5716f475-856e-7fd2-637b-67927f4f78bc@wetzel-home.de> (raw)
In-Reply-To: <2c6bc637-62c2-020c-ab83-d2e1677d96b2@gmail.com>

Hi Denis,

>> I don't know anything about the driver here but in mac80211 the idea 
>> to avoid the race is to simply flush the queues prior deleting the 
>> outgoing key.
> 
> Maybe a silly question, but what does flushing the queue mean in this 
> context?  Is it waiting for all the packets to be sent or dropping them 
> on the floor?
> 

It's stopping them to make sure nothing can be added and then sends out 
all MPDUs in the queues.

>>
>> Now wpa_supplicant is not yet bypassing qdisks, but adding the socket 
>> parameter PACKET_QDISC_BYPASS is basically a one-liner in 
>> wpa_supplicant and should allow a generic way for drivers to avoid the 
>> race with a simple queue flush...
> 
> Can you expand on this actually?  What would the sequence of events be?
> 

1) wpa_supplicant hands over eapol #4 to the kernel.
    When bypassing the QDISC the frame is directly added to a driver
    queue or directly send out. When the send call returns the driver
    has eapol 4 either in the queuem already send it or the send command
    has failed.

2) wpa_supplicant deletes the old key (NL80211_CMD_DEL_KEY)

3) The driver stops all hw queues and sends out all MPDUs queued up to
    that time

4) Driver makes sure no traffic can be send with no/wrong key or PN to
    STA

5) the driver really removes the key from the HW/installs the new and
    resumes normal operation

I've just posted my hostpad patch to use PACKET_QDISC_BYPASS for eapol 
frames; It's probably too optimistic and need more code to retry a 
transmit to compensate for the missing QDISC buffers.

> Also, how would this be made to work with CONTROL_PORT over NL80211 ?
> 

Control port is an optional feature drivers can provide. wpa_supplicant 
should just use it when available or fall back to the "traditional" path 
when not. Now the driver don't has to flush all queues when using 
control port, as long as it makes sure the control port frame will be 
send out prior to deleting the key.

But then the driver must know that eapol frames will really be handed 
over via control port; So I guess flushing all queues is still the 
simpler solution. So I guess it will change next to nothing...

Alexander


  reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-08  8:32 [PATCH 00/11] wil6210 patches Maya Erez
2019-09-08  8:32 ` [PATCH 01/11] wil6210: add wil_netif_rx() helper function Maya Erez
2019-09-12 15:08   ` Kalle Valo
2019-09-08  8:32 ` [PATCH 02/11] wil6210: add support for pci linkdown recovery Maya Erez
2019-09-12 15:22   ` Kalle Valo
2019-09-08  8:32 ` [PATCH 03/11] wil6210: add debugfs to show PMC ring content Maya Erez
2019-09-08  8:32 ` [PATCH 04/11] wil6210: fix PTK re-key race Maya Erez
2019-09-10 13:23   ` Kalle Valo
2019-09-11  7:50     ` Arend Van Spriel
2019-09-11 18:32     ` Alexander Wetzel
2019-09-12 17:39       ` Denis Kenzior
2019-09-12 21:04         ` Alexander Wetzel [this message]
2019-09-13  8:04           ` Arend Van Spriel
2019-09-13 14:33             ` Denis Kenzior
2019-09-13 20:48               ` Alexander Wetzel
2019-09-17 15:32                 ` Denis Kenzior
2019-09-13 18:43             ` Alexander Wetzel
2019-09-08  8:32 ` [PATCH 05/11] wil6210: make sure DR bit is read before rest of the status message Maya Erez
2019-09-08  8:32 ` [PATCH 06/11] wil6210: verify cid value is valid Maya Erez
2019-09-08  8:32 ` [PATCH 07/11] wil6210: properly initialize discovery_expired_work Maya Erez
2019-09-08  8:32 ` [PATCH 08/11] wil6210: report boottime_ns in scan results Maya Erez
2019-09-08  8:32 ` [PATCH 09/11] wil6210: use writel_relaxed in wil_debugfs_iomem_x32_set Maya Erez
2019-09-08  8:32 ` [PATCH 10/11] wil6210: fix RX short frame check Maya Erez
2019-09-08  8:32 ` [PATCH 11/11] wil6210: ignore reset errors for FW during probe Maya Erez

Reply instructions:

You may reply publically 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=5716f475-856e-7fd2-637b-67927f4f78bc@wetzel-home.de \
    --to=alexander@wetzel-home.de \
    --cc=amasri@codeaurora.org \
    --cc=denkenz@gmail.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=merez@codeaurora.org \
    --cc=wil6210@qti.qualcomm.com \
    /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

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