linux-kernel - Re: [PATCH] wifi: brcmfmac: pcie: handle randbuf allocation failure

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]

Date: Wed, 6 Mar 2024 13:53:19 +0100
From: Arend van Spriel <arend.vanspriel@...adcom.com>
To: Arnd Bergmann <arnd@...db.de>, Kalle Valo <kvalo@...nel.org>
Cc: Duoming Zhou <duoming@....edu.cn>, linux-kernel@...r.kernel.org,
 Konrad Dybcio <konrad.dybcio@...aro.org>, Hans de Goede
 <hdegoede@...hat.com>, minipli@...ecurity.net,
 linux-wireless@...r.kernel.org, brcm80211@...ts.linux.dev,
 brcm80211-dev-list.pdl@...adcom.com
Subject: Re: [PATCH] wifi: brcmfmac: pcie: handle randbuf allocation failure

On 3/6/2024 12:07 PM, Arnd Bergmann wrote:
> On Wed, Mar 6, 2024, at 11:53, Kalle Valo wrote:
>> Arend van Spriel <arend.vanspriel@...adcom.com> writes:
>>
>>> On 3/1/2024 2:51 PM, Duoming Zhou wrote:
>>>> The kzalloc() in brcmf_pcie_download_fw_nvram() will return
>>>> null if the physical memory has run out. As a result, if we
>>>> use get_random_bytes() to generate random bytes in the randbuf,
>>>> the null pointer dereference bug will happen.
>>>> Return -ENOMEM from brcmf_pcie_download_fw_nvram() if kzalloc()
>>>> fails for randbuf.
>>>> Fixes: 91918ce88d9f ("wifi: brcmfmac: pcie: Provide a buffer of
>>>> random bytes to the device")
>>>
>>> Looks good to me. Looking for kernel guideline about stack usage to
>>> determine whether it would be ok to just use buffer on stack. Does
>>> anyone know. This one is 256 bytes so I guess the allocation is
>>> warranted here.
>>
>> Arnd, what do you suggest? Do we have any documentation or guidelines
>> anywhere?
> 
> I don't think we have anything document about this. I usually
> consider anything more than half a kilobyte as excessive,
> even though the warning limit is higher.
> 
> 256 bytes is usually fine, but in this case I would split out
> the basic block that does this into a separate function
> so it does not share the stack frame with other leaf functions
> below brcmf_pcie_download_fw_nvram(). It might also be justified
> to then mark it as noinline_for_stack.

Thanks, Arnd

Makes sense.

@Duoming Zhou,

Can you provide a v2 with separate function using buffer on stack?

static noinline_for_stack
void brcmf_pcie_provide_random_bytes(struct brcmf_pciedev_info *devinfo, 
u32 address)
{
	u8 randbuf[BRCMF_RANDOM_SEED_LENGTH];
	:
	:
}

Regards,
Arend

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