2019-11-05 09:08:24

by Tony Chuang

Subject: [PATCH] rtw88: fix potential NULL pointer access for firmware

From: Yan-Hsuan Chuang <[email protected]>

Driver could access a NULL firmware pointer if we don't
return here.

Fixes: 5195b90426409 ("rtw88: avoid FW info flood")
Reported-by: kbuild test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Yan-Hsuan Chuang <[email protected]>
---
drivers/net/wireless/realtek/rtw88/main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 021668f1b74f..de82d08ea29e 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -1024,8 +1024,10 @@ static void rtw_load_firmware_cb(const struct firmware *firmware, void *context)
struct rtw_fw_state *fw = &rtwdev->fw;
const struct rtw_fw_hdr *fw_hdr;

- if (!firmware)
+ if (!firmware || !firmware->data) {
rtw_err(rtwdev, "failed to request firmware\n");
+ return;
+ }

fw_hdr = (const struct rtw_fw_hdr *)firmware->data;
fw->h2c_version = le16_to_cpu(fw_hdr->h2c_fmt_ver);
--
2.17.1

2019-11-06 17:59:09

by Kalle Valo

Subject: Re: [PATCH] rtw88: fix potential NULL pointer access for firmware

<[email protected]> wrote:

> From: Yan-Hsuan Chuang <[email protected]>
>
> Driver could access a NULL firmware pointer if we don't
> return here.
>
> Fixes: 5195b90426409 ("rtw88: avoid FW info flood")
> Reported-by: kbuild test robot <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Yan-Hsuan Chuang <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

f530c1961af2 rtw88: fix potential NULL pointer access for firmware

--
https://patchwork.kernel.org/patch/11227185/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2019-11-06 21:34:47

by Brian Norris

Subject: Re: [PATCH] rtw88: fix potential NULL pointer access for firmware

On Tue, Nov 5, 2019 at 1:06 AM <[email protected]> wrote:
...
> Fixes: 5195b90426409 ("rtw88: avoid FW info flood")
...
> --- a/drivers/net/wireless/realtek/rtw88/main.c
> +++ b/drivers/net/wireless/realtek/rtw88/main.c
> @@ -1024,8 +1024,10 @@ static void rtw_load_firmware_cb(const struct firmware *firmware, void *context)
> struct rtw_fw_state *fw = &rtwdev->fw;
> const struct rtw_fw_hdr *fw_hdr;
>
> - if (!firmware)
> + if (!firmware || !firmware->data) {
> rtw_err(rtwdev, "failed to request firmware\n");

I think you still wanted 'complete_all()' here, otherwise your waiters
will hang forever. (They correctly check whether the firmware is NULL,
so that's not a problem there.)

I'll send a follow-up to the follow-up :)

Brian

> + return;
> + }