Hello,
syzbot found the following issue on:
HEAD commit: 0fb2c41f Merge 5.10-rc4 into here.
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
console output: https://syzkaller.appspot.com/x/log.txt?x=15746da1500000
kernel config: https://syzkaller.appspot.com/x/.config?x=b99cde3c953e8f6e
dashboard link: https://syzkaller.appspot.com/bug?extid=03110230a11411024147
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12cc9bba500000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=120b1cc2500000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]
==================================================================
BUG: KASAN: use-after-free in memcpy include/linux/string.h:399 [inline]
BUG: KASAN: use-after-free in ath9k_hif_usb_rx_stream drivers/net/wireless/ath/ath9k/hif_usb.c:562 [inline]
BUG: KASAN: use-after-free in ath9k_hif_usb_rx_cb+0x3ab/0x1020 drivers/net/wireless/ath/ath9k/hif_usb.c:680
Read of size 49146 at addr ffff888113938000 by task swapper/1/0
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.10.0-rc4-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x107/0x163 lib/dump_stack.c:118
print_address_description.constprop.0.cold+0xae/0x4c8 mm/kasan/report.c:385
__kasan_report mm/kasan/report.c:545 [inline]
kasan_report.cold+0x1f/0x37 mm/kasan/report.c:562
check_memory_region_inline mm/kasan/generic.c:186 [inline]
check_memory_region+0x13d/0x180 mm/kasan/generic.c:192
memcpy+0x20/0x60 mm/kasan/common.c:105
memcpy include/linux/string.h:399 [inline]
ath9k_hif_usb_rx_stream drivers/net/wireless/ath/ath9k/hif_usb.c:562 [inline]
ath9k_hif_usb_rx_cb+0x3ab/0x1020 drivers/net/wireless/ath/ath9k/hif_usb.c:680
__usb_hcd_giveback_urb+0x2b0/0x5c0 drivers/usb/core/hcd.c:1657
usb_hcd_giveback_urb+0x367/0x410 drivers/usb/core/hcd.c:1728
dummy_timer+0x11f4/0x3280 drivers/usb/gadget/udc/dummy_hcd.c:1969
call_timer_fn+0x1a5/0x630 kernel/time/timer.c:1410
expire_timers kernel/time/timer.c:1455 [inline]
__run_timers.part.0+0x67c/0xa10 kernel/time/timer.c:1747
__run_timers kernel/time/timer.c:1728 [inline]
run_timer_softirq+0x80/0x120 kernel/time/timer.c:1760
__do_softirq+0x1b2/0x945 kernel/softirq.c:298
asm_call_irq_on_stack+0xf/0x20
</IRQ>
__run_on_irqstack arch/x86/include/asm/irq_stack.h:26 [inline]
run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:77 [inline]
do_softirq_own_stack+0x80/0xa0 arch/x86/kernel/irq_64.c:77
invoke_softirq kernel/softirq.c:393 [inline]
__irq_exit_rcu kernel/softirq.c:423 [inline]
irq_exit_rcu+0x110/0x1a0 kernel/softirq.c:435
sysvec_apic_timer_interrupt+0x43/0xa0 arch/x86/kernel/apic/apic.c:1091
asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:631
RIP: 0010:native_save_fl arch/x86/include/asm/irqflags.h:29 [inline]
RIP: 0010:arch_local_save_flags arch/x86/include/asm/irqflags.h:79 [inline]
RIP: 0010:arch_irqs_disabled arch/x86/include/asm/irqflags.h:169 [inline]
RIP: 0010:acpi_safe_halt drivers/acpi/processor_idle.c:112 [inline]
RIP: 0010:acpi_idle_do_entry+0x1c9/0x250 drivers/acpi/processor_idle.c:517
Code: 8d d1 a1 fb 84 db 75 ac e8 34 d9 a1 fb e8 df 7f a7 fb e9 0c 00 00 00 e8 25 d9 a1 fb 0f 00 2d 6e 7b 6a 00 e8 19 d9 a1 fb fb f4 <9c> 5b 81 e3 00 02 00 00 fa 31 ff 48 89 de e8 b4 d1 a1 fb 48 85 db
RSP: 0018:ffffc900000dfd18 EFLAGS: 00000293
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 1ffffffff10796c9
RDX: ffff888100293280 RSI: ffffffff859d0937 RDI: ffffffff859d0921
RBP: ffff8881031b7864 R08: 0000000000000001 R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000001
R13: ffff8881031b7800 R14: ffff8881031b7864 R15: ffff888105aee804
acpi_idle_enter+0x355/0x4f0 drivers/acpi/processor_idle.c:648
cpuidle_enter_state+0x1b1/0xc80 drivers/cpuidle/cpuidle.c:237
cpuidle_enter+0x4a/0xa0 drivers/cpuidle/cpuidle.c:351
call_cpuidle kernel/sched/idle.c:132 [inline]
cpuidle_idle_call kernel/sched/idle.c:213 [inline]
do_idle+0x3d5/0x580 kernel/sched/idle.c:273
cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:369
start_secondary+0x265/0x340 arch/x86/kernel/smpboot.c:266
secondary_startup_64_no_verify+0xb0/0xbb
The buggy address belongs to the page:
page:00000000d417cdb1 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x113938
head:00000000d417cdb1 order:3 compound_mapcount:0 compound_pincount:0
flags: 0x200000000010000(head)
raw: 0200000000010000 dead000000000100 dead000000000122 0000000000000000
raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88811393ff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff88811393ff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff888113940000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff888113940080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888113940100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at [email protected].
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
syzbot has bisected this issue to:
commit dcd479e10a0510522a5d88b29b8f79ea3467d501
Author: Johannes Berg <[email protected]>
Date: Fri Oct 9 12:17:11 2020 +0000
mac80211: always wind down STA state
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=100c9c16500000
start commit: 0fa8ee0d Merge branch 'for-linus' of git://git.kernel.org/..
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=120c9c16500000
console output: https://syzkaller.appspot.com/x/log.txt?x=140c9c16500000
kernel config: https://syzkaller.appspot.com/x/.config?x=75292221eb79ace2
dashboard link: https://syzkaller.appspot.com/bug?extid=03110230a11411024147
userspace arch: i386
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1587f841500000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ec0fe6500000
Reported-by: [email protected]
Fixes: dcd479e10a05 ("mac80211: always wind down STA state")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
Although hif_dev->htc_handle is allocated by ath9k_htc_hw_alloc() from
ath9k_hif_usb_firmware_cb(), hif_dev->htc_handle->drv_priv is not assigned
until ieee80211_alloc_hw() from ath9k_htc_probe_device() from
ath9k_htc_hw_init() from ath9k_hif_usb_firmware_cb() returns. However, as
soon as ath9k_hif_usb_alloc_rx_urbs() from ath9k_hif_usb_alloc_urbs() from
ath9k_hif_usb_dev_init() from ath9k_hif_usb_firmware_cb() returns, a timer
interrupt can access hif_dev->htc_handle->drv_priv via RX_STAT_INC() from
ath9k_hif_usb_rx_stream() from ath9k_hif_usb_rx_cb() from
usb_hcd_giveback_urb(), which results in NULL pointer deference problem.
Also, even after htc_handle->drv_priv is assigned, when
ath9k_htc_wait_for_target() from ath9k_htc_probe_device() from
ath9k_htc_hw_init() from ath9k_hif_usb_firmware_cb() fails,
ieee80211_free_hw() (which does not reset hif_dev->htc_handle->drv_priv)
is immediately called due to "goto err_free;". As a result, a timer
interrupt which happens after ieee80211_free_hw() triggers use-after-free
problem at the abovementioned location.
We can flush in-flight requests by calling ath9k_hif_usb_dealloc_urbs()
before calling ieee80211_free_hw(). But we need to take from two choices
for not yet assigned case. One is to change e.g. RX_STAT_INC() to check
for NULL because it depends on CONFIG_ATH9K_HTC_DEBUGFS=y. The other is to
assign a dummy object which is used until initialization. This patch took
the latter.
Link: https://syzkaller.appspot.com/bug?extid=03110230a11411024147
Reported-by: syzbot <[email protected]>
Signed-off-by: Tetsuo Handa <[email protected]>
Tested-by: syzbot <[email protected]>
---
Pavel Skripkin has tested "check for NULL" approach, but not yet accepted.
What was wrong with Pavel's approach?
drivers/net/wireless/ath/ath9k/htc_drv_init.c | 6 +++---
drivers/net/wireless/ath/ath9k/htc_hst.c | 5 +++++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index ff61ae34ecdf..e497a44aff88 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -931,7 +931,6 @@ static int ath9k_init_device(struct ath9k_htc_priv *priv,
int ath9k_htc_probe_device(struct htc_target *htc_handle, struct device *dev,
u16 devid, char *product, u32 drv_info)
{
- struct hif_device_usb *hif_dev;
struct ath9k_htc_priv *priv;
struct ieee80211_hw *hw;
int ret;
@@ -969,10 +968,11 @@ int ath9k_htc_probe_device(struct htc_target *htc_handle, struct device *dev,
err_init:
ath9k_stop_wmi(priv);
- hif_dev = (struct hif_device_usb *)htc_handle->hif_dev;
- ath9k_hif_usb_dealloc_urbs(hif_dev);
+ ath9k_hif_usb_dealloc_urbs((struct hif_device_usb *)htc_handle->hif_dev);
ath9k_destroy_wmi(priv);
err_free:
+ ath9k_hif_usb_dealloc_urbs((struct hif_device_usb *)htc_handle->hif_dev);
+ htc_handle->drv_priv = NULL;
ieee80211_free_hw(hw);
return ret;
}
diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
index 994ec48b2f66..d461eca389ab 100644
--- a/drivers/net/wireless/ath/ath9k/htc_hst.c
+++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
@@ -468,6 +468,9 @@ void ath9k_htc_rx_msg(struct htc_target *htc_handle,
}
}
+/* A dummy object used until device is initialized. */
+static struct ath9k_htc_priv ath9k_uninitialized_htc_priv;
+
struct htc_target *ath9k_htc_hw_alloc(void *hif_handle,
struct ath9k_htc_hif *hif,
struct device *dev)
@@ -493,6 +496,8 @@ struct htc_target *ath9k_htc_hw_alloc(void *hif_handle,
atomic_set(&target->tgt_ready, 0);
+ target->drv_priv = &ath9k_uninitialized_htc_priv;
+
return target;
}
--
2.34.1
Hi Tetsuo,
On 4/29/22 14:18, Tetsuo Handa wrote:
> Although hif_dev->htc_handle is allocated by ath9k_htc_hw_alloc() from
> ath9k_hif_usb_firmware_cb(), hif_dev->htc_handle->drv_priv is not assigned
> until ieee80211_alloc_hw() from ath9k_htc_probe_device() from
> ath9k_htc_hw_init() from ath9k_hif_usb_firmware_cb() returns. However, as
> soon as ath9k_hif_usb_alloc_rx_urbs() from ath9k_hif_usb_alloc_urbs() from
> ath9k_hif_usb_dev_init() from ath9k_hif_usb_firmware_cb() returns, a timer
> interrupt can access hif_dev->htc_handle->drv_priv via RX_STAT_INC() from
> ath9k_hif_usb_rx_stream() from ath9k_hif_usb_rx_cb() from
> usb_hcd_giveback_urb(), which results in NULL pointer deference problem.
>
> Also, even after htc_handle->drv_priv is assigned, when
> ath9k_htc_wait_for_target() from ath9k_htc_probe_device() from
> ath9k_htc_hw_init() from ath9k_hif_usb_firmware_cb() fails,
> ieee80211_free_hw() (which does not reset hif_dev->htc_handle->drv_priv)
> is immediately called due to "goto err_free;". As a result, a timer
> interrupt which happens after ieee80211_free_hw() triggers use-after-free
> problem at the abovementioned location.
>
> We can flush in-flight requests by calling ath9k_hif_usb_dealloc_urbs()
> before calling ieee80211_free_hw(). But we need to take from two choices
> for not yet assigned case. One is to change e.g. RX_STAT_INC() to check
> for NULL because it depends on CONFIG_ATH9K_HTC_DEBUGFS=y. The other is to
> assign a dummy object which is used until initialization. This patch took
> the latter.
>
> Link: https://syzkaller.appspot.com/bug?extid=03110230a11411024147
> Reported-by: syzbot <[email protected]>
> Signed-off-by: Tetsuo Handa <[email protected]>
> Tested-by: syzbot <[email protected]>
> ---
> Pavel Skripkin has tested "check for NULL" approach, but not yet accepted.
> What was wrong with Pavel's approach?
>
I don't know. IIRC the problem is that nobody has tested my patch on
real hw, so they can't accept it as-is. And maybe it just got lost
You can check out [1] thread. It's the latest version I have posted
[1]
https://lore.kernel.org/all/80962aae265995d1cdb724f5362c556d494c7566.1644265120.git.paskripkin@gmail.com/
With regards,
Pavel Skripkin