| Message ID | 20220131160713.245637-4-marcan@marcan.st (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | misc brcmfmac fixes (M1/T2 series spin-off) | expand |
On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> wrote: > > This unbreaks support for USB devices, which do not have a board_type > to create an alt_path out of and thus were running into a NULL > dereference. ... > @@ -599,6 +599,9 @@ static char *brcm_alt_fw_path(const char *path, const char *board_type) > char alt_path[BRCMF_FW_NAME_LEN]; > char suffix[5]; > > + if (!board_type) > + return NULL; I still think it's better to have both callers do the same thing. Now it will be the double check in one case,
Andy Shevchenko <andy.shevchenko@gmail.com> writes: > On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> wrote: >> >> This unbreaks support for USB devices, which do not have a board_type >> to create an alt_path out of and thus were running into a NULL >> dereference. > > ... > >> @@ -599,6 +599,9 @@ static char *brcm_alt_fw_path(const char *path, >> const char *board_type) >> char alt_path[BRCMF_FW_NAME_LEN]; >> char suffix[5]; >> >> + if (!board_type) >> + return NULL; > > I still think it's better to have both callers do the same thing. > > Now it will be the double check in one case, I already applied a similar patch: https://git.kernel.org/wireless/wireless/c/665408f4c3a5
On 01/02/2022 01.49, Kalle Valo wrote: > Andy Shevchenko <andy.shevchenko@gmail.com> writes: > >> On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> wrote: >>> >>> This unbreaks support for USB devices, which do not have a board_type >>> to create an alt_path out of and thus were running into a NULL >>> dereference. >> >> ... >> >>> @@ -599,6 +599,9 @@ static char *brcm_alt_fw_path(const char *path, >>> const char *board_type) >>> char alt_path[BRCMF_FW_NAME_LEN]; >>> char suffix[5]; >>> >>> + if (!board_type) >>> + return NULL; >> >> I still think it's better to have both callers do the same thing. >> >> Now it will be the double check in one case, > > I already applied a similar patch: > > https://git.kernel.org/wireless/wireless/c/665408f4c3a5 > Feel free to drop this one from the series then, if everything else looks good.
On Mon, Jan 31, 2022 at 6:49 PM Kalle Valo <kvalo@kernel.org> wrote: > Andy Shevchenko <andy.shevchenko@gmail.com> writes: > > On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> wrote: ... > >> + if (!board_type) > >> + return NULL; > > > > I still think it's better to have both callers do the same thing. > > > > Now it will be the double check in one case, > > I already applied a similar patch: > > https://git.kernel.org/wireless/wireless/c/665408f4c3a5 "Similar" means that it took into account the concern I expressed here :-) I would have done slightly differently, but the main idea is the same. Thank you!
Hector Martin <marcan@marcan.st> writes: > On 01/02/2022 01.49, Kalle Valo wrote: >> Andy Shevchenko <andy.shevchenko@gmail.com> writes: >> >>> On Mon, Jan 31, 2022 at 6:07 PM Hector Martin <marcan@marcan.st> wrote: >>>> >>>> This unbreaks support for USB devices, which do not have a board_type >>>> to create an alt_path out of and thus were running into a NULL >>>> dereference. >>> >>> ... >>> >>>> @@ -599,6 +599,9 @@ static char *brcm_alt_fw_path(const char *path, >>>> const char *board_type) >>>> char alt_path[BRCMF_FW_NAME_LEN]; >>>> char suffix[5]; >>>> >>>> + if (!board_type) >>>> + return NULL; >>> >>> I still think it's better to have both callers do the same thing. >>> >>> Now it will be the double check in one case, >> >> I already applied a similar patch: >> >> https://git.kernel.org/wireless/wireless/c/665408f4c3a5 >> > > Feel free to drop this one from the series then, if everything else > looks good. Yes, I'll drop this patch 3.
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c index 1001c8888bfe..63821856bbe1 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c @@ -599,6 +599,9 @@ static char *brcm_alt_fw_path(const char *path, const char *board_type) char alt_path[BRCMF_FW_NAME_LEN]; char suffix[5]; + if (!board_type) + return NULL; + strscpy(alt_path, path, BRCMF_FW_NAME_LEN); /* At least one character + suffix */ if (strlen(alt_path) < 5)
This unbreaks support for USB devices, which do not have a board_type to create an alt_path out of and thus were running into a NULL dereference. Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries") Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> Reviewed-by: Dmitry Osipenko <digetx@gmail.com> Cc: stable@vger.kernel.org Signed-off-by: Hector Martin <marcan@marcan.st> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 3 +++ 1 file changed, 3 insertions(+)