brcmfmac: Fixes potential buffer overflow in 'brcmf_fweh_event_worker'
PATCH 1/1 brcmfmac: Fixes potential buffer overflow in 'brcmf_fweh_event_worker'
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 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 Workqueue: events brcmf_fweh_event_worker Call Trace: dump_stack_lvl+0x57/0x7d ubsan_epilogue+0x5/0x40 __ubsan_handle_out_of_bounds+0x69/0x80 ? memcpy+0x39/0x60 brcmf_fweh_event_worker+0xae1/0xc00 ? brcmf_fweh_call_event_handler.isra.0+0x100/0x100 ? rcu_read_lock_sched_held+0xa1/0xd0 ? rcu_read_lock_bh_held+0xb0/0xb0 ? lockdep_hardirqs_on_prepare+0x273/0x3e0 process_one_work+0x873/0x13e0 ? lock_release+0x640/0x640 ? pwq_dec_nr_in_flight+0x320/0x320 ? rwlock_bug.part.0+0x90/0x90 worker_thread+0x8b/0xd10 ? __kthread_parkme+0xd9/0x1d0 ? process_one_work+0x13e0/0x13e0 kthread+0x379/0x450 ? _raw_spin_unlock_irq+0x24/0x30 ? set_kthread_struct+0x100/0x100 ret_from_fork+0x1f/0x30 ================================================================================ general protection fault, probably for non-canonical address 0xe5601c0020023fff: 0000 [#1] SMP KASAN KASAN: maybe wild-memory-access in range [0x2b0100010011fff8-0x2b0100010011ffff] CPU: 0 PID: 1898 Comm: kworker/0:2 Tainted: G O 5.14.0+ #132 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 Workqueue: events brcmf_fweh_event_worker RIP: 0010:brcmf_fweh_call_event_handler.isra.0+0x42/0x100 Code: 89 f5 53 48 89 fb 48 83 ec 08 e8 79 0b 38 fe 48 85 ed 74 7e e8 6f 0b 38 fe 48 89 ea 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80> 3c 02 00 0f 85 8b 00 00 00 4c 8b 7d 00 44 89 e0 48 ba 00 00 00 RSP: 0018:ffffc9000259fbd8 EFLAGS: 00010207 RAX: dffffc0000000000 RBX: ffff888115d8cd50 RCX: 0000000000000000 RDX: 0560200020023fff RSI: ffffffff8304bc91 RDI: ffff888115d8cd50 RBP: 2b0100010011ffff R08: ffff888112340050 R09: ffffed1023549809 R10: ffff88811aa4c047 R11: ffffed1023549808 R12: 0000000000000045 R13: ffffc9000259fca0 R14: ffff888112340050 R15: ffff888112340000 FS: 0000000000000000(0000) GS:ffff88811aa00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 000000004053ccc0 CR3: 0000000112740000 CR4: 0000000000750ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: brcmf_fweh_event_worker+0x117/0xc00 ? brcmf_fweh_call_event_handler.isra.0+0x100/0x100 ? rcu_read_lock_sched_held+0xa1/0xd0 ? rcu_read_lock_bh_held+0xb0/0xb0 ? lockdep_hardirqs_on_prepare+0x273/0x3e0 process_one_work+0x873/0x13e0 ? lock_release+0x640/0x640 ? pwq_dec_nr_in_flight+0x320/0x320 ? rwlock_bug.part.0+0x90/0x90 worker_thread+0x8b/0xd10 ? __kthread_parkme+0xd9/0x1d0 ? process_one_work+0x13e0/0x13e0 kthread+0x379/0x450 ? _raw_spin_unlock_irq+0x24/0x30 ? set_kthread_struct+0x100/0x100 ret_from_fork+0x1f/0x30 Modules linked in: 88XXau(O) 88x2bu(O) ---[ end trace 41d302138f3ff55a ]--- RIP: 0010:brcmf_fweh_call_event_handler.isra.0+0x42/0x100 Code: 89 f5 53 48 89 fb 48 83 ec 08 e8 79 0b 38 fe 48 85 ed 74 7e e8 6f 0b 38 fe 48 89 ea 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80> 3c 02 00 0f 85 8b 00 00 00 4c 8b 7d 00 44 89 e0 48 ba 00 00 00 RSP: 0018:ffffc9000259fbd8 EFLAGS: 00010207 RAX: dffffc0000000000 RBX: ffff888115d8cd50 RCX: 0000000000000000 RDX: 0560200020023fff RSI: ffffffff8304bc91 RDI: ffff888115d8cd50 RBP: 2b0100010011ffff R08: ffff888112340050 R09: ffffed1023549809 R10: ffff88811aa4c047 R11: ffffed1023549808 R12: 0000000000000045 R13: ffffc9000259fca0 R14: ffff888112340050 R15: ffff888112340000 FS: 0000000000000000(0000) GS:ffff88811aa00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 000000004053ccc0 CR3: 0000000112740000 CR4: 0000000000750ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Kernel panic - not syncing: Fatal exception
--- drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c +7 -2
@@ -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; + } ifp = drvr->iflist[emsg.bsscfgidx]; + } err =brcmf_fweh_call_event_handler (drvr, ifp, event->code, &emsg, event->data);if (err) {
Signature · 1 line
2.25.1
Quoted 10 lines · Dokyung Song
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
[...]
Quoted 5 lines
Kernel panic - not syncing: Fatal exceptionReported-by Dokyung Song <[email protected]>Reported-by Jisoo Jang <[email protected]>Reported-by Minsuk Kang <[email protected]>
Not sure what the rules are for using 'Reported-by' tag, but it looks a bit odd. I leave it to the wireless maintainer.
Quoted 12 lines
Signed-off-by Dokyung Song <[email protected]>--- drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c +0
-0@@ -255,10 +255,15 @@ static void brcmf_fweh_event_worker(struct work_struct *work) goto event_free; }
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c static void brcmf_fweh_event_worker(struct work_struct *work) · line 264
Quoted 10 lines
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c +6
-2-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
Quoted 5 lines
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c +1
-0ifp = drvr->iflist[emsg.bsscfgidx]; + } err =brcmf_fweh_call_event_handler (drvr, ifp, event->code, &emsg, event->data);if (err) {
Quoted 11 lines · Arend Van Spriel
Quoted 7 lines
- else
+ } else {
+ if (emsg.bsscfgidx >=BRCMF_MAX_IFS) { +bphy_err(drvr, "invalid bsscfg index: %u\n", emsg.bsscfgidx);
+ gotoevent_free;
+ }probably better to do the validation before any other handling. So right after converting the event message at line 245
Actually, if we ignore the event message we can avoid the whole conversion as well doing the validation before the convert.
Regards,
Arend