diff mbox series

brcmfmac: Fix use after free in brcmf_sdio_readframes()

Message ID 20191203095855.c4fradmsixgbq7mc@kili.mountain (mailing list archive)
State Accepted
Commit 216b44000ada87a63891a8214c347e05a4aea8fe
Delegated to: Kalle Valo
Headers show
Series brcmfmac: Fix use after free in brcmf_sdio_readframes() | expand

Commit Message

Dan Carpenter Dec. 3, 2019, 9:58 a.m. UTC
The brcmu_pkt_buf_free_skb() function frees "pkt" so it leads to a
static checker warning:

    drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c:1974 brcmf_sdio_readframes()
    error: dereferencing freed memory 'pkt'

It looks like there was supposed to be a continue after we free "pkt".

Fixes: 4754fceeb9a6 ("brcmfmac: streamline SDIO read frame routine")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Static analysis.  Not tested.

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Franky Lin Dec. 3, 2019, 6:42 p.m. UTC | #1
On Tue, Dec 3, 2019 at 1:59 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> The brcmu_pkt_buf_free_skb() function frees "pkt" so it leads to a
> static checker warning:
>
>     drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c:1974 brcmf_sdio_readframes()
>     error: dereferencing freed memory 'pkt'
>
> It looks like there was supposed to be a continue after we free "pkt".

Good catch!

Acked-by: Franky Lin <franky.lin@broadcom.com>

>
> Fixes: 4754fceeb9a6 ("brcmfmac: streamline SDIO read frame routine")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static analysis.  Not tested.
>
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index 264ad63232f8..1dea0178832e 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -1935,6 +1935,7 @@ static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
>                                                BRCMF_SDIO_FT_NORMAL)) {
>                                 rd->len = 0;
>                                 brcmu_pkt_buf_free_skb(pkt);
> +                               continue;
>                         }
>                         bus->sdcnt.rx_readahead_cnt++;
>                         if (rd->len != roundup(rd_new.len, 16)) {
> --
> 2.11.0
>
Kalle Valo Dec. 18, 2019, 6:58 p.m. UTC | #2
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> The brcmu_pkt_buf_free_skb() function frees "pkt" so it leads to a
> static checker warning:
> 
>     drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c:1974 brcmf_sdio_readframes()
>     error: dereferencing freed memory 'pkt'
> 
> It looks like there was supposed to be a continue after we free "pkt".
> 
> Fixes: 4754fceeb9a6 ("brcmfmac: streamline SDIO read frame routine")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Acked-by: Franky Lin <franky.lin@broadcom.com>

Patch applied to wireless-drivers-next.git, thanks.

216b44000ada brcmfmac: Fix use after free in brcmf_sdio_readframes()
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 264ad63232f8..1dea0178832e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -1935,6 +1935,7 @@  static uint brcmf_sdio_readframes(struct brcmf_sdio *bus, uint maxframes)
 					       BRCMF_SDIO_FT_NORMAL)) {
 				rd->len = 0;
 				brcmu_pkt_buf_free_skb(pkt);
+				continue;
 			}
 			bus->sdcnt.rx_readahead_cnt++;
 			if (rd->len != roundup(rd_new.len, 16)) {