Received-SPF: pass (google.com: domain of linux-wireless+bounces-6340-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <1cbb57c2-3d8f-4932-9132-d46a871c944b@gmail.com>
Date: Mon, 15 Apr 2024 22:14:54 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
Subject: Re: [PATCH] wifi: rtl8xxxu: enable MFP support
To: Martin Kaistra <martin.kaistra@linutronix.de>,
 linux-wireless@vger.kernel.org
Cc: Jes Sorensen <Jes.Sorensen@gmail.com>, Kalle Valo <kvalo@kernel.org>,
 Ping-Ke Shih <pkshih@realtek.com>,
 Sebastian Andrzej Siewior <bigeasy@linutronix.de>
References: <20240314164850.86432-1-martin.kaistra@linutronix.de>
 <fa903f58-2362-49a1-9880-2b3fcbe1869e@gmail.com>
 <5b3af47c-7efb-4ca3-93bd-06c682dfc84f@linutronix.de>
Content-Language: en-US
In-Reply-To: <5b3af47c-7efb-4ca3-93bd-06c682dfc84f@linutronix.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 15/04/2024 09:49, Martin Kaistra wrote:
> Am 14.04.24 um 13:32 schrieb Bitterblue Smith:
>> On 14/03/2024 18:48, Martin Kaistra wrote:
>>> In order to connect to networks which require 802.11w, add the
>>> MFP_CAPABLE flag and let mac80211 do the actual crypto in software.
>>>
>>> When a robust management frames is received, rx_dec->swdec is not set,
>>> even though the HW did not decrypt it. Extend the check and don't set
>>> RX_FLAG_DECRYPTED for these frames in order to use SW decryption.
>>>
>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>>> ---
>>>   drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 +++++++--
>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> index 4a49f8f9d80f2..870bd952f5902 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> @@ -6473,7 +6473,9 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>>>               rx_status->mactime = rx_desc->tsfl;
>>>               rx_status->flag |= RX_FLAG_MACTIME_START;
>>>   -            if (!rx_desc->swdec)
>>> +            if (!rx_desc->swdec &&
>>> +                !(_ieee80211_is_robust_mgmt_frame(hdr) &&
>>> +                  ieee80211_has_protected(hdr->frame_control)))
>>>                   rx_status->flag |= RX_FLAG_DECRYPTED;
>>>               if (rx_desc->crc32)
>>>                   rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
>>> @@ -6578,7 +6580,9 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>>>               rx_status->mactime = rx_desc->tsfl;
>>>               rx_status->flag |= RX_FLAG_MACTIME_START;
>>>   -            if (!rx_desc->swdec)
>>> +            if (!rx_desc->swdec &&
>>> +                !(_ieee80211_is_robust_mgmt_frame(hdr) &&
>>> +                  ieee80211_has_protected(hdr->frame_control)))
>>>                   rx_status->flag |= RX_FLAG_DECRYPTED;
>>>               if (rx_desc->crc32)
>>>                   rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
>>> @@ -7998,6 +8002,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>>>       ieee80211_hw_set(hw, HAS_RATE_CONTROL);
>>>       ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
>>>       ieee80211_hw_set(hw, AMPDU_AGGREGATION);
>>> +    ieee80211_hw_set(hw, MFP_CAPABLE);
>>>         wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
>>>   
>>
>> I ran into this problem recently with rtl8192du:
>> https://lore.kernel.org/linux-wireless/ed12ec17-ae6e-45fa-a72f-23e0a34654da@gmail.com/
>>
>> Does the same fix work for you in rtl8xxxu? Checking the "security"
>> field of the RX descriptor is simpler than calling two functions.
>> Sorry to bother you when the patch is already applied.
> 
> Thanks for the hint. I tried to do something similar to what has been done in other rtlwifi drivers and missed the solution in rtw88, which is probably better:
> 
> rtlwifi/rtl8188ee/trx.c
> rtlwifi/rtl8192ce/trx.c
> rtlwifi/rtl8192ee/trx.c
> rtlwifi/rtl8192se/trx.c
> rtlwifi/rtl8723ae/trx.c
> rtlwifi/rtl8723be/trx.c
> rtlwifi/rtl8821ae/trx.c
> 
> Shouldn't it be changed in these locations as well?
> 
> I will do a test for rtl8xxxu and if it is successful send a new patch.
> 
>>
>> Also, won't you send the patch to the stable tree?
> 
> The rtl8xxxu driver previously did not have the MFP_CAPABLE flag set. As I am adding new functionality (support for WPA3), I don't think this should go to stable.

Without your patch I can't connect to my phone's hotspot
when it uses WPA3:

Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: nl80211: kernel reports: key setting validation failed
Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: WPA: Failed to configure IGTK to the driver
Apr 08 12:50:57 ideapad2 wpa_supplicant[1231]: wlp3s0f3u2: RSN: Failed to configure IGTK

It doesn't say anything about WPA3 or management frame
protection, just prints those unhelpful errors and tries
to connect over and over again. To me that looks more like
fixing a bug than adding new functionality. It's just sad
that people need to install kernel 6.10+ in order to support
WPA3, when the patch is so small.