* [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
@ 2024-02-08 8:51 Alexey Berezhok
2024-02-08 14:06 ` Arend van Spriel
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Alexey Berezhok @ 2024-02-08 8:51 UTC (permalink / raw)
To: Arend van Spriel; +Cc: Kalle Valo, linux-wireless, lvc-project, Alexey Berezhok
In 'brcmf_cfg80211_start_ap()', not assume that
NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
an explicit check instead. Compile tested only.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 736b2ada6..63f6e9436 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5272,7 +5272,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
settings->hidden_ssid);
if (err) {
bphy_err(drvr, "%s closednet error (%d)\n",
- settings->hidden_ssid ?
+ (settings->hidden_ssid != NL80211_HIDDEN_SSID_NOT_IN_USE) ?
"enabled" : "disabled",
err);
goto exit;
--
2.39.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-08 8:51 [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean Alexey Berezhok
@ 2024-02-08 14:06 ` Arend van Spriel
2024-02-08 14:15 ` Johannes Berg
2024-02-12 15:38 ` Kalle Valo
2024-02-19 18:25 ` andy.shevchenko
2 siblings, 1 reply; 15+ messages in thread
From: Arend van Spriel @ 2024-02-08 14:06 UTC (permalink / raw)
To: Alexey Berezhok, Arend van Spriel, Johannes Berg
Cc: Kalle Valo, linux-wireless, lvc-project
[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]
On 2/8/2024 9:51 AM, Alexey Berezhok wrote:
> In 'brcmf_cfg80211_start_ap()', not assume that
> NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
> an explicit check instead. Compile tested only.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
Thanks, Alexey
Makes sense, but ....
> Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 736b2ada6..63f6e9436 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5272,7 +5272,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
> settings->hidden_ssid);
settings->hidden_ssid has already been used above in following call:
err = brcmf_fil_iovar_int_set(ifp, "closednet",
settings->hidden_ssid);
So we pass the value as is to firmware using the same assumption, ie.
NL80211_HIDDEN_SSID_NOT_IN_USE. Is this not ABI thus very unlikely to
change?
@Johannes:
Actually not quite understanding the reason for having this setting in
nl80211. hidden_ssid means SSID element length is zero, right?
Regards,
Arend
> if (err) {
> bphy_err(drvr, "%s closednet error (%d)\n",
> - settings->hidden_ssid ?
> + (settings->hidden_ssid != NL80211_HIDDEN_SSID_NOT_IN_USE) ?
> "enabled" : "disabled",
> err);
> goto exit;
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-08 14:06 ` Arend van Spriel
@ 2024-02-08 14:15 ` Johannes Berg
2024-02-08 15:56 ` Arend Van Spriel
0 siblings, 1 reply; 15+ messages in thread
From: Johannes Berg @ 2024-02-08 14:15 UTC (permalink / raw)
To: Arend van Spriel, Alexey Berezhok, Arend van Spriel
Cc: Kalle Valo, linux-wireless, lvc-project
On Thu, 2024-02-08 at 15:06 +0100, Arend van Spriel wrote:
>
> settings->hidden_ssid has already been used above in following call:
>
> err = brcmf_fil_iovar_int_set(ifp, "closednet",
> settings->hidden_ssid);
>
> So we pass the value as is to firmware using the same assumption, ie.
> NL80211_HIDDEN_SSID_NOT_IN_USE. Is this not ABI thus very unlikely to
> change?
The ABI won't change, that'd break all the users of nl80211 that use
this :-)
> @Johannes:
> Actually not quite understanding the reason for having this setting in
> nl80211. hidden_ssid means SSID element length is zero, right?
>
Well, there at least _were_ APs doing the correct SSID length but
setting all octets to zero ... Not sure that's still a thing though.
johannes
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-08 14:15 ` Johannes Berg
@ 2024-02-08 15:56 ` Arend Van Spriel
0 siblings, 0 replies; 15+ messages in thread
From: Arend Van Spriel @ 2024-02-08 15:56 UTC (permalink / raw)
To: Johannes Berg, Alexey Berezhok, Arend van Spriel
Cc: Kalle Valo, linux-wireless, lvc-project
[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]
On February 8, 2024 3:15:25 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2024-02-08 at 15:06 +0100, Arend van Spriel wrote:
>>
>> settings->hidden_ssid has already been used above in following call:
>>
>> err = brcmf_fil_iovar_int_set(ifp, "closednet",
>> settings->hidden_ssid);
>>
>> So we pass the value as is to firmware using the same assumption, ie.
>> NL80211_HIDDEN_SSID_NOT_IN_USE. Is this not ABI thus very unlikely to
>> change?
>
> The ABI won't change, that'd break all the users of nl80211 that use
> this :-)
Right. So basically the assumption that NL80211_HIDDEN_SSID_NOT_IN_USE is
and will be zero is a safe one.
>
>> @Johannes:
>> Actually not quite understanding the reason for having this setting in
>> nl80211. hidden_ssid means SSID element length is zero, right?
>
> Well, there at least _were_ APs doing the correct SSID length but
> setting all octets to zero ... Not sure that's still a thing though.
I now looked at the definition and see indeed two distinct flavours of
"hidden SSID". Interesting although I have never considered it a useful
feature.
Regards,
Arend
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-08 8:51 [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean Alexey Berezhok
2024-02-08 14:06 ` Arend van Spriel
@ 2024-02-12 15:38 ` Kalle Valo
2024-02-12 16:00 ` Arend van Spriel
2024-02-19 18:25 ` andy.shevchenko
2 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2024-02-12 15:38 UTC (permalink / raw)
To: Alexey Berezhok
Cc: Arend van Spriel, linux-wireless, lvc-project, Alexey Berezhok
Alexey Berezhok <a@bayrepo.ru> wrote:
> In 'brcmf_cfg80211_start_ap()', not assume that
> NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
> an explicit check instead. Compile tested only.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
Patch applied to wireless-next.git, thanks.
f20073f50dfd wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
--
https://patchwork.kernel.org/project/linux-wireless/patch/20240208085121.2430-1-a@bayrepo.ru/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-12 15:38 ` Kalle Valo
@ 2024-02-12 16:00 ` Arend van Spriel
2024-02-12 16:03 ` Kalle Valo
2024-02-14 10:28 ` Alexey Berezhok
0 siblings, 2 replies; 15+ messages in thread
From: Arend van Spriel @ 2024-02-12 16:00 UTC (permalink / raw)
To: Kalle Valo, Alexey Berezhok; +Cc: Arend van Spriel, linux-wireless, lvc-project
[-- Attachment #1: Type: text/plain, Size: 700 bytes --]
On 2/12/2024 4:38 PM, Kalle Valo wrote:
> Alexey Berezhok <a@bayrepo.ru> wrote:
>
>> In 'brcmf_cfg80211_start_ap()', not assume that
>> NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
>> an explicit check instead. Compile tested only.
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>
>> Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
>
> Patch applied to wireless-next.git, thanks.
>
> f20073f50dfd wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
Alexey,
Can you do a follow-up patch addressing my comment? If not I will do it
myself.
Regards,
Arend
--
https://patchwork.kernel.org/project/linux-wireless/patch/20240208085121.2430-1-a@bayrepo.ru/
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-12 16:00 ` Arend van Spriel
@ 2024-02-12 16:03 ` Kalle Valo
2024-02-12 16:43 ` Arend Van Spriel
2024-02-14 10:28 ` Alexey Berezhok
1 sibling, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2024-02-12 16:03 UTC (permalink / raw)
To: Arend van Spriel
Cc: Alexey Berezhok, Arend van Spriel, linux-wireless, lvc-project
Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> On 2/12/2024 4:38 PM, Kalle Valo wrote:
>> Alexey Berezhok <a@bayrepo.ru> wrote:
>>
>>> In 'brcmf_cfg80211_start_ap()', not assume that
>>> NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
>>> an explicit check instead. Compile tested only.
>>>
>>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>>
>>> Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
>> Patch applied to wireless-next.git, thanks.
>> f20073f50dfd wifi: brcmfmac: do not cast hidden SSID attribute value
>> to boolean
>
> Alexey,
>
> Can you do a follow-up patch addressing my comment? If not I will do
> it myself.
Sorry, was I not supposed to apply the patch? What did I miss?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-12 16:03 ` Kalle Valo
@ 2024-02-12 16:43 ` Arend Van Spriel
2024-02-13 13:34 ` Kalle Valo
0 siblings, 1 reply; 15+ messages in thread
From: Arend Van Spriel @ 2024-02-12 16:43 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]
On February 12, 2024 5:03:14 PM Kalle Valo <kvalo@kernel.org> wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 2/12/2024 4:38 PM, Kalle Valo wrote:
>>> Alexey Berezhok <a@bayrepo.ru> wrote:
>>>
>>>> In 'brcmf_cfg80211_start_ap()', not assume that
>>>> NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
>>>> an explicit check instead. Compile tested only.
>>>>
>>>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>>>
>>>> Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
>>> Patch applied to wireless-next.git, thanks.
>>> f20073f50dfd wifi: brcmfmac: do not cast hidden SSID attribute value
>>> to boolean
>>
>> Alexey,
>>
>> Can you do a follow-up patch addressing my comment? If not I will do
>> it myself.
>
> Sorry, was I not supposed to apply the patch? What did I miss?
Nothing serious. settings->hidden_ssid value is used as-is to configure
firmware. I wanted Alexey to address that in a v2.
Regards,
Arend
>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-12 16:43 ` Arend Van Spriel
@ 2024-02-13 13:34 ` Kalle Valo
2024-02-13 14:01 ` Arend van Spriel
0 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2024-02-13 13:34 UTC (permalink / raw)
To: Arend Van Spriel; +Cc: linux-wireless
Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
> On February 12, 2024 5:03:14 PM Kalle Valo <kvalo@kernel.org> wrote:
>
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 2/12/2024 4:38 PM, Kalle Valo wrote:
>>>> Alexey Berezhok <a@bayrepo.ru> wrote:
>>>>
>>>>> In 'brcmf_cfg80211_start_ap()', not assume that
>>>>> NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
>>>>> an explicit check instead. Compile tested only.
>>>>>
>>>>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>>>>
>>>>> Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
>>>> Patch applied to wireless-next.git, thanks.
>>>> f20073f50dfd wifi: brcmfmac: do not cast hidden SSID attribute value
>>>> to boolean
>>>
>>> Alexey,
>>>
>>> Can you do a follow-up patch addressing my comment? If not I will do
>>> it myself.
>>
>> Sorry, was I not supposed to apply the patch? What did I miss?
>
> Nothing serious. settings->hidden_ssid value is used as-is to
> configure firmware. I wanted Alexey to address that in a v2.
My bad, I misunderstood your intentions. Luckily this time it wasn't
serious.
BTW to make super clear to me I would prefer that you (Arend) use
Acked-by. It shows up in my script like the number '1' here:
*[ 4] [next] wifi: carl9170: Remove redundant assignment t... 1 - - 2 5d Colin Ian Ki Under Review
So if I don't see your Acked-by then I will not even look at the patch :)
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-13 13:34 ` Kalle Valo
@ 2024-02-13 14:01 ` Arend van Spriel
2024-02-13 15:46 ` Kalle Valo
0 siblings, 1 reply; 15+ messages in thread
From: Arend van Spriel @ 2024-02-13 14:01 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]
On 2/13/2024 2:34 PM, Kalle Valo wrote:
> Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On February 12, 2024 5:03:14 PM Kalle Valo <kvalo@kernel.org> wrote:
>>
>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>
>>>> On 2/12/2024 4:38 PM, Kalle Valo wrote:
>>>>> Alexey Berezhok <a@bayrepo.ru> wrote:
>>>>>
>>>>>> In 'brcmf_cfg80211_start_ap()', not assume that
>>>>>> NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
>>>>>> an explicit check instead. Compile tested only.
>>>>>>
>>>>>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>>>>>
>>>>>> Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
>>>>> Patch applied to wireless-next.git, thanks.
>>>>> f20073f50dfd wifi: brcmfmac: do not cast hidden SSID attribute value
>>>>> to boolean
>>>>
>>>> Alexey,
>>>>
>>>> Can you do a follow-up patch addressing my comment? If not I will do
>>>> it myself.
>>>
>>> Sorry, was I not supposed to apply the patch? What did I miss?
>>
>> Nothing serious. settings->hidden_ssid value is used as-is to
>> configure firmware. I wanted Alexey to address that in a v2.
>
> My bad, I misunderstood your intentions. Luckily this time it wasn't
> serious.
>
> BTW to make super clear to me I would prefer that you (Arend) use
> Acked-by. It shows up in my script like the number '1' here:
>
> *[ 4] [next] wifi: carl9170: Remove redundant assignment t... 1 - - 2 5d Colin Ian Ki Under Review
>
> So if I don't see your Acked-by then I will not even look at the patch :)
Sure. I tend to use Acked-by if things look sane a quick glance. If I
need to dig further I prefer to use Reviewed-by. If I have comments to
revise the patch I will refrain using them. Is that ok or you really
want it to be Acked-by?
Regards,
Arend
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-13 14:01 ` Arend van Spriel
@ 2024-02-13 15:46 ` Kalle Valo
2024-02-13 17:40 ` Arend Van Spriel
0 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2024-02-13 15:46 UTC (permalink / raw)
To: Arend van Spriel; +Cc: linux-wireless
Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>> My bad, I misunderstood your intentions. Luckily this time it wasn't
>> serious.
>> BTW to make super clear to me I would prefer that you (Arend) use
>> Acked-by. It shows up in my script like the number '1' here:
>> *[ 4] [next] wifi: carl9170: Remove redundant assignment t... 1 - -
>> 2 5d Colin Ian Ki Under Review
>> So if I don't see your Acked-by then I will not even look at the
>> patch :)
>
> Sure. I tend to use Acked-by if things look sane a quick glance. If I
> need to dig further I prefer to use Reviewed-by. If I have comments to
> revise the patch I will refrain using them. Is that ok or you really
> want it to be Acked-by?
Ah, now I understand better. My understanding is that the maintainer of
the driver uses Acked-by and others use Reviewed-by. This says the same:
https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
And yes, I would prefer if you could use Acked-by always. It's up to
your judgement if you do just a peek or in-depth review :)
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-13 15:46 ` Kalle Valo
@ 2024-02-13 17:40 ` Arend Van Spriel
0 siblings, 0 replies; 15+ messages in thread
From: Arend Van Spriel @ 2024-02-13 17:40 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]
On February 13, 2024 4:46:38 PM Kalle Valo <kvalo@kernel.org> wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>>> My bad, I misunderstood your intentions. Luckily this time it wasn't
>>> serious.
>>> BTW to make super clear to me I would prefer that you (Arend) use
>>> Acked-by. It shows up in my script like the number '1' here:
>>> *[ 4] [next] wifi: carl9170: Remove redundant assignment t... 1 - -
>>> 2 5d Colin Ian Ki Under Review
>>> So if I don't see your Acked-by then I will not even look at the
>>> patch :)
>>
>> Sure. I tend to use Acked-by if things look sane a quick glance. If I
>> need to dig further I prefer to use Reviewed-by. If I have comments to
>> revise the patch I will refrain using them. Is that ok or you really
>> want it to be Acked-by?
>
> Ah, now I understand better. My understanding is that the maintainer of
> the driver uses Acked-by and others use Reviewed-by. This says the same:
>
> https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
>
> And yes, I would prefer if you could use Acked-by always. It's up to
> your judgement if you do just a peek or in-depth review :)
No problem. I think I can do that.
Regards,
Arend
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-12 16:00 ` Arend van Spriel
2024-02-12 16:03 ` Kalle Valo
@ 2024-02-14 10:28 ` Alexey Berezhok
2024-02-14 11:44 ` Arend Van Spriel
1 sibling, 1 reply; 15+ messages in thread
From: Alexey Berezhok @ 2024-02-14 10:28 UTC (permalink / raw)
To: Arend van Spriel, Kalle Valo
Cc: Arend van Spriel, linux-wireless, lvc-project
12.02.2024 19:00, Arend van Spriel wrote:
> On 2/12/2024 4:38 PM, Kalle Valo wrote:
>> Alexey Berezhok <a@bayrepo.ru> wrote:
>>
>>> In 'brcmf_cfg80211_start_ap()', not assume that
>>> NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
>>> an explicit check instead. Compile tested only.
>>>
>>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>>
>>> Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
>>
>> Patch applied to wireless-next.git, thanks.
>>
>> f20073f50dfd wifi: brcmfmac: do not cast hidden SSID attribute value
>> to boolean
>
> Alexey,
>
> Can you do a follow-up patch addressing my comment? If not I will do it
> myself.
>
> Regards,
> Arend
> --
> https://patchwork.kernel.org/project/linux-wireless/patch/20240208085121.2430-1-a@bayrepo.ru/
Hello, do you mean this kind of modification:
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 63f6e9436..d8f7bd5ce 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5269,7 +5269,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy,
struct net_device *ndev,
}
err = brcmf_fil_iovar_int_set(ifp, "closednet",
- settings->hidden_ssid);
+ (settings->hidden_ssid ==
NL80211_HIDDEN_SSID_NOT_IN_USE? 0 : 1));
if (err) {
bphy_err(drvr, "%s closednet error (%d)\n",
(settings->hidden_ssid !=
NL80211_HIDDEN_SSID_NOT_IN_USE) ?
If so, I will make additional patch.
Regards,
Alexey
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-14 10:28 ` Alexey Berezhok
@ 2024-02-14 11:44 ` Arend Van Spriel
0 siblings, 0 replies; 15+ messages in thread
From: Arend Van Spriel @ 2024-02-14 11:44 UTC (permalink / raw)
To: Alexey Berezhok, Kalle Valo; +Cc: Arend van Spriel, linux-wireless, lvc-project
[-- Attachment #1: Type: text/plain, Size: 1964 bytes --]
On February 14, 2024 11:28:28 AM Alexey Berezhok <a@bayrepo.ru> wrote:
> 12.02.2024 19:00, Arend van Spriel wrote:
>> On 2/12/2024 4:38 PM, Kalle Valo wrote:
>>> Alexey Berezhok <a@bayrepo.ru> wrote:
>>>
>>>> In 'brcmf_cfg80211_start_ap()', not assume that
>>>> NL80211_HIDDEN_SSID_NOT_IN_USE is zero but prefer
>>>> an explicit check instead. Compile tested only.
>>>>
>>>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>>>
>>>> Signed-off-by: Alexey Berezhok <a@bayrepo.ru>
>>>
>>> Patch applied to wireless-next.git, thanks.
>>>
>>> f20073f50dfd wifi: brcmfmac: do not cast hidden SSID attribute value
>>> to boolean
>>
>> Alexey,
>>
>> Can you do a follow-up patch addressing my comment? If not I will do it
>> myself.
>>
>> Regards,
>> Arend
>> --
>> https://patchwork.kernel.org/project/linux-wireless/patch/20240208085121.2430-1-a@bayrepo.ru/
>
> Hello, do you mean this kind of modification:
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 63f6e9436..d8f7bd5ce 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -5269,7 +5269,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy,
> struct net_device *ndev,
> }
>
> err = brcmf_fil_iovar_int_set(ifp, "closednet",
> - settings->hidden_ssid);
> + (settings->hidden_ssid ==
> NL80211_HIDDEN_SSID_NOT_IN_USE? 0 : 1));
> if (err) {
> bphy_err(drvr, "%s closednet error (%d)\n",
> (settings->hidden_ssid !=
> NL80211_HIDDEN_SSID_NOT_IN_USE) ?
>
> If so, I will make additional patch.
That was indeed what I meant. Consider using local variable 'closednet' to
pass in function call and use for error message.
Regards,
Arend
> Regards,
> Alexey
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean
2024-02-08 8:51 [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean Alexey Berezhok
2024-02-08 14:06 ` Arend van Spriel
2024-02-12 15:38 ` Kalle Valo
@ 2024-02-19 18:25 ` andy.shevchenko
2 siblings, 0 replies; 15+ messages in thread
From: andy.shevchenko @ 2024-02-19 18:25 UTC (permalink / raw)
To: Alexey Berezhok; +Cc: Arend van Spriel, Kalle Valo, linux-wireless, lvc-project
Thu, Feb 08, 2024 at 11:51:21AM +0300, Alexey Berezhok kirjoitti:
...
> - settings->hidden_ssid ?
> + (settings->hidden_ssid != NL80211_HIDDEN_SSID_NOT_IN_USE) ?
> "enabled" : "disabled",
Can somebody switch this to use str_enabled_disabled() from string_choices.h?
Ideally, add str_disabled_enabled() in that header (as macro with negation on
its argument) and use here as
str_disabled_enabled(settings->hidden_ssid == NL80211_HIDDEN_SSID_NOT_IN_USE)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-02-19 18:26 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-08 8:51 [PATCH] wifi: brcmfmac: do not cast hidden SSID attribute value to boolean Alexey Berezhok
2024-02-08 14:06 ` Arend van Spriel
2024-02-08 14:15 ` Johannes Berg
2024-02-08 15:56 ` Arend Van Spriel
2024-02-12 15:38 ` Kalle Valo
2024-02-12 16:00 ` Arend van Spriel
2024-02-12 16:03 ` Kalle Valo
2024-02-12 16:43 ` Arend Van Spriel
2024-02-13 13:34 ` Kalle Valo
2024-02-13 14:01 ` Arend van Spriel
2024-02-13 15:46 ` Kalle Valo
2024-02-13 17:40 ` Arend Van Spriel
2024-02-14 10:28 ` Alexey Berezhok
2024-02-14 11:44 ` Arend Van Spriel
2024-02-19 18:25 ` andy.shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.