diff mbox series

brcmfmac: firmware: Fix crash in brcm_alt_fw_path

Message ID 20220118154514.3245524-1-phil@raspberrypi.com (mailing list archive)
State Accepted
Commit 665408f4c3a5c83e712871daa062721624b2b79e
Delegated to: Kalle Valo
Headers show
Series brcmfmac: firmware: Fix crash in brcm_alt_fw_path | expand

Commit Message

Phil Elwell Jan. 18, 2022, 3:45 p.m. UTC
The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
by a check to the validity of the fwctx->req->board_type pointer. This
results in a crash in strlcat when, for example, the WLAN chip is found
in a USB dongle.

Prevent the crash by adding the necessary check.

See: https://github.com/raspberrypi/linux/issues/4833

Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
Signed-off-by: Phil Elwell <phil@raspberrypi.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Kalle Valo Jan. 19, 2022, 6:01 a.m. UTC | #1
Phil Elwell <phil@raspberrypi.com> writes:

> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
> by a check to the validity of the fwctx->req->board_type pointer. This
> results in a crash in strlcat when, for example, the WLAN chip is found
> in a USB dongle.
>
> Prevent the crash by adding the necessary check.
>
> See: https://github.com/raspberrypi/linux/issues/4833
>
> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>

I think this should go to v5.17.
Phil Elwell Jan. 19, 2022, 8:53 a.m. UTC | #2
On 19/01/2022 06:01, Kalle Valo wrote:
> Phil Elwell <phil@raspberrypi.com> writes:
> 
>> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
>> by a check to the validity of the fwctx->req->board_type pointer. This
>> results in a crash in strlcat when, for example, the WLAN chip is found
>> in a USB dongle.
>>
>> Prevent the crash by adding the necessary check.
>>
>> See: https://github.com/raspberrypi/linux/issues/4833
>>
>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> 
> I think this should go to v5.17.

Is that an Ack? Are you asking me to submit the patch in a different way?

Phil
Kalle Valo Jan. 19, 2022, 10 a.m. UTC | #3
Phil Elwell <phil@raspberrypi.com> writes:

> On 19/01/2022 06:01, Kalle Valo wrote:
>> Phil Elwell <phil@raspberrypi.com> writes:
>>
>>> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
>>> by a check to the validity of the fwctx->req->board_type pointer. This
>>> results in a crash in strlcat when, for example, the WLAN chip is found
>>> in a USB dongle.
>>>
>>> Prevent the crash by adding the necessary check.
>>>
>>> See: https://github.com/raspberrypi/linux/issues/4833
>>>
>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>
>> I think this should go to v5.17.
>
> Is that an Ack? 

It's a note to myself and other maintainers/reviewers that I'm planning
to take this to the wireless tree. At the moment I'm waiting for other
people to comment.

> Are you asking me to submit the patch in a different way?

No need, unless something is found during review.
Arend Van Spriel Jan. 19, 2022, 3:48 p.m. UTC | #4
On 1/19/2022 9:53 AM, Phil Elwell wrote:
> On 19/01/2022 06:01, Kalle Valo wrote:
>> Phil Elwell <phil@raspberrypi.com> writes:
>>
>>> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
>>> by a check to the validity of the fwctx->req->board_type pointer. This
>>> results in a crash in strlcat when, for example, the WLAN chip is found
>>> in a USB dongle.
>>>
>>> Prevent the crash by adding the necessary check.
>>>
>>> See: https://github.com/raspberrypi/linux/issues/4833
>>>
>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware 
>>> binaries")
>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>
>> I think this should go to v5.17.
> 
> Is that an Ack? Are you asking me to submit the patch in a different way?

Similar/same patch was submitted by Hector Martin [1].

Regards,
Arend

[1] 
https://patchwork.kernel.org/project/linux-wireless/patch/20220117142919.207370-4-marcan@marcan.st/
Kalle Valo Jan. 27, 2022, 12:08 p.m. UTC | #5
Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 1/19/2022 9:53 AM, Phil Elwell wrote:
>> On 19/01/2022 06:01, Kalle Valo wrote:
>>> Phil Elwell <phil@raspberrypi.com> writes:
>>>
>>>> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
>>>> by a check to the validity of the fwctx->req->board_type pointer. This
>>>> results in a crash in strlcat when, for example, the WLAN chip is found
>>>> in a USB dongle.
>>>>
>>>> Prevent the crash by adding the necessary check.
>>>>
>>>> See: https://github.com/raspberrypi/linux/issues/4833
>>>>
>>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>>> binaries")
>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>
>>> I think this should go to v5.17.
>>
>> Is that an Ack? Are you asking me to submit the patch in a different way?
>
> Similar/same patch was submitted by Hector Martin [1].
>
> Regards,
> Arend
>
> [1]
> https://patchwork.kernel.org/project/linux-wireless/patch/20220117142919.207370-4-marcan@marcan.st/

I would prefer to take this patch to wireless tree and drop Hector's
version. Is that ok?
Arend Van Spriel Jan. 27, 2022, 12:59 p.m. UTC | #6
On 1/27/2022 1:08 PM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> 
>> On 1/19/2022 9:53 AM, Phil Elwell wrote:
>>> On 19/01/2022 06:01, Kalle Valo wrote:
>>>> Phil Elwell <phil@raspberrypi.com> writes:
>>>>
>>>>> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
>>>>> by a check to the validity of the fwctx->req->board_type pointer. This
>>>>> results in a crash in strlcat when, for example, the WLAN chip is found
>>>>> in a USB dongle.
>>>>>
>>>>> Prevent the crash by adding the necessary check.
>>>>>
>>>>> See: https://github.com/raspberrypi/linux/issues/4833
>>>>>
>>>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>>>> binaries")
>>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>>
>>>> I think this should go to v5.17.
>>>
>>> Is that an Ack? Are you asking me to submit the patch in a different way?
>>
>> Similar/same patch was submitted by Hector Martin [1].

Fine by me. Hector's subset series (fixes) is ready to be taken as well, 
right?

Regards,
Arend
Kalle Valo Jan. 27, 2022, 1:17 p.m. UTC | #7
Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 1/27/2022 1:08 PM, Kalle Valo wrote:
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 1/19/2022 9:53 AM, Phil Elwell wrote:
>>>> On 19/01/2022 06:01, Kalle Valo wrote:
>>>>> Phil Elwell <phil@raspberrypi.com> writes:
>>>>>
>>>>>> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
>>>>>> by a check to the validity of the fwctx->req->board_type pointer. This
>>>>>> results in a crash in strlcat when, for example, the WLAN chip is found
>>>>>> in a USB dongle.
>>>>>>
>>>>>> Prevent the crash by adding the necessary check.
>>>>>>
>>>>>> See: https://github.com/raspberrypi/linux/issues/4833
>>>>>>
>>>>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>>>>> binaries")
>>>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>>>
>>>>> I think this should go to v5.17.
>>>>
>>>> Is that an Ack? Are you asking me to submit the patch in a different way?
>>>
>>> Similar/same patch was submitted by Hector Martin [1].
>
> Fine by me. Hector's subset series (fixes) is ready to be taken as
> well, right?

I have not looked at Hector's patches yet, my plan is to take them to
wireless-next.
Arend Van Spriel Jan. 27, 2022, 2:30 p.m. UTC | #8
On 1/27/2022 2:17 PM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> 
>> On 1/27/2022 1:08 PM, Kalle Valo wrote:
>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>
>>>> On 1/19/2022 9:53 AM, Phil Elwell wrote:
>>>>> On 19/01/2022 06:01, Kalle Valo wrote:
>>>>>> Phil Elwell <phil@raspberrypi.com> writes:
>>>>>>
>>>>>>> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
>>>>>>> by a check to the validity of the fwctx->req->board_type pointer. This
>>>>>>> results in a crash in strlcat when, for example, the WLAN chip is found
>>>>>>> in a USB dongle.
>>>>>>>
>>>>>>> Prevent the crash by adding the necessary check.
>>>>>>>
>>>>>>> See: https://github.com/raspberrypi/linux/issues/4833
>>>>>>>
>>>>>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>>>>>> binaries")
>>>>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>>>>
>>>>>> I think this should go to v5.17.
>>>>>
>>>>> Is that an Ack? Are you asking me to submit the patch in a different way?
>>>>
>>>> Similar/same patch was submitted by Hector Martin [1].
>>
>> Fine by me. Hector's subset series (fixes) is ready to be taken as
>> well, right?
> 
> I have not looked at Hector's patches yet, my plan is to take them to
> wireless-next.

Some of them are improvements so wireless-next is where those belong, 
but a few (patches #1-3, and #6) are actual bug fixes.

Regards,
Arend
Kalle Valo Jan. 27, 2022, 3:36 p.m. UTC | #9
Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 1/27/2022 2:17 PM, Kalle Valo wrote:
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 1/27/2022 1:08 PM, Kalle Valo wrote:
>>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>>
>>>>> On 1/19/2022 9:53 AM, Phil Elwell wrote:
>>>>>> On 19/01/2022 06:01, Kalle Valo wrote:
>>>>>>> Phil Elwell <phil@raspberrypi.com> writes:
>>>>>>>
>>>>>>>> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
>>>>>>>> by a check to the validity of the fwctx->req->board_type pointer. This
>>>>>>>> results in a crash in strlcat when, for example, the WLAN chip is found
>>>>>>>> in a USB dongle.
>>>>>>>>
>>>>>>>> Prevent the crash by adding the necessary check.
>>>>>>>>
>>>>>>>> See: https://github.com/raspberrypi/linux/issues/4833
>>>>>>>>
>>>>>>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>>>>>>> binaries")
>>>>>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>>>>>
>>>>>>> I think this should go to v5.17.
>>>>>>
>>>>>> Is that an Ack? Are you asking me to submit the patch in a different way?
>>>>>
>>>>> Similar/same patch was submitted by Hector Martin [1].
>>>
>>> Fine by me. Hector's subset series (fixes) is ready to be taken as
>>> well, right?
>>
>> I have not looked at Hector's patches yet, my plan is to take them to
>> wireless-next.
>
> Some of them are improvements so wireless-next is where those belong,
> but a few (patches #1-3, and #6) are actual bug fixes.

To avoid conflicts I'm keeping the bar high for patches going to
wireless, so it's mostly regression fixes or otherwise important fixes.
Arend Van Spriel Jan. 27, 2022, 4:22 p.m. UTC | #10
On 1/27/2022 4:36 PM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> 
>> On 1/27/2022 2:17 PM, Kalle Valo wrote:
>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>
>>>> On 1/27/2022 1:08 PM, Kalle Valo wrote:
>>>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>>>
>>>>>> On 1/19/2022 9:53 AM, Phil Elwell wrote:
>>>>>>> On 19/01/2022 06:01, Kalle Valo wrote:
>>>>>>>> Phil Elwell <phil@raspberrypi.com> writes:
>>>>>>>>
>>>>>>>>> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
>>>>>>>>> by a check to the validity of the fwctx->req->board_type pointer. This
>>>>>>>>> results in a crash in strlcat when, for example, the WLAN chip is found
>>>>>>>>> in a USB dongle.
>>>>>>>>>
>>>>>>>>> Prevent the crash by adding the necessary check.
>>>>>>>>>
>>>>>>>>> See: https://github.com/raspberrypi/linux/issues/4833
>>>>>>>>>
>>>>>>>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>>>>>>>> binaries")
>>>>>>>>> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
>>>>>>>>
>>>>>>>> I think this should go to v5.17.
>>>>>>>
>>>>>>> Is that an Ack? Are you asking me to submit the patch in a different way?
>>>>>>
>>>>>> Similar/same patch was submitted by Hector Martin [1].
>>>>
>>>> Fine by me. Hector's subset series (fixes) is ready to be taken as
>>>> well, right?
>>>
>>> I have not looked at Hector's patches yet, my plan is to take them to
>>> wireless-next.
>>
>> Some of them are improvements so wireless-next is where those belong,
>> but a few (patches #1-3, and #6) are actual bug fixes.
> 
> To avoid conflicts I'm keeping the bar high for patches going to
> wireless, so it's mostly regression fixes or otherwise important fixes.

Understood. No problem.

Regards,
Arend
Kalle Valo Jan. 28, 2022, 2:07 p.m. UTC | #11
Phil Elwell <phil@raspberrypi.com> wrote:

> The call to brcm_alt_fw_path in brcmf_fw_get_firmwares is not protected
> by a check to the validity of the fwctx->req->board_type pointer. This
> results in a crash in strlcat when, for example, the WLAN chip is found
> in a USB dongle.
> 
> Prevent the crash by adding the necessary check.
> 
> See: https://github.com/raspberrypi/linux/issues/4833
> 
> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>

Patch applied to wireless.git, thanks.

665408f4c3a5 brcmfmac: firmware: Fix crash in brcm_alt_fw_path
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 0eb13e5df5177..d99140960a820 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -693,7 +693,7 @@  int brcmf_fw_get_firmwares(struct device *dev, struct brcmf_fw_request *req,
 {
 	struct brcmf_fw_item *first = &req->items[0];
 	struct brcmf_fw *fwctx;
-	char *alt_path;
+	char *alt_path = NULL;
 	int ret;
 
 	brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(dev));
@@ -712,7 +712,9 @@  int brcmf_fw_get_firmwares(struct device *dev, struct brcmf_fw_request *req,
 	fwctx->done = fw_cb;
 
 	/* First try alternative board-specific path if any */
-	alt_path = brcm_alt_fw_path(first->path, fwctx->req->board_type);
+	if (fwctx->req->board_type)
+		alt_path = brcm_alt_fw_path(first->path,
+					    fwctx->req->board_type);
 	if (alt_path) {
 		ret = request_firmware_nowait(THIS_MODULE, true, alt_path,
 					      fwctx->dev, GFP_KERNEL, fwctx,