The function brcmf_usb_dl_writeimage() calls the function brcmf_usb_dl_cmd() but dose not check its return value. The 'state.state' and the 'state.bytes' are uninitialized if the function brcmf_usb_dl_cmd() fails. It is dangerous to use uninitialized variables in the conditions.
Add error handling for brcmf_usb_dl_cmd() to jump to error handling path if the brcmf_usb_dl_cmd() fails and the 'state.state' and the 'state.bytes' are uninitialized.
Improve the error message to report more detailed error information.
Fixes: 71bb244ba2fd ("brcm80211: fmac: add USB support for bcm43235/6/8 chipsets") Cc: stable@vger.kernel.org # v3.4+ Signed-off-by: Wentao Liang vulab@iscas.ac.cn --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c index 2821c27f317e..d06c724f63d9 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c @@ -896,14 +896,16 @@ brcmf_usb_dl_writeimage(struct brcmf_usbdev_info *devinfo, u8 *fw, int fwlen) }
/* 1) Prepare USB boot loader for runtime image */ - brcmf_usb_dl_cmd(devinfo, DL_START, &state, sizeof(state)); + err = brcmf_usb_dl_cmd(devinfo, DL_START, &state, sizeof(state)); + if (err) + goto fail;
rdlstate = le32_to_cpu(state.state); rdlbytes = le32_to_cpu(state.bytes);
/* 2) Check we are in the Waiting state */ if (rdlstate != DL_WAITING) { - brcmf_err("Failed to DL_START\n"); + brcmf_err("Invalid DL state: %u\n", rdlstate); err = -EINVAL; goto fail; }
On April 22, 2025 6:22:48 AM Wentao Liang vulab@iscas.ac.cn wrote:
The function brcmf_usb_dl_writeimage() calls the function brcmf_usb_dl_cmd() but dose not check its return value. The 'state.state' and the 'state.bytes' are uninitialized if the function brcmf_usb_dl_cmd() fails. It is dangerous to use uninitialized variables in the conditions.
Add error handling for brcmf_usb_dl_cmd() to jump to error handling path if the brcmf_usb_dl_cmd() fails and the 'state.state' and the 'state.bytes' are uninitialized.
Improve the error message to report more detailed error information.
Fixes: 71bb244ba2fd ("brcm80211: fmac: add USB support for bcm43235/6/8 chipsets") Cc: stable@vger.kernel.org # v3.4+
Thanks for this patch.
Acked-by: Arend van Spriel arend.vanspriel@broadcom.com
Signed-off-by: Wentao Liang vulab@iscas.ac.cn
drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c index 2821c27f317e..d06c724f63d9 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c @@ -896,14 +896,16 @@ brcmf_usb_dl_writeimage(struct brcmf_usbdev_info *devinfo, u8 *fw, int fwlen) }
/* 1) Prepare USB boot loader for runtime image */
- brcmf_usb_dl_cmd(devinfo, DL_START, &state, sizeof(state));
- err = brcmf_usb_dl_cmd(devinfo, DL_START, &state, sizeof(state));
- if (err)
- goto fail;
rdlstate = le32_to_cpu(state.state); rdlbytes = le32_to_cpu(state.bytes);
/* 2) Check we are in the Waiting state */ if (rdlstate != DL_WAITING) {
- brcmf_err("Failed to DL_START\n");
- brcmf_err("Invalid DL state: %u\n", rdlstate);
err = -EINVAL; goto fail; } -- 2.42.0.windows.2
…
brcmf_usb_dl_cmd() but dose not check its return value. The
…
Please avoid typos in such a change description.
Add error handling for brcmf_usb_dl_cmd() to jump to error handling path if the brcmf_usb_dl_cmd() fails and the 'state.state' and the 'state.bytes' are uninitialized.
This wording is improvable.
Improve the error message to report more detailed error information.
Please offer such an adjustment by a separate update step. https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/...
Regards, Markus
On 4/22/2025 12:43 PM, Markus Elfring wrote:
…
brcmf_usb_dl_cmd() but dose not check its return value. The
…
Please avoid typos in such a change description.
Please avoid such a stupid comment and just indicate what you think is wrong and give a proper suggestion to what is should be.
Add error handling for brcmf_usb_dl_cmd() to jump to error handling path if the brcmf_usb_dl_cmd() fails and the 'state.state' and the 'state.bytes' are uninitialized.
This wording is improvable.
That is quite a generic wording you use there. Speak your mind and come up with improved feedback.
Improve the error message to report more detailed error information.
Please offer such an adjustment by a separate update step. https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/...
Nah. The changes are small and within context. The patch is small enough to assure the bar is low enough for swift review. Splitting it up only increases the burden.
Regards, Arend
linux-stable-mirror@lists.linaro.org