[PATCH v2] wifi: mac80211: fix cfg80211_bss always hold when assoc response fail for MLO connection

[Wen Gong]

On 8/24/2023 3:03 PM, Johannes Berg wrote:
> On Thu, 2023-08-24 at 10:26 +0800, Wen Gong wrote:
>> On 8/23/2023 2:34 PM, Johannes Berg wrote:
>>> On Wed, 2023-08-23 at 10:28 +0800, Wen Gong wrote:
>>>>>                    /* need to have local link addresses for MLO connections */
>>>>>
>>>>>                    WARN_ON(cr.ap_mld_addr && !cr.links[link_id].addr);
>>>>>
>>>>> makes no sense anymore. Not sure if that's the only one.
>>>> After this patch, the cr.links[link_id].addr will be a valid local link
>>>> address from
>>>>
>>>> struct cfg80211_rx_assoc_resp, so I think it is not needed remove now.
>>> You don't understand.
>>>
>>> The issue is that it's set the line above.
>>>
>>>>                   cr.links[link_id].addr = data->links[link_id].addr;
>>>>                   /* need to have local link addresses for MLO connections */
>>>>                   WARN_ON(cr.ap_mld_addr && !cr.links[link_id].addr);
>>> But look at that! What values can cr.links[link_id].addr get? Note how
>>> it's a pointer - assigned from an array.
>> Oh, I know it now. the cr.links[link_id].addr will always NOT 0 because
>> it is pointer
>>
>> to an array "u8 addr[ETH_ALEN]" of struct cfg80211_rx_assoc_resp.
> Yep.
>
>> So maybe we can choose one of the 2 things to do:
>>
>> 1. remove the "WARN_ON(cr.ap_mld_addr && !cr.links[link_id].addr);"
>>
>> 2. change like this:
>>
>> WARN_ON(cr.ap_mld_addr && !is_valid_ether_addr(cr.links[link_id].addr));
>>
> I think we should check that it's valid, because if you don't fill it,
> it'll (hopefully) point to zeroed data somewhere.
>
> johannes
OK. So I will change it in next version.