diff mbox series

[v3,3/9] brcmfmac: firmware: Do not crash on a NULL board_type

Message ID 20220117142919.207370-4-marcan@marcan.st
State New
Headers show
Series misc brcmfmac fixes (M1/T2 series spin-off) | expand

Commit Message

Hector Martin Jan. 17, 2022, 2:29 p.m. UTC
This unbreaks support for USB devices, which do not have a board_type
to create an alt_path out of and thus were running into a NULL
dereference.

Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Arend van Spriel Jan. 19, 2022, 12:34 p.m. UTC | #1
On 1/17/2022 3:29 PM, Hector Martin wrote:
> This unbreaks support for USB devices, which do not have a board_type
> to create an alt_path out of and thus were running into a NULL
> dereference.
> 
> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 3 +++
>   1 file changed, 3 insertions(+)
Dmitry Osipenko Jan. 19, 2022, 10:02 p.m. UTC | #2
17.01.2022 17:29, Hector Martin пишет:
> This unbreaks support for USB devices, which do not have a board_type
> to create an alt_path out of and thus were running into a NULL
> dereference.
> 
> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
> Signed-off-by: Hector Martin <marcan@marcan.st>

Technically, all patches that are intended to be included into next
stable kernel update require the "Cc: stable@vger.kernel.org" tag.

In practice such patches usually auto-picked by the patch bot, so no
need to resend.

> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> index 1001c8888bfe..63821856bbe1 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> @@ -599,6 +599,9 @@ static char *brcm_alt_fw_path(const char *path, const char *board_type)
>  	char alt_path[BRCMF_FW_NAME_LEN];
>  	char suffix[5];
>  
> +	if (!board_type)
> +		return NULL;
> +
>  	strscpy(alt_path, path, BRCMF_FW_NAME_LEN);
>  	/* At least one character + suffix */
>  	if (strlen(alt_path) < 5)

Good catch!

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Arend van Spriel Jan. 20, 2022, 8:29 a.m. UTC | #3
On 1/19/2022 11:02 PM, Dmitry Osipenko wrote:
> 17.01.2022 17:29, Hector Martin пишет:
>> This unbreaks support for USB devices, which do not have a board_type
>> to create an alt_path out of and thus were running into a NULL
>> dereference.
>>
>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
>> Signed-off-by: Hector Martin <marcan@marcan.st>
> 
> Technically, all patches that are intended to be included into next
> stable kernel update require the "Cc: stable@vger.kernel.org" tag.

Being the nit picker that I am I would say it is recommended to safe 
yourself extra work, not required, for the reason you give below.

> In practice such patches usually auto-picked by the patch bot, so no
> need to resend.

Regards,
Arend
Dmitry Osipenko Jan. 20, 2022, 1:23 p.m. UTC | #4
20.01.2022 11:29, Arend van Spriel пишет:
> On 1/19/2022 11:02 PM, Dmitry Osipenko wrote:
>> 17.01.2022 17:29, Hector Martin пишет:
>>> This unbreaks support for USB devices, which do not have a board_type
>>> to create an alt_path out of and thus were running into a NULL
>>> dereference.
>>>
>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>> binaries")
>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>
>> Technically, all patches that are intended to be included into next
>> stable kernel update require the "Cc: stable@vger.kernel.org" tag.
> 
> Being the nit picker that I am I would say it is recommended to safe
> yourself extra work, not required, for the reason you give below.

Will be nice if stable tag could officially become a recommendation,
implying the stable tag. It's a requirement today, at least Greg KH
always demands to add it :)
Arend van Spriel Jan. 20, 2022, 1:37 p.m. UTC | #5
On 1/20/2022 2:24 PM, Dmitry Osipenko wrote:
> 20.01.2022 16:23, Dmitry Osipenko пишет:
>> 20.01.2022 11:29, Arend van Spriel пишет:
>>> On 1/19/2022 11:02 PM, Dmitry Osipenko wrote:
>>>> 17.01.2022 17:29, Hector Martin пишет:
>>>>> This unbreaks support for USB devices, which do not have a board_type
>>>>> to create an alt_path out of and thus were running into a NULL
>>>>> dereference.
>>>>>
>>>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>>>> binaries")
>>>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>>>
>>>> Technically, all patches that are intended to be included into next
>>>> stable kernel update require the "Cc: stable@vger.kernel.org" tag.
>>>
>>> Being the nit picker that I am I would say it is recommended to safe
>>> yourself extra work, not required, for the reason you give below.
>>
>> Will be nice if stable tag could officially become a recommendation,
>> implying the stable tag. It's a requirement today, at least Greg KH
>> always demands to add it :)
> 
> *implying the stable tag if "fixes" tag presents.

I was a little confused reading your previous email in this thread. This 
makes a lot more sense :-p
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 1001c8888bfe..63821856bbe1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -599,6 +599,9 @@  static char *brcm_alt_fw_path(const char *path, const char *board_type)
 	char alt_path[BRCMF_FW_NAME_LEN];
 	char suffix[5];
 
+	if (!board_type)
+		return NULL;
+
 	strscpy(alt_path, path, BRCMF_FW_NAME_LEN);
 	/* At least one character + suffix */
 	if (strlen(alt_path) < 5)