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

[Arend van Spriel]

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