| Message ID | YXsidrRuK6zBJicZ@10-18-43-117.dynapool.wireless.nyu.edu (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6ce708f54cc8d73beca213cec66ede5ce100a781 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath9k: Fix out-of-bound memcpy in ath9k_hif_usb_rx_stream | expand |
Zekun Shen <bruceshenzk@gmail.com> writes: > Large pkt_len can lead to out-out-bound memcpy. Current > ath9k_hif_usb_rx_stream allows combining the content of two urb > inputs to one pkt. The first input can indicate the size of the > pkt. Any remaining size is saved in hif_dev->rx_remain_len. > While processing the next input, memcpy is used with rx_remain_len. > > 4-byte pkt_len can go up to 0xffff, while a single input is 0x4000 > maximum in size (MAX_RX_BUF_SIZE). Thus, the patch adds a check for > pkt_len which must not exceed 2 * MAX_RX_BUG_SIZE. > > BUG: KASAN: slab-out-of-bounds in ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > Read of size 46393 at addr ffff888018798000 by task kworker/0:1/23 > > CPU: 0 PID: 23 Comm: kworker/0:1 Not tainted 5.6.0 #63 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014 > Workqueue: events request_firmware_work_func > Call Trace: > <IRQ> > dump_stack+0x76/0xa0 > print_address_description.constprop.0+0x16/0x200 > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > __kasan_report.cold+0x37/0x7c > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > kasan_report+0xe/0x20 > check_memory_region+0x15a/0x1d0 > memcpy+0x20/0x50 > ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > ? hif_usb_mgmt_cb+0x2d9/0x2d9 [ath9k_htc] > ? _raw_spin_lock_irqsave+0x7b/0xd0 > ? _raw_spin_trylock_bh+0x120/0x120 > ? __usb_unanchor_urb+0x12f/0x210 > __usb_hcd_giveback_urb+0x1e4/0x380 > usb_giveback_urb_bh+0x241/0x4f0 > ? __hrtimer_run_queues+0x316/0x740 > ? __usb_hcd_giveback_urb+0x380/0x380 > tasklet_action_common.isra.0+0x135/0x330 > __do_softirq+0x18c/0x634 > irq_exit+0x114/0x140 > smp_apic_timer_interrupt+0xde/0x380 > apic_timer_interrupt+0xf/0x20 > > Signed-off-by: Zekun Shen <bruceshenzk@gmail.com> How did you test this?
On Fri, Oct 29, 2021 at 06:52:31AM +0300, Kalle Valo wrote: > Zekun Shen <bruceshenzk@gmail.com> writes: > > > Large pkt_len can lead to out-out-bound memcpy. Current > > ath9k_hif_usb_rx_stream allows combining the content of two urb > > inputs to one pkt. The first input can indicate the size of the > > pkt. Any remaining size is saved in hif_dev->rx_remain_len. > > While processing the next input, memcpy is used with rx_remain_len. > > > > 4-byte pkt_len can go up to 0xffff, while a single input is 0x4000 > > maximum in size (MAX_RX_BUF_SIZE). Thus, the patch adds a check for > > pkt_len which must not exceed 2 * MAX_RX_BUG_SIZE. > > > > BUG: KASAN: slab-out-of-bounds in ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > > Read of size 46393 at addr ffff888018798000 by task kworker/0:1/23 > > > > CPU: 0 PID: 23 Comm: kworker/0:1 Not tainted 5.6.0 #63 > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > > BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014 > > Workqueue: events request_firmware_work_func > > Call Trace: > > <IRQ> > > dump_stack+0x76/0xa0 > > print_address_description.constprop.0+0x16/0x200 > > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > > __kasan_report.cold+0x37/0x7c > > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > > kasan_report+0xe/0x20 > > check_memory_region+0x15a/0x1d0 > > memcpy+0x20/0x50 > > ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > > ? hif_usb_mgmt_cb+0x2d9/0x2d9 [ath9k_htc] > > ? _raw_spin_lock_irqsave+0x7b/0xd0 > > ? _raw_spin_trylock_bh+0x120/0x120 > > ? __usb_unanchor_urb+0x12f/0x210 > > __usb_hcd_giveback_urb+0x1e4/0x380 > > usb_giveback_urb_bh+0x241/0x4f0 > > ? __hrtimer_run_queues+0x316/0x740 > > ? __usb_hcd_giveback_urb+0x380/0x380 > > tasklet_action_common.isra.0+0x135/0x330 > > __do_softirq+0x18c/0x634 > > irq_exit+0x114/0x140 > > smp_apic_timer_interrupt+0xde/0x380 > > apic_timer_interrupt+0xf/0x20 > > > > Signed-off-by: Zekun Shen <bruceshenzk@gmail.com> > > How did you test this? I found the bug using a custome USBFuzz port. It's a research work to fuzz USB stack/drivers. I modified it to fuzz ath9k driver only, providing hand-crafted usb descriptors to QEMU. After fixing the value of pkt_tag to ATH_USB_RX_STREAM_MODE_TAG in QEMU emulation, I found the KASAN report. The bug is triggerable whenever pkt_len is above two MAX_RX_BUG_SIZE. I used the same input that crashes to test the driver works when applying the patch. > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Zekun Shen <bruceshenzk@gmail.com> wrote: > Large pkt_len can lead to out-out-bound memcpy. Current > ath9k_hif_usb_rx_stream allows combining the content of two urb > inputs to one pkt. The first input can indicate the size of the > pkt. Any remaining size is saved in hif_dev->rx_remain_len. > While processing the next input, memcpy is used with rx_remain_len. > > 4-byte pkt_len can go up to 0xffff, while a single input is 0x4000 > maximum in size (MAX_RX_BUF_SIZE). Thus, the patch adds a check for > pkt_len which must not exceed 2 * MAX_RX_BUG_SIZE. > > BUG: KASAN: slab-out-of-bounds in ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > Read of size 46393 at addr ffff888018798000 by task kworker/0:1/23 > > CPU: 0 PID: 23 Comm: kworker/0:1 Not tainted 5.6.0 #63 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014 > Workqueue: events request_firmware_work_func > Call Trace: > <IRQ> > dump_stack+0x76/0xa0 > print_address_description.constprop.0+0x16/0x200 > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > __kasan_report.cold+0x37/0x7c > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > kasan_report+0xe/0x20 > check_memory_region+0x15a/0x1d0 > memcpy+0x20/0x50 > ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > ? hif_usb_mgmt_cb+0x2d9/0x2d9 [ath9k_htc] > ? _raw_spin_lock_irqsave+0x7b/0xd0 > ? _raw_spin_trylock_bh+0x120/0x120 > ? __usb_unanchor_urb+0x12f/0x210 > __usb_hcd_giveback_urb+0x1e4/0x380 > usb_giveback_urb_bh+0x241/0x4f0 > ? __hrtimer_run_queues+0x316/0x740 > ? __usb_hcd_giveback_urb+0x380/0x380 > tasklet_action_common.isra.0+0x135/0x330 > __do_softirq+0x18c/0x634 > irq_exit+0x114/0x140 > smp_apic_timer_interrupt+0xde/0x380 > apic_timer_interrupt+0xf/0x20 > > Signed-off-by: Zekun Shen <bruceshenzk@gmail.com> I need to test this myself. Patch set to Deferred.
Zekun Shen <bruceshenzk@gmail.com> wrote: > Large pkt_len can lead to out-out-bound memcpy. Current > ath9k_hif_usb_rx_stream allows combining the content of two urb > inputs to one pkt. The first input can indicate the size of the > pkt. Any remaining size is saved in hif_dev->rx_remain_len. > While processing the next input, memcpy is used with rx_remain_len. > > 4-byte pkt_len can go up to 0xffff, while a single input is 0x4000 > maximum in size (MAX_RX_BUF_SIZE). Thus, the patch adds a check for > pkt_len which must not exceed 2 * MAX_RX_BUG_SIZE. > > BUG: KASAN: slab-out-of-bounds in ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > Read of size 46393 at addr ffff888018798000 by task kworker/0:1/23 > > CPU: 0 PID: 23 Comm: kworker/0:1 Not tainted 5.6.0 #63 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014 > Workqueue: events request_firmware_work_func > Call Trace: > <IRQ> > dump_stack+0x76/0xa0 > print_address_description.constprop.0+0x16/0x200 > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > __kasan_report.cold+0x37/0x7c > ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > kasan_report+0xe/0x20 > check_memory_region+0x15a/0x1d0 > memcpy+0x20/0x50 > ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] > ? hif_usb_mgmt_cb+0x2d9/0x2d9 [ath9k_htc] > ? _raw_spin_lock_irqsave+0x7b/0xd0 > ? _raw_spin_trylock_bh+0x120/0x120 > ? __usb_unanchor_urb+0x12f/0x210 > __usb_hcd_giveback_urb+0x1e4/0x380 > usb_giveback_urb_bh+0x241/0x4f0 > ? __hrtimer_run_queues+0x316/0x740 > ? __usb_hcd_giveback_urb+0x380/0x380 > tasklet_action_common.isra.0+0x135/0x330 > __do_softirq+0x18c/0x634 > irq_exit+0x114/0x140 > smp_apic_timer_interrupt+0xde/0x380 > apic_timer_interrupt+0xf/0x20 > > I found the bug using a custome USBFuzz port. It's a research work > to fuzz USB stack/drivers. I modified it to fuzz ath9k driver only, > providing hand-crafted usb descriptors to QEMU. > > After fixing the value of pkt_tag to ATH_USB_RX_STREAM_MODE_TAG in QEMU > emulation, I found the KASAN report. The bug is triggerable whenever > pkt_len is above two MAX_RX_BUG_SIZE. I used the same input that crashes > to test the driver works when applying the patch. > > Signed-off-by: Zekun Shen <bruceshenzk@gmail.com> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> Patch applied to ath-next branch of ath.git, thanks. 6ce708f54cc8 ath9k: Fix out-of-bound memcpy in ath9k_hif_usb_rx_stream
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c index 860da13bf..0681bc5fa 100644 --- a/drivers/net/wireless/ath/ath9k/hif_usb.c +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c @@ -589,6 +589,12 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev, RX_STAT_INC(skb_dropped); return; } + if (pkt_len > 2 * MAX_RX_BUF_SIZE) { + dev_err(&hif_dev->udev->dev, + "ath9k_htc: invalid pkt_len (%x)\n", pkt_len); + RX_STAT_INC(skb_dropped); + return; + } pad_len = 4 - (pkt_len & 0x3); if (pad_len == 4)
Large pkt_len can lead to out-out-bound memcpy. Current ath9k_hif_usb_rx_stream allows combining the content of two urb inputs to one pkt. The first input can indicate the size of the pkt. Any remaining size is saved in hif_dev->rx_remain_len. While processing the next input, memcpy is used with rx_remain_len. 4-byte pkt_len can go up to 0xffff, while a single input is 0x4000 maximum in size (MAX_RX_BUF_SIZE). Thus, the patch adds a check for pkt_len which must not exceed 2 * MAX_RX_BUG_SIZE. BUG: KASAN: slab-out-of-bounds in ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] Read of size 46393 at addr ffff888018798000 by task kworker/0:1/23 CPU: 0 PID: 23 Comm: kworker/0:1 Not tainted 5.6.0 #63 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014 Workqueue: events request_firmware_work_func Call Trace: <IRQ> dump_stack+0x76/0xa0 print_address_description.constprop.0+0x16/0x200 ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] __kasan_report.cold+0x37/0x7c ? ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] kasan_report+0xe/0x20 check_memory_region+0x15a/0x1d0 memcpy+0x20/0x50 ath9k_hif_usb_rx_cb+0x490/0xed7 [ath9k_htc] ? hif_usb_mgmt_cb+0x2d9/0x2d9 [ath9k_htc] ? _raw_spin_lock_irqsave+0x7b/0xd0 ? _raw_spin_trylock_bh+0x120/0x120 ? __usb_unanchor_urb+0x12f/0x210 __usb_hcd_giveback_urb+0x1e4/0x380 usb_giveback_urb_bh+0x241/0x4f0 ? __hrtimer_run_queues+0x316/0x740 ? __usb_hcd_giveback_urb+0x380/0x380 tasklet_action_common.isra.0+0x135/0x330 __do_softirq+0x18c/0x634 irq_exit+0x114/0x140 smp_apic_timer_interrupt+0xde/0x380 apic_timer_interrupt+0xf/0x20 Signed-off-by: Zekun Shen <bruceshenzk@gmail.com> --- drivers/net/wireless/ath/ath9k/hif_usb.c | 6 ++++++ 1 file changed, 6 insertions(+)