On 6/1/21 11:02 AM, Fuzzey, Martin wrote:
Hi Marek,
thanks for the review.
Hi,
On Fri, 28 May 2021 at 20:11, Marek Vasut <marex@xxxxxxx> wrote:
Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
CC: stable@xxxxxxxxxxxxxxx
This likely needs a Fixes: tag ?
I'm not quite sure what that should be.
Based on git log -p --follow drivers/net/wireless/rsi/rsi_91x_hal.c
Fixes: dad0d04fa7ba ("rsi: Add RS9113 wireless driver")
The test involved here has been present since the very first version
of the driver back in 2014 but at that time AP mode wasn't supported.
AP mode was added in 2017 by the patch series "rsi: support for AP mode" [1]
In particular 38ef62353acb ("rsi: security enhancements for AP mode")
does some stuff relating to AP key configuration but it doesn't
actually change the behaviour concerning the encryption condition.
In fact I don't understand how it ever worked in AP WPA2 mode given
that secinfo->security_enable (which is tested in the encryption
condition) has always been unconditionally set in set_key (when
setting not deleting).
Yet the series cover letter [1] says "Tests are performed to confirm
aggregation, connections in WEP and WPA/WPA2 security."
There are way too many bugs in that RSI driver, yes. Compared to the
other WiFi vendors, this driver seems to be real poor.
The problem is that in AP mode with WPA2 there is a set_key done at AP
startup time to set the GTK (but not yet the pairwise key which is
only done after the 4 way handshake) so this sets security_enable to
true which later causes the EAPOL messages to be sent encrypted.
Maybe there have been userspace changes to hostapd that have changed
the time at which the GTK is set?
I had a quick look at the hostapd history but didn't see anything obvious.
I recall seeing a patched WPA supplicant in one set of the RSI
downstream drivers. And then there is another older RSI "onebox" driver
which definitely ships patched userspace components. So I wouldn't be
surprised if some of that was involved.
I'm going to send a V2 completely removing the security_enable flag in
addition to adding the new test (which is what downstream has too).
Keeping security_enable doesn't actually break anything but is
redundant and confusing.
Unfortunately I cannot find any downstream history, I just have 2
downstream tarballs, a "2.0 RC2" which has the same problem as
mainline and a "2.0 RC4" which does not
Yes indeed, I pointed that out to RSI before, that this kind of release
management is useless. I'm still waiting for any input from them.
[1] https://www.spinics.net/lists/linux-wireless/msg165302.html
if ((!(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) &&
+ (info->control.hw_key) &&
The () are not needed.
Ok, will fix for V2
Thanks
