[PATCH v4 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[PATCH v4 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Hector Martin]

This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
the CLM blob is released in the device remove path.

Fixes: 82f93cf46d60 ("brcmfmac: get chip's default RAM info during PCIe setup")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 8b149996fc00..f876b1d8d00d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1777,6 +1777,8 @@ static void brcmf_pcie_setup(struct device *dev, int ret,
 	ret = brcmf_chip_get_raminfo(devinfo->ci);
 	if (ret) {
 		brcmf_err(bus, "Failed to get RAM info\n");
+		release_firmware(fw);
+		brcmf_fw_nvram_free(nvram);
 		goto fail;
 	}
 
-- 
2.33.0

[PATCH v4 2/9] brcmfmac: firmware: Allocate space for default boardrev in nvram

[Hector Martin]

If boardrev is missing from the NVRAM we add a default one, but this
might need more space in the output buffer than was allocated. Ensure
we have enough padding for this in the buffer.

Fixes: 46f2b38a91b0 ("brcmfmac: insert default boardrev in nvram data if missing")
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 0eb13e5df517..1001c8888bfe 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -207,6 +207,8 @@ static int brcmf_init_nvram_parser(struct nvram_parser *nvp,
 		size = BRCMF_FW_MAX_NVRAM_SIZE;
 	else
 		size = data_len;
+	/* Add space for properties we may add */
+	size += strlen(BRCMF_FW_DEFAULT_BOARDREV) + 1;
 	/* Alloc for extra 0 byte + roundup by 4 + length field */
 	size += 1 + 3 + sizeof(u32);
 	nvp->nvram = kzalloc(size, GFP_KERNEL);
-- 
2.33.0

[PATCH v4 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[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")
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 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)
-- 
2.33.0

[PATCH v4 4/9] brcmfmac: pcie: Declare missing firmware files in pcie.c

[Hector Martin]

Move one of the declarations from sdio.c to pcie.c, since it makes no
sense in the former (SDIO support is optional), and add missing ones.

Fixes: 75729e110e68 ("brcmfmac: expose firmware config files through modinfo")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 7 +++++++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 -
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index f876b1d8d00d..b1ae6c41013f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -59,6 +59,13 @@ BRCMF_FW_DEF(4366B, "brcmfmac4366b-pcie");
 BRCMF_FW_DEF(4366C, "brcmfmac4366c-pcie");
 BRCMF_FW_DEF(4371, "brcmfmac4371-pcie");
 
+/* firmware config files */
+MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.txt");
+MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.*.txt");
+
+/* per-board firmware binaries */
+MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.*.bin");
+
 static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
 	BRCMF_FW_ENTRY(BRCM_CC_43602_CHIP_ID, 0xFFFFFFFF, 43602),
 	BRCMF_FW_ENTRY(BRCM_CC_43465_CHIP_ID, 0xFFFFFFF0, 4366C),
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 8effeb7a7269..5d156e591b35 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -629,7 +629,6 @@ BRCMF_FW_CLM_DEF(43752, "brcmfmac43752-sdio");
 
 /* firmware config files */
 MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-sdio.*.txt");
-MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.*.txt");
 
 /* per-board firmware binaries */
 MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-sdio.*.bin");
-- 
2.33.0

[PATCH v4 5/9] brcmfmac: pcie: Replace brcmf_pcie_copy_mem_todev with memcpy_toio

[Hector Martin]

The alignment check was wrong (e.g. & 4 instead of & 3), and the logic
was also inefficient if the length was not a multiple of 4, since it
would needlessly fall back to copying the entire buffer bytewise.

We already have a perfectly good memcpy_toio function, so just call that
instead of rolling our own copy logic here. brcmf_pcie_init_ringbuffers
was already using it anyway.

Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../broadcom/brcm80211/brcmfmac/pcie.c        | 48 ++-----------------
 1 file changed, 4 insertions(+), 44 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index b1ae6c41013f..c25f48db1f60 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -12,6 +12,7 @@
 #include <linux/interrupt.h>
 #include <linux/bcma/bcma.h>
 #include <linux/sched.h>
+#include <linux/io.h>
 #include <asm/unaligned.h>
 
 #include <soc.h>
@@ -454,47 +455,6 @@ brcmf_pcie_write_ram32(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
 }
 
 
-static void
-brcmf_pcie_copy_mem_todev(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
-			  void *srcaddr, u32 len)
-{
-	void __iomem *address = devinfo->tcm + mem_offset;
-	__le32 *src32;
-	__le16 *src16;
-	u8 *src8;
-
-	if (((ulong)address & 4) || ((ulong)srcaddr & 4) || (len & 4)) {
-		if (((ulong)address & 2) || ((ulong)srcaddr & 2) || (len & 2)) {
-			src8 = (u8 *)srcaddr;
-			while (len) {
-				iowrite8(*src8, address);
-				address++;
-				src8++;
-				len--;
-			}
-		} else {
-			len = len / 2;
-			src16 = (__le16 *)srcaddr;
-			while (len) {
-				iowrite16(le16_to_cpu(*src16), address);
-				address += 2;
-				src16++;
-				len--;
-			}
-		}
-	} else {
-		len = len / 4;
-		src32 = (__le32 *)srcaddr;
-		while (len) {
-			iowrite32(le32_to_cpu(*src32), address);
-			address += 4;
-			src32++;
-			len--;
-		}
-	}
-}
-
-
 static void
 brcmf_pcie_copy_dev_tomem(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
 			  void *dstaddr, u32 len)
@@ -1570,8 +1530,8 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo,
 		return err;
 
 	brcmf_dbg(PCIE, "Download FW %s\n", devinfo->fw_name);
-	brcmf_pcie_copy_mem_todev(devinfo, devinfo->ci->rambase,
-				  (void *)fw->data, fw->size);
+	memcpy_toio(devinfo->tcm + devinfo->ci->rambase,
+		    (void *)fw->data, fw->size);
 
 	resetintr = get_unaligned_le32(fw->data);
 	release_firmware(fw);
@@ -1585,7 +1545,7 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo,
 		brcmf_dbg(PCIE, "Download NVRAM %s\n", devinfo->nvram_name);
 		address = devinfo->ci->rambase + devinfo->ci->ramsize -
 			  nvram_len;
-		brcmf_pcie_copy_mem_todev(devinfo, address, nvram, nvram_len);
+		memcpy_toio(devinfo->tcm + address, nvram, nvram_len);
 		brcmf_fw_nvram_free(nvram);
 	} else {
 		brcmf_dbg(PCIE, "No matching NVRAM file found %s\n",
-- 
2.33.0

[PATCH v4 6/9] brcmfmac: pcie: Fix crashes due to early IRQs

[Hector Martin]

The driver was enabling IRQs before the message processing was
initialized. This could cause IRQs to come in too early and crash the
driver. Instead, move the IRQ enable and hostready to a bus preinit
function, at which point everything is properly initialized.

Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../wireless/broadcom/brcm80211/brcmfmac/pcie.c  | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index c25f48db1f60..3ff4997e1c97 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1315,6 +1315,18 @@ static void brcmf_pcie_down(struct device *dev)
 {
 }
 
+static int brcmf_pcie_preinit(struct device *dev)
+{
+	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
+	struct brcmf_pciedev *buspub = bus_if->bus_priv.pcie;
+
+	brcmf_dbg(PCIE, "Enter\n");
+
+	brcmf_pcie_intr_enable(buspub->devinfo);
+	brcmf_pcie_hostready(buspub->devinfo);
+
+	return 0;
+}
 
 static int brcmf_pcie_tx(struct device *dev, struct sk_buff *skb)
 {
@@ -1423,6 +1435,7 @@ static int brcmf_pcie_reset(struct device *dev)
 }
 
 static const struct brcmf_bus_ops brcmf_pcie_bus_ops = {
+	.preinit = brcmf_pcie_preinit,
 	.txdata = brcmf_pcie_tx,
 	.stop = brcmf_pcie_down,
 	.txctl = brcmf_pcie_tx_ctlpkt,
@@ -1795,9 +1808,6 @@ static void brcmf_pcie_setup(struct device *dev, int ret,
 
 	init_waitqueue_head(&devinfo->mbdata_resp_wait);
 
-	brcmf_pcie_intr_enable(devinfo);
-	brcmf_pcie_hostready(devinfo);
-
 	ret = brcmf_attach(&devinfo->pdev->dev);
 	if (ret)
 		goto fail;
-- 
2.33.0

Re: [PATCH v4 6/9] brcmfmac: pcie: Fix crashes due to early IRQs

[Andy Shevchenko]

On Mon, Jan 31, 2022 at 6:08 PM Hector Martin <marcan@marcan.st> wrote:
>
> The driver was enabling IRQs before the message processing was
> initialized. This could cause IRQs to come in too early and crash the
> driver. Instead, move the IRQ enable and hostready to a bus preinit
> function, at which point everything is properly initialized.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  .../wireless/broadcom/brcm80211/brcmfmac/pcie.c  | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index c25f48db1f60..3ff4997e1c97 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -1315,6 +1315,18 @@ static void brcmf_pcie_down(struct device *dev)
>  {
>  }
>
> +static int brcmf_pcie_preinit(struct device *dev)
> +{
> +       struct brcmf_bus *bus_if = dev_get_drvdata(dev);
> +       struct brcmf_pciedev *buspub = bus_if->bus_priv.pcie;
> +
> +       brcmf_dbg(PCIE, "Enter\n");
> +
> +       brcmf_pcie_intr_enable(buspub->devinfo);
> +       brcmf_pcie_hostready(buspub->devinfo);
> +
> +       return 0;
> +}
>
>  static int brcmf_pcie_tx(struct device *dev, struct sk_buff *skb)
>  {
> @@ -1423,6 +1435,7 @@ static int brcmf_pcie_reset(struct device *dev)
>  }
>
>  static const struct brcmf_bus_ops brcmf_pcie_bus_ops = {
> +       .preinit = brcmf_pcie_preinit,
>         .txdata = brcmf_pcie_tx,
>         .stop = brcmf_pcie_down,
>         .txctl = brcmf_pcie_tx_ctlpkt,
> @@ -1795,9 +1808,6 @@ static void brcmf_pcie_setup(struct device *dev, int ret,
>
>         init_waitqueue_head(&devinfo->mbdata_resp_wait);
>
> -       brcmf_pcie_intr_enable(devinfo);
> -       brcmf_pcie_hostready(devinfo);
> -
>         ret = brcmf_attach(&devinfo->pdev->dev);
>         if (ret)
>                 goto fail;
> --
> 2.33.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 2/9] brcmfmac: firmware: Allocate space for default boardrev in nvram

[Andy Shevchenko]

On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> wrote:
>
> If boardrev is missing from the NVRAM we add a default one, but this
> might need more space in the output buffer than was allocated. Ensure
> we have enough padding for this in the buffer.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: 46f2b38a91b0 ("brcmfmac: insert default boardrev in nvram data if missing")
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> index 0eb13e5df517..1001c8888bfe 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> @@ -207,6 +207,8 @@ static int brcmf_init_nvram_parser(struct nvram_parser *nvp,
>                 size = BRCMF_FW_MAX_NVRAM_SIZE;
>         else
>                 size = data_len;
> +       /* Add space for properties we may add */
> +       size += strlen(BRCMF_FW_DEFAULT_BOARDREV) + 1;
>         /* Alloc for extra 0 byte + roundup by 4 + length field */
>         size += 1 + 3 + sizeof(u32);
>         nvp->nvram = kzalloc(size, GFP_KERNEL);
> --
> 2.33.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Andy Shevchenko]

On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> wrote:
>
> This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
> the CLM blob is released in the device remove path.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> Fixes: 82f93cf46d60 ("brcmfmac: get chip's default RAM info during PCIe setup")
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index 8b149996fc00..f876b1d8d00d 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -1777,6 +1777,8 @@ static void brcmf_pcie_setup(struct device *dev, int ret,
>         ret = brcmf_chip_get_raminfo(devinfo->ci);
>         if (ret) {
>                 brcmf_err(bus, "Failed to get RAM info\n");
> +               release_firmware(fw);
> +               brcmf_fw_nvram_free(nvram);
>                 goto fail;
>         }
>
> --
> 2.33.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Andy Shevchenko]

On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> 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.

...

> @@ -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;

I still think it's better to have both callers do the same thing.

Now it will be the double check in one case,

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Kalle Valo]

Andy Shevchenko <andy.shevchenko@gmail.com> writes:

> On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> 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.
>
> ...
>
>> @@ -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;
>
> I still think it's better to have both callers do the same thing.
>
> Now it will be the double check in one case,

I already applied a similar patch:

https://git.kernel.org/wireless/wireless/c/665408f4c3a5

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v4 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Hector Martin]

On 01/02/2022 01.49, Kalle Valo wrote:
> Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> 
>> On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> 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.
>>
>> ...
>>
>>> @@ -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;
>>
>> I still think it's better to have both callers do the same thing.
>>
>> Now it will be the double check in one case,
> 
> I already applied a similar patch:
> 
> https://git.kernel.org/wireless/wireless/c/665408f4c3a5
> 

Feel free to drop this one from the series then, if everything else
looks good.


-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

Re: [PATCH v4 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Andy Shevchenko]

On Mon, Jan 31, 2022 at 6:49 PM Kalle Valo <kvalo@kernel.org> wrote:
> Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> > On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> wrote:

...

> >> +       if (!board_type)
> >> +               return NULL;
> >
> > I still think it's better to have both callers do the same thing.
> >
> > Now it will be the double check in one case,
>
> I already applied a similar patch:
>
> https://git.kernel.org/wireless/wireless/c/665408f4c3a5

"Similar" means that it took into account the concern I expressed here :-)
I would have done slightly differently, but the main idea is the same.
Thank you!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Kalle Valo]

Hector Martin <marcan@marcan.st> writes:

> On 01/02/2022 01.49, Kalle Valo wrote:
>> Andy Shevchenko <andy.shevchenko@gmail.com> writes:
>> 
>>> On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> 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.
>>>
>>> ...
>>>
>>>> @@ -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;
>>>
>>> I still think it's better to have both callers do the same thing.
>>>
>>> Now it will be the double check in one case,
>> 
>> I already applied a similar patch:
>> 
>> https://git.kernel.org/wireless/wireless/c/665408f4c3a5
>> 
>
> Feel free to drop this one from the series then, if everything else
> looks good.

Yes, I'll drop this patch 3.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v4 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Kalle Valo]

Hector Martin <marcan@marcan.st> wrote:

> This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
> the CLM blob is released in the device remove path.
> 
> Fixes: 82f93cf46d60 ("brcmfmac: get chip's default RAM info during PCIe setup")
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

8 patches applied to wireless-next.git, thanks.

5e90f0f3ead0 brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path
d19d8e3ba256 brcmfmac: firmware: Allocate space for default boardrev in nvram
6d766d8cb505 brcmfmac: pcie: Declare missing firmware files in pcie.c
9466987f2467 brcmfmac: pcie: Replace brcmf_pcie_copy_mem_todev with memcpy_toio
b50255c83b91 brcmfmac: pcie: Fix crashes due to early IRQs
9cf6d7f2c554 brcmfmac: of: Use devm_kstrdup for board_type & check for errors
e7191182adc5 brcmfmac: fwil: Constify iovar name arguments
b4bb8469e90e brcmfmac: pcie: Read the console on init and shutdown

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20220131160713.245637-2-marcan@marcan.st/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches