[PATCH] mac80211: fix low throughput due to invalid addba extension

[PATCH] mac80211: fix low throughput due to invalid addba extension

[Govindaraj]

Addba request action frame received with the extension element from
certain 11ac stations, but the cmd id and length not matching to addba
extension and it failing in element parsing. Due to this, addba request
not acknowledged and aggregation not started which is causing low
throughput. Hence validating the cmd id before processing addba extension.

Signed-off-by: Govindaraj <gsamin@codeaurora.org>
---
 net/mac80211/agg-rx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index cce28e3..ea473d7 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -494,7 +494,8 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,
 
 	ies_len = len - offsetof(struct ieee80211_mgmt,
 				 u.action.u.addba_req.variable);
-	if (ies_len) {
+	if (ies_len &&
+	    mgmt->u.action.u.addba_req.variable[0] == WLAN_EID_ADDBA_EXT) {
 		ieee802_11_parse_elems(mgmt->u.action.u.addba_req.variable,
                                 ies_len, true, &elems, mgmt->bssid, NULL);
 		if (elems.parse_error)
-- 
2.7.4

Re: [PATCH] mac80211: fix low throughput due to invalid addba extension

[Johannes Berg]

On Tue, 2021-03-16 at 21:17 +0530, Govindaraj wrote:
> Addba request action frame received with the extension element from
> certain 11ac stations,
> 

Please indicate which so we have a record of who's shipping broken junk.

> but the cmd id and length not matching to addba
> extension and it failing in element parsing.

> Due to this, addba request
> not acknowledged and aggregation not started which is causing low
> throughput. Hence validating the cmd id before processing addba extension.

>  	ies_len = len - offsetof(struct ieee80211_mgmt,
>  				 u.action.u.addba_req.variable);
> -	if (ies_len) {
> +	if (ies_len &&
> +	    mgmt->u.action.u.addba_req.variable[0] == WLAN_EID_ADDBA_EXT) {
>  		ieee802_11_parse_elems(mgmt->u.action.u.addba_req.variable,
>                                  ies_len, true, &elems, mgmt->bssid, NULL);
>  		if (elems.parse_error)

So we get into parse_error without this?

What are they putting there instead?

johannes

Re: [PATCH] mac80211: fix low throughput due to invalid addba extension

[Govindaraj Saminathan]

On 2021-03-16 21:21, Johannes Berg wrote:
> On Tue, 2021-03-16 at 21:17 +0530, Govindaraj wrote:
>> Addba request action frame received with the extension element from
>> certain 11ac stations,
>> 
> 
> Please indicate which so we have a record of who's shipping broken 
> junk.
> 
>> but the cmd id and length not matching to addba
>> extension and it failing in element parsing.
> 
>> Due to this, addba request
>> not acknowledged and aggregation not started which is causing low
>> throughput. Hence validating the cmd id before processing addba 
>> extension.
> 
>>  	ies_len = len - offsetof(struct ieee80211_mgmt,
>>  				 u.action.u.addba_req.variable);
>> -	if (ies_len) {
>> +	if (ies_len &&
>> +	    mgmt->u.action.u.addba_req.variable[0] == WLAN_EID_ADDBA_EXT) {
>>  		ieee802_11_parse_elems(mgmt->u.action.u.addba_req.variable,
>>                                  ies_len, true, &elems, mgmt->bssid, 
>> NULL);
>>  		if (elems.parse_error)
> 
> So we get into parse_error without this?
yes, we getting parse error.
> 
> What are they putting there instead?
first 9 bytes are addba request action frame and remaining 7 bytes 
extension causing for parse error.
03 00 01 02 10 00 00 e0 0a cf 08 06 11a 46 09 fe
> 
> johannes

Re: [PATCH] mac80211: fix low throughput due to invalid addba extension

[Govindaraj Saminathan]

On 2021-03-16 23:15, Govindaraj Saminathan wrote:
> On 2021-03-16 21:21, Johannes Berg wrote:
>> On Tue, 2021-03-16 at 21:17 +0530, Govindaraj wrote:
>>> Addba request action frame received with the extension element from
>>> certain 11ac stations,
>>> 
>> 
>> Please indicate which so we have a record of who's shipping broken 
>> junk.
The below log i taken with pixel3 client device
>> 
>>> but the cmd id and length not matching to addba
>>> extension and it failing in element parsing.
>> 
>>> Due to this, addba request
>>> not acknowledged and aggregation not started which is causing low
>>> throughput. Hence validating the cmd id before processing addba 
>>> extension.
>> 
>>>  	ies_len = len - offsetof(struct ieee80211_mgmt,
>>>  				 u.action.u.addba_req.variable);
>>> -	if (ies_len) {
>>> +	if (ies_len &&
>>> +	    mgmt->u.action.u.addba_req.variable[0] == WLAN_EID_ADDBA_EXT) {
>>>  		ieee802_11_parse_elems(mgmt->u.action.u.addba_req.variable,
>>>                                  ies_len, true, &elems, mgmt->bssid, 
>>> NULL);
>>>  		if (elems.parse_error)
>> 
>> So we get into parse_error without this?
> yes, we getting parse error.
>> 
>> What are they putting there instead?
> first 9 bytes are addba request action frame and remaining 7 bytes
> extension causing for parse error.
> 03 00 01 02 10 00 00 e0 0a cf 08 06 11a 46 09 fe
>> 
>> johannes