netdev - Re: [PATCH] wifi: brcmfmac: Fix allocation size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <173e144f-7814-b9c2-3f97-8401f4f9b34a@broadcom.com>
Date:   Tue, 17 Jan 2023 14:56:53 +0100
From:   Arend van Spriel <arend.vanspriel@...adcom.com>
To:     "Alexey V. Vissarionov" <gremlin@...linux.org>,
        Simon Horman <simon.horman@...igine.com>
Cc:     Arend van Spriel <aspriel@...il.com>,
        Franky Lin <franky.lin@...adcom.com>,
        Hante Meuleman <hante.meuleman@...adcom.com>,
        Kalle Valo <kvalo@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Alvin Šipraga <alsi@...g-olufsen.dk>,
        Ahmad Fatoum <a.fatoum@...gutronix.de>,
        Wataru Gohda <wataru.gohda@...ress.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Pieter-Paul Giesberts <pieter-paul.giesberts@...adcom.com>,
        linux-wireless@...r.kernel.org,
        brcm80211-dev-list.pdl@...adcom.com,
        SHA-cyfmac-dev-list@...ineon.com, netdev@...r.kernel.org,
        lvc-project@...uxtesting.org
Subject: Re: [PATCH] wifi: brcmfmac: Fix allocation size

On 1/17/2023 12:54 PM, Alexey V. Vissarionov wrote:
> On 2023-01-17 12:13:06 +0100, Simon Horman wrote:
> 
>   >> buf_size = sizeof(*rfi);
>   >> max_idx = reorder_data[BRCMF_RXREORDER_MAXIDX_OFFSET];
>   >> - buf_size += (max_idx + 1) * sizeof(pkt);
>   >> + buf_size += (max_idx + 1) * sizeof(struct sk_buff);
> 
>   > This is followed by:
>   > rfi = kzalloc(buf_size, GFP_ATOMIC);
>   > ...
>   > rfi->pktslots = (struct sk_buff **)(rfi + 1);
>   > The type of rfi is struct brcmf_ampdu_rx_reorder, which
>   > looks like this:
>   > struct brcmf_ampdu_rx_reorder
>   > { struct sk_buff **pktslots; ... };
>   > And it looks to me that pkt is used as an array of
>   > (struct sk_buff *).
>   > So in all, it seems to me that the current code is correct.
> 
> So, the buf_size is a sum of sizeof(struct brcmf_ampdu_rx_reorder)
> and size of array of pointers... and yes, this array is filled with
> pointers: rfi->pktslots[rfi->cur_idx] = pkt;
> 
> Hmmm... looks correct. Sorry for bothering.

No problem. Nice to see the water went still without me chiming in. It 
has been a while since this was added to the driver and there could be 
issues with this code, but if this allocation was wrong we would have 
had reports by now.

Thanks,
Arend

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4219 bytes)