2016-01-19 11:39:28

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH] brcmfmac: fix sdio sg table alloc crash

From: Hante Meuleman <[email protected]>

With commit 7d34b0560567 ("brcmfmac: Move all module parameters to
one place") a bug was introduced causing a null pointer exception.
This patch fixes the bug by initializing the sg table till after
the settings have been initialized.

Reported-by: Marc Zyngier <[email protected]>
Tested-by: Marc Zyngier <[email protected]>
Reviewed-by: Arend Van Spriel <[email protected]>
Reviewed-by: Franky (Zhenhui) Lin <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Signed-off-by: Hante Meuleman <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
.../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 40 ++++++++--------------
.../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 5 +++
.../wireless/broadcom/brcm80211/brcmfmac/sdio.h | 1 +
3 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 5363739..b98db8a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -879,11 +879,24 @@ int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn)
return 0;
}

-static void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev)
+void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev)
{
+ struct sdio_func *func;
+ struct mmc_host *host;
+ uint max_blocks;
uint nents;
int err;

+ func = sdiodev->func[2];
+ host = func->card->host;
+ sdiodev->sg_support = host->max_segs > 1;
+ max_blocks = min_t(uint, host->max_blk_count, 511u);
+ sdiodev->max_request_size = min_t(uint, host->max_req_size,
+ max_blocks * func->cur_blksize);
+ sdiodev->max_segment_count = min_t(uint, host->max_segs,
+ SG_MAX_SINGLE_ALLOC);
+ sdiodev->max_segment_size = host->max_seg_size;
+
if (!sdiodev->sg_support)
return;

@@ -1021,9 +1034,6 @@ static void brcmf_sdiod_host_fixup(struct mmc_host *host)

static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
{
- struct sdio_func *func;
- struct mmc_host *host;
- uint max_blocks;
int ret = 0;

sdiodev->num_funcs = 2;
@@ -1054,26 +1064,6 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
goto out;
}

- /*
- * determine host related variables after brcmf_sdiod_probe()
- * as func->cur_blksize is properly set and F2 init has been
- * completed successfully.
- */
- func = sdiodev->func[2];
- host = func->card->host;
- sdiodev->sg_support = host->max_segs > 1;
- max_blocks = min_t(uint, host->max_blk_count, 511u);
- sdiodev->max_request_size = min_t(uint, host->max_req_size,
- max_blocks * func->cur_blksize);
- sdiodev->max_segment_count = min_t(uint, host->max_segs,
- SG_MAX_SINGLE_ALLOC);
- sdiodev->max_segment_size = host->max_seg_size;
-
- /* allocate scatter-gather table. sg support
- * will be disabled upon allocation failure.
- */
- brcmf_sdiod_sgtable_alloc(sdiodev);
-
ret = brcmf_sdiod_freezer_attach(sdiodev);
if (ret)
goto out;
@@ -1084,7 +1074,7 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
ret = -ENODEV;
goto out;
}
- brcmf_sdiod_host_fixup(host);
+ brcmf_sdiod_host_fixup(sdiodev->func[2]->card->host);
out:
if (ret)
brcmf_sdiod_remove(sdiodev);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index dd66143..a14d9d9d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4114,6 +4114,11 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
goto fail;
}

+ /* allocate scatter-gather table. sg support
+ * will be disabled upon allocation failure.
+ */
+ brcmf_sdiod_sgtable_alloc(bus->sdiodev);
+
/* Query the F2 block size, set roundup accordingly */
bus->blocksize = bus->sdiodev->func[2]->cur_blksize;
bus->roundup = min(max_roundup, bus->blocksize);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index 5ec7a6d..23f2231 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -342,6 +342,7 @@ int brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,

/* Issue an abort to the specified function */
int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn);
+void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev);
void brcmf_sdiod_change_state(struct brcmf_sdio_dev *sdiodev,
enum brcmf_sdiod_state state);
#ifdef CONFIG_PM_SLEEP
--
1.9.1



2016-01-19 19:27:52

by Kalle Valo

[permalink] [raw]
Subject: Re: brcmfmac: fix sdio sg table alloc crash


> From: Hante Meuleman <[email protected]>
>
> With commit 7d34b0560567 ("brcmfmac: Move all module parameters to
> one place") a bug was introduced causing a null pointer exception.
> This patch fixes the bug by initializing the sg table till after
> the settings have been initialized.
>
> Fixes: 7d34b0560567 ("brcmfmac: Move all module parameters to one place")
> Reported-by: Marc Zyngier <[email protected]>
> Tested-by: Marc Zyngier <[email protected]>
> Reviewed-by: Arend Van Spriel <[email protected]>
> Reviewed-by: Franky (Zhenhui) Lin <[email protected]>
> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
> Signed-off-by: Hante Meuleman <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>

Thanks, applied to wireless-drivers.git.

Kalle Valo

2016-01-19 12:55:57

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: fix sdio sg table alloc crash

Arend van Spriel <[email protected]> writes:

> From: Hante Meuleman <[email protected]>
>
> With commit 7d34b0560567 ("brcmfmac: Move all module parameters to
> one place") a bug was introduced causing a null pointer exception.
> This patch fixes the bug by initializing the sg table till after
> the settings have been initialized.
>
> Reported-by: Marc Zyngier <[email protected]>
> Tested-by: Marc Zyngier <[email protected]>
> Reviewed-by: Arend Van Spriel <[email protected]>
> Reviewed-by: Franky (Zhenhui) Lin <[email protected]>
> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
> Signed-off-by: Hante Meuleman <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>

Thanks, I'm planning to push this to 4.5.

--
Kalle Valo