| Message ID | 20220131160713.245637-1-marcan@marcan.st |
|---|---|
| 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.