Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4e602611-aed5-dfe7-6ce7-42d1fc7ca53e@infineon.com>
Date:   Wed, 28 Sep 2022 18:07:54 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH v3 4/5] brcmfmac: Update SSID of hidden AP while informing
 its bss to cfg80211 layer
Content-Language: en-US
To:     Kalle Valo <kvalo@kernel.org>
CC:     <linux-wireless@vger.kernel.org>,
        <brcm80211-dev-list@broadcom.com>,
        <brcm80211-dev-list@cypress.com>, <franky.lin@broadcom.com>,
        <hante.meuleman@broadcom.com>, <Double.Lo@infineon.com>
References: <20220927034138.20463-1-ian.lin@infineon.com>
 <20220927034138.20463-5-ian.lin@infineon.com> <874jwsrojr.fsf@kernel.org>
From:   "Lin Ian (CSSITB CSS ICW SW WFS / EE)" <ian.lin@infineon.com>
In-Reply-To: <874jwsrojr.fsf@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 9/28/2022 2:38 PM, Kalle Valo wrote:
> Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safe<https://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx>.
>
>
>
> Ian Lin <ian.lin@infineon.com> writes:
>
>> From: Syed Rafiuddeen <syed.rafiuddeen@cypress.com>
>>
>> cfg80211 layer on DUT STA is disconnecting ongoing connection attempt after
>> receiving association response, because cfg80211 layer does not have valid
>> AP bss information. On association response event, brcmfmac communicates
>> the AP bss information to cfg80211 layer, but SSID seem to be empty in AP
>> bss information, and cfg80211 layer prints kernel warning and then
>> disconnects the ongoing connection attempt.
>>
>> SSID is empty in SSID IE, but 'bi->SSID' contains a valid SSID, so
>> updating the SSID for hidden AP while informing its bss information
>> to cfg80211 layer.
>>
>> Signed-off-by: Syed Rafiuddeen <syed.rafiuddeen@cypress.com>
>> Signed-off-by: Chung-Hsien Hsu <chung-hsien.hsu@infineon.com>
>> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@infineon.com>
>> Signed-off-by: Ian Lin <ian.lin@infineon.com>
>> ---
>>   .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c   | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index 6c37da42e61b..3560afe0ccfe 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -3003,6 +3003,7 @@ static s32 brcmf_inform_single_bss(struct brcmf_cfg80211_info *cfg,
>>        u8 *notify_ie;
>>        size_t notify_ielen;
>>        struct cfg80211_inform_bss bss_data = {};
>> +     struct brcmf_tlv *ssid;
>>
>>        if (le32_to_cpu(bi->length) > WL_BSS_INFO_MAX) {
>>                bphy_err(drvr, "Bss info is larger than buffer. Discarding\n");
>> @@ -3032,6 +3033,13 @@ static s32 brcmf_inform_single_bss(struct brcmf_cfg80211_info *cfg,
>>        notify_ielen = le32_to_cpu(bi->ie_length);
>>        bss_data.signal = (s16)le16_to_cpu(bi->RSSI) * 100;
>>
>> +     ssid = (struct brcmf_tlv *)
>> +             brcmf_parse_tlvs(notify_ie, notify_ielen, WLAN_EID_SSID);
> This still casts away the const. For some reason brcmf_parse_tlvs()
> takes a const buffer:
>
> static const struct brcmf_tlv *brcmf_parse_tlvs(const void *buf, int buflen, uint key)
>
> I'm not familiar with brcmfmac internal so I don't know why it does
> that, but that means the buffer cannot be modified. If you need to
> modify the ssid should you make a copy of it?

In brcmf_parse_tlvs(const void *buf, int buflen, uint key),
it will find the key in buf and return the address of this key, as the 
return pointer.
This function don't intend caller to modify content of buf in most 
cases, so it defines a const return pointer.
But in this case, it just need to do it, so I need the typecast here.

> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches