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

[Arend van Spriel <arend.vanspriel@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

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@broadcom.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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]