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

Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx> · Wed, 6 Mar 2024 13:53:19 +0100

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@xxxxxxxxxxxx> 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:
smime.p7s

Description: S/MIME Cryptographic Signature