Received-SPF: pass (google.com: domain of linux-wireless+bounces-4427-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-208-g3f1d79aedb-fm-20240301.002-g3f1d79ae
Precedence: bulk
MIME-Version: 1.0
Message-Id: <3d433b58-384f-452e-904d-62e23b3b5a0b@app.fastmail.com>
In-Reply-To: <87h6hjhbqy.fsf@kernel.org>
References: <20240301135134.29577-1-duoming@zju.edu.cn>
 <fdaefac9-1d02-4424-b893-4306b97028ca@broadcom.com>
 <87h6hjhbqy.fsf@kernel.org>
Date: Wed, 06 Mar 2024 12:07:41 +0100
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Kalle Valo" <kvalo@kernel.org>,
 "Arend van Spriel" <arend.vanspriel@broadcom.com>
Cc: "Duoming Zhou" <duoming@zju.edu.cn>, linux-kernel@vger.kernel.org,
 "Konrad Dybcio" <konrad.dybcio@linaro.org>,
 "Hans de Goede" <hdegoede@redhat.com>, minipli@grsecurity.net,
 linux-wireless@vger.kernel.org, brcm80211@lists.linux.dev,
 brcm80211-dev-list.pdl@broadcom.com
Subject: Re: [PATCH] wifi: brcmfmac: pcie: handle randbuf allocation failure
Content-Type: text/plain

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.

      Arnd