Re: KASAN: invalid-access Write in enqueue_timer

Re: KASAN: invalid-access Write in enqueue_timer

[Jason A. Donenfeld]

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

Re: KASAN: invalid-access Write in enqueue_timer

[Jason A. Donenfeld]

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?

Re: KASAN: invalid-access Write in enqueue_timer

[Catalin Marinas]

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

Re: KASAN: invalid-access Write in enqueue_timer

[Dmitry Vyukov]

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

Re: KASAN: invalid-access Write in enqueue_timer

[Dmitry Vyukov]

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.