On 10/20/2022 3:49 AM, Dokyung Song wrote:
This patch fixes an intra-object buffer overflow in brcmfmac that occurs
when the device provides a 'bsscfgidx' equal to or greater than the
buffer size. The patch adds a check that leads to a safe failure if that
is the case.
Found by a modified version of syzkaller.
UBSAN: array-index-out-of-bounds in drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 52 is out of range for type 'brcmf_if *[16]'
CPU: 0 PID: 1898 Comm: kworker/0:2 Tainted: G O 5.14.0+ #132
[...]
Kernel panic - not syncing: Fatal exception
Reported-by: Dokyung Song <dokyungs@xxxxxxxxxxxx>
Reported-by: Jisoo Jang <jisoo.jang@xxxxxxxxxxxx>
Reported-by: Minsuk Kang <linuxlovemin@xxxxxxxxxxxx>
Not sure what the rules are for using 'Reported-by' tag, but it looks a
bit odd. I leave it to the wireless maintainer.
Reviewed-by: Arend van Spriel <aspriel@xxxxxxxxx>
Signed-off-by: Dokyung Song <dokyung.song@xxxxxxxxx>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index bc3f4e4edcdf..e035e9c5a1fa 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -255,10 +255,15 @@ static void brcmf_fweh_event_worker(struct work_struct *work)
goto event_free;
}
- if (event->code == BRCMF_E_TDLS_PEER_EVENT)
+ if (event->code == BRCMF_E_TDLS_PEER_EVENT) {
ifp = drvr->iflist[0];
- else
+ } else {
+ if (emsg.bsscfgidx >= BRCMF_MAX_IFS) {
+ bphy_err(drvr, "invalid bsscfg index: %u\n", emsg.bsscfgidx);
+ goto event_free;
+ }
probably better to do the validation before any other handling. So right
after converting the event message at line 245
https://elixir.bootlin.com/linux/latest/source/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c#L245
ifp = drvr->iflist[emsg.bsscfgidx];
+ }
err = brcmf_fweh_call_event_handler(drvr, ifp, event->code,
&emsg, event->data);
if (err) {
