* Re: KASAN: invalid-access Write in enqueue_timer [not found] ` <20210216172817.GA14978@arm.com> @ 2021-02-16 17:46 ` Jason A. Donenfeld 2021-02-16 17:50 ` Jason A. Donenfeld 0 siblings, 1 reply; 5+ messages in thread From: Jason A. Donenfeld @ 2021-02-16 17:46 UTC (permalink / raw) To: Netdev Cc: syzbot, Mark Brown, Kees Cook, linux-arm-kernel, LKML, Mark Rutland, mbenes, syzkaller-bugs, Will Deacon, Ard Biesheuvel, Catalin Marinas Hi Catalin, On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > Adding Jason and Ard. It may be a use-after-free in the wireguard > driver. Thanks for sending this my way. Note: to my knowledge, Ard doesn't work on wireguard. > > hlist_add_head include/linux/list.h:883 [inline] > > enqueue_timer+0x18/0xc0 kernel/time/timer.c:581 > > mod_timer+0x14/0x20 kernel/time/timer.c:1106 > > mod_peer_timer drivers/net/wireguard/timers.c:37 [inline] > > wg_timers_any_authenticated_packet_traversal+0x68/0x90 drivers/net/wireguard/timers.c:215 The line of hlist_add_head that it's hitting is: static inline void hlist_add_head(struct hlist_node *n, struct hlist_head *h) { struct hlist_node *first = h->first; WRITE_ONCE(n->next, first); if (first) So that means it's the dereferencing of h that's a problem. That comes from: static void enqueue_timer(struct timer_base *base, struct timer_list *timer, unsigned int idx, unsigned long bucket_expiry) { hlist_add_head(&timer->entry, base->vectors + idx); That means it concerns base->vectors + idx, not the timer_list object that wireguard manages. That's confusing. Could that imply that the bug is in freeing a previous timer without removing it from the timer lists, so that it winds up being in base->vectors? The allocation and deallocation backtrace is confusing > > alloc_netdev_mqs+0x5c/0x3bc net/core/dev.c:10546 > > rtnl_create_link+0xc8/0x2b0 net/core/rtnetlink.c:3171 > > __rtnl_newlink+0x5bc/0x800 net/core/rtnetlink.c:3433 This suggests it's part of the `ip link add wg0 type wireguard` nelink call, during it's allocation of the netdevice's private area. For this, the wg_device struct is used. It has no timer_list structures in it! Similarly, > > netdev_freemem+0x18/0x2c net/core/dev.c:10500 > > netdev_release+0x30/0x44 net/core/net-sysfs.c:1828 > > device_release+0x34/0x90 drivers/base/core.c:1980 That smells like `ip link del wg0 type wireguard`. But again, wg_device doesn't have any timer_lists in it. So what's happening here exactly? I'm not really sure yet... It'd be nice to have a reproducer. Jason ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: KASAN: invalid-access Write in enqueue_timer 2021-02-16 17:46 ` KASAN: invalid-access Write in enqueue_timer Jason A. Donenfeld @ 2021-02-16 17:50 ` Jason A. Donenfeld 2021-02-16 18:01 ` Catalin Marinas 0 siblings, 1 reply; 5+ messages in thread From: Jason A. Donenfeld @ 2021-02-16 17:50 UTC (permalink / raw) To: Netdev Cc: syzbot, Mark Brown, Kees Cook, linux-arm-kernel, LKML, Mark Rutland, mbenes, syzkaller-bugs, Will Deacon, Ard Biesheuvel, Catalin Marinas On Tue, Feb 16, 2021 at 6:46 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi Catalin, > > On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > > Adding Jason and Ard. It may be a use-after-free in the wireguard > > driver. > > Thanks for sending this my way. Note: to my knowledge, Ard doesn't > work on wireguard. > > > > hlist_add_head include/linux/list.h:883 [inline] > > > enqueue_timer+0x18/0xc0 kernel/time/timer.c:581 > > > mod_timer+0x14/0x20 kernel/time/timer.c:1106 > > > mod_peer_timer drivers/net/wireguard/timers.c:37 [inline] > > > wg_timers_any_authenticated_packet_traversal+0x68/0x90 drivers/net/wireguard/timers.c:215 > > The line of hlist_add_head that it's hitting is: > > static inline void hlist_add_head(struct hlist_node *n, struct hlist_head *h) > { > struct hlist_node *first = h->first; > WRITE_ONCE(n->next, first); > if (first) > > So that means it's the dereferencing of h that's a problem. That comes from: > > static void enqueue_timer(struct timer_base *base, struct timer_list *timer, > unsigned int idx, unsigned long bucket_expiry) > { > > hlist_add_head(&timer->entry, base->vectors + idx); > > That means it concerns base->vectors + idx, not the timer_list object > that wireguard manages. That's confusing. Could that imply that the > bug is in freeing a previous timer without removing it from the timer > lists, so that it winds up being in base->vectors? > > The allocation and deallocation backtrace is confusing > > > > alloc_netdev_mqs+0x5c/0x3bc net/core/dev.c:10546 > > > rtnl_create_link+0xc8/0x2b0 net/core/rtnetlink.c:3171 > > > __rtnl_newlink+0x5bc/0x800 net/core/rtnetlink.c:3433 > > This suggests it's part of the `ip link add wg0 type wireguard` nelink > call, during it's allocation of the netdevice's private area. For > this, the wg_device struct is used. It has no timer_list structures in > it! > > Similarly, > > > > netdev_freemem+0x18/0x2c net/core/dev.c:10500 > > > netdev_release+0x30/0x44 net/core/net-sysfs.c:1828 > > > device_release+0x34/0x90 drivers/base/core.c:1980 > > That smells like `ip link del wg0 type wireguard`. But again, > wg_device doesn't have any timer_lists in it. > > So what's happening here exactly? I'm not really sure yet... > > It'd be nice to have a reproducer. > > > Jason Digging around on syzkaller, it looks like there's a similar bug on jbd2, concerning iptunnels's allocation: https://syzkaller.appspot.com/text?tag=CrashReport&x=13afb19cd00000 And one from ext4: https://syzkaller.appspot.com/text?tag=CrashReport&x=17685330d00000 And from from ext4 with fddup: https://syzkaller.appspot.com/text?tag=CrashReport&x=17685330d00000 https://syzkaller.appspot.com/text?tag=CrashReport&x=12d326e8d00000 It might not actually be a wireguard bug? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: KASAN: invalid-access Write in enqueue_timer 2021-02-16 17:50 ` Jason A. Donenfeld @ 2021-02-16 18:01 ` Catalin Marinas 2021-02-16 18:15 ` Dmitry Vyukov 0 siblings, 1 reply; 5+ messages in thread From: Catalin Marinas @ 2021-02-16 18:01 UTC (permalink / raw) To: Jason A. Donenfeld Cc: Netdev, syzbot, Mark Brown, Kees Cook, linux-arm-kernel, LKML, Mark Rutland, mbenes, syzkaller-bugs, Will Deacon, Ard Biesheuvel On Tue, Feb 16, 2021 at 06:50:20PM +0100, Jason A. Donenfeld wrote: > On Tue, Feb 16, 2021 at 6:46 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > > > > hlist_add_head include/linux/list.h:883 [inline] > > > > enqueue_timer+0x18/0xc0 kernel/time/timer.c:581 > > > > mod_timer+0x14/0x20 kernel/time/timer.c:1106 > > > > mod_peer_timer drivers/net/wireguard/timers.c:37 [inline] > > > > wg_timers_any_authenticated_packet_traversal+0x68/0x90 drivers/net/wireguard/timers.c:215 > > > > The line of hlist_add_head that it's hitting is: > > > > static inline void hlist_add_head(struct hlist_node *n, struct hlist_head *h) > > { > > struct hlist_node *first = h->first; > > WRITE_ONCE(n->next, first); > > if (first) > > > > So that means it's the dereferencing of h that's a problem. That comes from: > > > > static void enqueue_timer(struct timer_base *base, struct timer_list *timer, > > unsigned int idx, unsigned long bucket_expiry) > > { > > > > hlist_add_head(&timer->entry, base->vectors + idx); > > > > That means it concerns base->vectors + idx, not the timer_list object > > that wireguard manages. That's confusing. Could that imply that the > > bug is in freeing a previous timer without removing it from the timer > > lists, so that it winds up being in base->vectors? Good point, it's indeed likely that the timer list is messed up already, just an unlucky encounter in the wireguard code. > Digging around on syzkaller, it looks like there's a similar bug on > jbd2, concerning iptunnels's allocation: > > https://syzkaller.appspot.com/text?tag=CrashReport&x=13afb19cd00000 [...] > It might not actually be a wireguard bug? I wonder whether syzbot reported similar issues with CONFIG_KASAN_SW_TAGS. It shouldn't be that different from the HW_TAGS but at least we can rule out qemu bugs with the MTE emulation. -- Catalin ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: KASAN: invalid-access Write in enqueue_timer 2021-02-16 18:01 ` Catalin Marinas @ 2021-02-16 18:15 ` Dmitry Vyukov 2021-02-17 6:37 ` Dmitry Vyukov 0 siblings, 1 reply; 5+ messages in thread From: Dmitry Vyukov @ 2021-02-16 18:15 UTC (permalink / raw) To: Catalin Marinas, Eric Dumazet Cc: Jason A. Donenfeld, Netdev, syzbot, Mark Brown, Kees Cook, linux-arm-kernel, LKML, Mark Rutland, mbenes, syzkaller-bugs, Will Deacon, Ard Biesheuvel On Tue, Feb 16, 2021 at 7:01 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Tue, Feb 16, 2021 at 06:50:20PM +0100, Jason A. Donenfeld wrote: > > On Tue, Feb 16, 2021 at 6:46 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > > > > > hlist_add_head include/linux/list.h:883 [inline] > > > > > enqueue_timer+0x18/0xc0 kernel/time/timer.c:581 > > > > > mod_timer+0x14/0x20 kernel/time/timer.c:1106 > > > > > mod_peer_timer drivers/net/wireguard/timers.c:37 [inline] > > > > > wg_timers_any_authenticated_packet_traversal+0x68/0x90 drivers/net/wireguard/timers.c:215 > > > > > > The line of hlist_add_head that it's hitting is: > > > > > > static inline void hlist_add_head(struct hlist_node *n, struct hlist_head *h) > > > { > > > struct hlist_node *first = h->first; > > > WRITE_ONCE(n->next, first); > > > if (first) > > > > > > So that means it's the dereferencing of h that's a problem. That comes from: > > > > > > static void enqueue_timer(struct timer_base *base, struct timer_list *timer, > > > unsigned int idx, unsigned long bucket_expiry) > > > { > > > > > > hlist_add_head(&timer->entry, base->vectors + idx); > > > > > > That means it concerns base->vectors + idx, not the timer_list object > > > that wireguard manages. That's confusing. Could that imply that the > > > bug is in freeing a previous timer without removing it from the timer > > > lists, so that it winds up being in base->vectors? > > Good point, it's indeed likely that the timer list is messed up already, > just an unlucky encounter in the wireguard code. > > > Digging around on syzkaller, it looks like there's a similar bug on > > jbd2, concerning iptunnels's allocation: > > > > https://syzkaller.appspot.com/text?tag=CrashReport&x=13afb19cd00000 > [...] > > It might not actually be a wireguard bug? > > I wonder whether syzbot reported similar issues with > CONFIG_KASAN_SW_TAGS. It shouldn't be that different from the HW_TAGS > but at least we can rule out qemu bugs with the MTE emulation. +Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: KASAN: invalid-access Write in enqueue_timer 2021-02-16 18:15 ` Dmitry Vyukov @ 2021-02-17 6:37 ` Dmitry Vyukov 0 siblings, 0 replies; 5+ messages in thread From: Dmitry Vyukov @ 2021-02-17 6:37 UTC (permalink / raw) To: Catalin Marinas, Eric Dumazet Cc: Jason A. Donenfeld, Netdev, syzbot, Mark Brown, Kees Cook, linux-arm-kernel, LKML, Mark Rutland, mbenes, syzkaller-bugs, Will Deacon, Ard Biesheuvel On Tue, Feb 16, 2021 at 7:15 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Tue, Feb 16, 2021 at 06:50:20PM +0100, Jason A. Donenfeld wrote: > > > On Tue, Feb 16, 2021 at 6:46 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > > > > > > hlist_add_head include/linux/list.h:883 [inline] > > > > > > enqueue_timer+0x18/0xc0 kernel/time/timer.c:581 > > > > > > mod_timer+0x14/0x20 kernel/time/timer.c:1106 > > > > > > mod_peer_timer drivers/net/wireguard/timers.c:37 [inline] > > > > > > wg_timers_any_authenticated_packet_traversal+0x68/0x90 drivers/net/wireguard/timers.c:215 > > > > > > > > The line of hlist_add_head that it's hitting is: > > > > > > > > static inline void hlist_add_head(struct hlist_node *n, struct hlist_head *h) > > > > { > > > > struct hlist_node *first = h->first; > > > > WRITE_ONCE(n->next, first); > > > > if (first) > > > > > > > > So that means it's the dereferencing of h that's a problem. That comes from: > > > > > > > > static void enqueue_timer(struct timer_base *base, struct timer_list *timer, > > > > unsigned int idx, unsigned long bucket_expiry) > > > > { > > > > > > > > hlist_add_head(&timer->entry, base->vectors + idx); > > > > > > > > That means it concerns base->vectors + idx, not the timer_list object > > > > that wireguard manages. That's confusing. Could that imply that the > > > > bug is in freeing a previous timer without removing it from the timer > > > > lists, so that it winds up being in base->vectors? > > > > Good point, it's indeed likely that the timer list is messed up already, > > just an unlucky encounter in the wireguard code. > > > > > Digging around on syzkaller, it looks like there's a similar bug on > > > jbd2, concerning iptunnels's allocation: > > > > > > https://syzkaller.appspot.com/text?tag=CrashReport&x=13afb19cd00000 > > [...] > > > It might not actually be a wireguard bug? > > > > I wonder whether syzbot reported similar issues with > > CONFIG_KASAN_SW_TAGS. It shouldn't be that different from the HW_TAGS > > but at least we can rule out qemu bugs with the MTE emulation. > > +Eric I've seen some similar reports on other syzkaller instances. They all have similar alloc/free stacks, but different access stacks. This does not seem to be wireguard nor arm/mte related. It seems that something released the device prematurely, and then some innocent code gets a use-after-free. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-17 6:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <0000000000000be4d705bb68dfa7@google.com>
[not found] ` <20210216172817.GA14978@arm.com>
2021-02-16 17:46 ` KASAN: invalid-access Write in enqueue_timer Jason A. Donenfeld
2021-02-16 17:50 ` Jason A. Donenfeld
2021-02-16 18:01 ` Catalin Marinas
2021-02-16 18:15 ` Dmitry Vyukov
2021-02-17 6:37 ` Dmitry Vyukov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).