diff mbox

brcmfmac: fix firmware request processing if nvram load fails

Message ID 1522743495-13004-1-git-send-email-arend.vanspriel@broadcom.com (mailing list archive)
State Accepted
Delegated to: Kalle Valo
Headers show

Commit Message

Arend van Spriel April 3, 2018, 8:18 a.m. UTC
When nvram loading fails a double free occurred. Fix this and reorg the
code a little.

Fixes: commit d09ae51a4b67 ("brcmfmac: pass struct in brcmf_fw_get_firmwares()")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
Hi Kalle,

This fix is for code currently(?) in linux-next in error path caused
a double free.

Regards,
Arend
---
 .../broadcom/brcm80211/brcmfmac/firmware.c         | 36 ++++++++++++----------
 1 file changed, 20 insertions(+), 16 deletions(-)

Comments

Kalle Valo April 3, 2018, 10:04 a.m. UTC | #1
Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> When nvram loading fails a double free occurred. Fix this and reorg the
> code a little.
>
> Fixes: commit d09ae51a4b67 ("brcmfmac: pass struct in brcmf_fw_get_firmwares()")

This should be:

Fixes: d09ae51a4b67 ("brcmfmac: pass struct in brcmf_fw_get_firmwares()")

I can fix that during commit.

> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> ---
> Hi Kalle,
>
> This fix is for code currently(?) in linux-next in error path caused
> a double free.

Ok, I'll queue this to wireless-drivers (after I have fast forwarded it)
for 4.17.
Arend van Spriel April 3, 2018, 11:16 a.m. UTC | #2
On 4/3/2018 12:04 PM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> When nvram loading fails a double free occurred. Fix this and reorg the
>> code a little.
>>
>> Fixes: commit d09ae51a4b67 ("brcmfmac: pass struct in brcmf_fw_get_firmwares()")
>
> This should be:
>
> Fixes: d09ae51a4b67 ("brcmfmac: pass struct in brcmf_fw_get_firmwares()")
>
> I can fix that during commit.
>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>> ---
>> Hi Kalle,
>>
>> This fix is for code currently(?) in linux-next in error path caused
>> a double free.
>
> Ok, I'll queue this to wireless-drivers (after I have fast forwarded it)
> for 4.17.

Thanks

Arend
Kalle Valo April 9, 2018, 3:59 p.m. UTC | #3
Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> When nvram loading fails a double free occurred. Fix this and reorg the
> code a little.
>
> Fixes: commit d09ae51a4b67 ("brcmfmac: pass struct in brcmf_fw_get_firmwares()")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>

Applied to wireless-drivers, thanks.

0b5c0305e57c brcmfmac: fix firmware request processing if nvram load fails

(My script failed so sending this mail manually)
diff mbox

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 9277f4c..94e177d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -459,7 +459,7 @@  static void brcmf_fw_free_request(struct brcmf_fw_request *req)
 	kfree(req);
 }
 
-static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
+static int brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
 {
 	struct brcmf_fw *fwctx = ctx;
 	struct brcmf_fw_item *cur;
@@ -498,13 +498,10 @@  static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
 	brcmf_dbg(TRACE, "nvram %p len %d\n", nvram, nvram_length);
 	cur->nv_data.data = nvram;
 	cur->nv_data.len = nvram_length;
-	return;
+	return 0;
 
 fail:
-	brcmf_dbg(TRACE, "failed: dev=%s\n", dev_name(fwctx->dev));
-	fwctx->done(fwctx->dev, -ENOENT, NULL);
-	brcmf_fw_free_request(fwctx->req);
-	kfree(fwctx);
+	return -ENOENT;
 }
 
 static int brcmf_fw_request_next_item(struct brcmf_fw *fwctx, bool async)
@@ -553,20 +550,27 @@  static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
 	brcmf_dbg(TRACE, "enter: firmware %s %sfound\n", cur->path,
 		  fw ? "" : "not ");
 
-	if (fw) {
-		if (cur->type == BRCMF_FW_TYPE_BINARY)
-			cur->binary = fw;
-		else if (cur->type == BRCMF_FW_TYPE_NVRAM)
-			brcmf_fw_request_nvram_done(fw, fwctx);
-		else
-			release_firmware(fw);
-	} else if (cur->type == BRCMF_FW_TYPE_NVRAM) {
-		brcmf_fw_request_nvram_done(NULL, fwctx);
-	} else if (!(cur->flags & BRCMF_FW_REQF_OPTIONAL)) {
+	if (!fw)
 		ret = -ENOENT;
+
+	switch (cur->type) {
+	case BRCMF_FW_TYPE_NVRAM:
+		ret = brcmf_fw_request_nvram_done(fw, fwctx);
+		break;
+	case BRCMF_FW_TYPE_BINARY:
+		cur->binary = fw;
+		break;
+	default:
+		/* something fishy here so bail out early */
+		brcmf_err("unknown fw type: %d\n", cur->type);
+		release_firmware(fw);
+		ret = -EINVAL;
 		goto fail;
 	}
 
+	if (ret < 0 && !(cur->flags & BRCMF_FW_REQF_OPTIONAL))
+		goto fail;
+
 	do {
 		if (++fwctx->curpos == fwctx->req->n_items) {
 			ret = 0;