Hi Catalin,
On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas [off-list ref] 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.
quoted
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
quoted
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,
quoted
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
On Tue, Feb 16, 2021 at 6:46 PM Jason A. Donenfeld [off-list ref] wrote:
Hi Catalin,
On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas [off-list ref] wrote:
quoted
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.
quoted
quoted
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
quoted
quoted
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,
quoted
quoted
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?
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 [off-list ref] wrote:
quoted
On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas [off-list ref] wrote:
quoted
quoted
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
On Tue, Feb 16, 2021 at 7:01 PM Catalin Marinas [off-list ref] wrote:
On Tue, Feb 16, 2021 at 06:50:20PM +0100, Jason A. Donenfeld wrote:
quoted
On Tue, Feb 16, 2021 at 6:46 PM Jason A. Donenfeld [off-list ref] wrote:
quoted
On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas [off-list ref] wrote:
quoted
quoted
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.
quoted
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
[...]
quoted
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
On Tue, Feb 16, 2021 at 7:15 PM Dmitry Vyukov [off-list ref] wrote:
quoted
On Tue, Feb 16, 2021 at 06:50:20PM +0100, Jason A. Donenfeld wrote:
quoted
On Tue, Feb 16, 2021 at 6:46 PM Jason A. Donenfeld [off-list ref] wrote:
quoted
On Tue, Feb 16, 2021 at 6:28 PM Catalin Marinas [off-list ref] wrote:
quoted
quoted
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.
quoted
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
[...]
quoted
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.