Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 04/11] wil6210: fix PTK re-key race
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
References: <1567931575-27984-5-git-send-email-merez@codeaurora.org>
 <20190910132315.D7AC7602F2@smtp.codeaurora.org>
 <7b636313-fa4a-5ee4-935a-ba2ed5dde1e5@wetzel-home.de>
 <2c6bc637-62c2-020c-ab83-d2e1677d96b2@gmail.com>
From:   Alexander Wetzel <alexander@wetzel-home.de>
Message-ID: <5716f475-856e-7fd2-637b-67927f4f78bc@wetzel-home.de>
Date:   Thu, 12 Sep 2019 23:04:14 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.0
MIME-Version: 1.0
In-Reply-To: <2c6bc637-62c2-020c-ab83-d2e1677d96b2@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

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