2023-05-08 17:11:49

by syzbot

[permalink] [raw]
Subject: [syzbot] [bpf?] [net?] WARNING: zero-size vmalloc in print_tainted

Hello,

syzbot found the following issue on:

HEAD commit: 457391b03803 Linux 6.3
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1697e9d4280000
kernel config: https://syzkaller.appspot.com/x/.config?x=385e197a58ca4afe
dashboard link: https://syzkaller.appspot.com/bug?extid=fae676d3cf469331fc89
compiler: arm-linux-gnueabi-gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
userspace arch: arm
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15916904280000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10910f18280000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/c35b5b2731d2/non_bootable_disk-457391b0.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/2a1bf3bafeb6/vmlinux-457391b0.xz
kernel image: https://storage.googleapis.com/syzbot-assets/21f1e3b4a5a9/zImage-457391b0.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]

------------[ cut here ]------------
WARNING: CPU: 1 PID: 2949 at mm/vmalloc.c:3132 __vmalloc_node_range+0x44c/0x584 mm/vmalloc.c:3132
Modules linked in:
Kernel panic - not syncing: kernel: panic_on_warn set ...
CPU: 1 PID: 2949 Comm: syz-executor398 Not tainted 6.3.0-syzkaller #0
Hardware name: ARM-Versatile Express
Backtrace:
[<817b2528>] (dump_backtrace) from [<817b261c>] (show_stack+0x18/0x1c arch/arm/kernel/traps.c:256)
r7:81d81ac0 r6:82422c04 r5:60000093 r4:81d901cc
[<817b2604>] (show_stack) from [<817cec84>] (__dump_stack lib/dump_stack.c:88 [inline])
[<817b2604>] (show_stack) from [<817cec84>] (dump_stack_lvl+0x48/0x54 lib/dump_stack.c:106)
[<817cec3c>] (dump_stack_lvl) from [<817ceca8>] (dump_stack+0x18/0x1c lib/dump_stack.c:113)
r5:00000000 r4:8264dd14
[<817cec90>] (dump_stack) from [<817b3110>] (panic+0x11c/0x36c kernel/panic.c:340)
[<817b2ff4>] (panic) from [<802422ec>] (print_tainted+0x0/0xa0 kernel/panic.c:236)
r3:8240c488 r2:00000001 r1:81d79fcc r0:81d81ac0
r7:80469ba0
[<80242268>] (check_panic_on_warn) from [<802424e0>] (__warn+0x7c/0x180 kernel/panic.c:673)
[<80242464>] (__warn) from [<802426bc>] (warn_slowpath_fmt+0xd8/0x1d8 kernel/panic.c:697)
r8:00000009 r7:00000c3c r6:81da5110 r5:8240c954 r4:822ab6bc
[<802425e8>] (warn_slowpath_fmt) from [<80469ba0>] (__vmalloc_node_range+0x44c/0x584 mm/vmalloc.c:3132)
r10:00000dc0 r9:8410d080 r8:83d04e80 r7:df800000 r6:00004000 r5:00000000
r4:00000000
[<80469754>] (__vmalloc_node_range) from [<80469db0>] (vmalloc_user+0x6c/0x74 mm/vmalloc.c:3359)
r10:00000126 r9:8410d080 r8:83d04e80 r7:00000000 r6:00000000 r5:842aa940
r4:00000000
[<80469d44>] (vmalloc_user) from [<81767778>] (xskq_create+0x74/0xc4 net/xdp/xsk_queue.c:39)
[<81767704>] (xskq_create) from [<81766c64>] (xsk_init_queue net/xdp/xsk.c:756 [inline])
[<81767704>] (xskq_create) from [<81766c64>] (xsk_setsockopt+0x1c0/0x2bc net/xdp/xsk.c:1080)
r7:83d04eac r6:83d04c00 r5:00000000 r4:00000003
[<81766aa8>] (xsk_setsockopt) from [<812f6720>] (__sys_setsockopt+0xd4/0x1c8 net/socket.c:2271)
r8:80200288 r7:00000126 r6:000118b0 r5:81766aa4 r4:844eb900
[<812f664c>] (__sys_setsockopt) from [<812f6830>] (__do_sys_setsockopt net/socket.c:2282 [inline])
[<812f664c>] (__sys_setsockopt) from [<812f6830>] (sys_setsockopt+0x1c/0x24 net/socket.c:2279)
r5:00000000 r4:00000020
[<812f6814>] (sys_setsockopt) from [<80200060>] (ret_fast_syscall+0x0/0x1c arch/arm/mm/proc-v7.S:66)
Exception stack(0xdf981fa8 to 0xdf981ff0)
1fa0: 00000020 00000000 00000003 0000011b 00000003 20000040
1fc0: 00000020 00000000 000118b0 00000126 000f4240 00000000 00000000 00003a97
1fe0: 7e9b4c90 7e9b4c80 00010624 0002a900
Rebooting in 86400 seconds..


---
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.

If the bug is already fixed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to change bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the bug is a duplicate of another bug, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup


2023-09-29 06:45:43

by Andrew Kanner

[permalink] [raw]
Subject: [PATCH net-next v1] net/xdp: fix zero-size allocation warning in xskq_create()

Syzkaller reported the following issue:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 2807 at mm/vmalloc.c:3247 __vmalloc_node_range (mm/vmalloc.c:3361)
Modules linked in:
CPU: 0 PID: 2807 Comm: repro Not tainted 6.6.0-rc2+ #12
Hardware name: Generic DT based system
unwind_backtrace from show_stack (arch/arm/kernel/traps.c:258)
show_stack from dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
dump_stack_lvl from __warn (kernel/panic.c:633 kernel/panic.c:680)
__warn from warn_slowpath_fmt (./include/linux/context_tracking.h:153 kernel/panic.c:700)
warn_slowpath_fmt from __vmalloc_node_range (mm/vmalloc.c:3361 (discriminator 3))
__vmalloc_node_range from vmalloc_user (mm/vmalloc.c:3478)
vmalloc_user from xskq_create (net/xdp/xsk_queue.c:40)
xskq_create from xsk_setsockopt (net/xdp/xsk.c:953 net/xdp/xsk.c:1286)
xsk_setsockopt from __sys_setsockopt (net/socket.c:2308)
__sys_setsockopt from ret_fast_syscall (arch/arm/kernel/entry-common.S:68)

xskq_get_ring_size() uses struct_size() macro to safely calculate the
size of struct xsk_queue and q->nentries of desc members. But the
syzkaller repro was able to set q->nentries with the value initially
taken from copy_from_sockptr() high enough to return SIZE_MAX by
struct_size(). The next PAGE_ALIGN(size) is such case will overflow
the size_t value and set it to 0. This will trigger WARN_ON_ONCE in
vmalloc_user() -> __vmalloc_node_range().

The issue is reproducible on 32-bit arm kernel.

Reported-and-tested-by: [email protected]
Closes: https://lore.kernel.org/all/[email protected]/T/
Link: https://syzkaller.appspot.com/bug?extid=fae676d3cf469331fc89
Fixes: 9f78bf330a66 ("xsk: support use vaddr as ring")
Signed-off-by: Andrew Kanner <[email protected]>
---
RFC notes:

It was found that net/xdp/xsk.c:xsk_setsockopt() uses
copy_from_sockptr() to get the number of entries (int) for cases with
XDP_RX_RING/XDP_TX_RING and XDP_UMEM_FILL_RING/XDP_UMEM_COMPLETION_RING.

Next in xsk_init_queue() there're 2 sanity checks (entries == 0) and
(!is_power_of_2(entries)) for which -EINVAL will be returned.

After that net/xdp/xsk_queue.c:xskq_create() will calculate the size
multipling the number of entries (int) with the size of u64, at least.

I wonder if there should be the upper bound (e.g. the 3rd sanity check
inside xsk_init_queue()). It seems that without the upper limit it's
quiet easy to overflow the allocated size (SIZE_MAX), especially for
32-bit architectures, for example arm nodes which were used by the
syzkaller.

In this patch I added a naive check for SIZE_MAX which helped to
skip zero-size allocation after overflow, but maybe it's not quite
right. Please, suggest if you have any thoughts about the appropriate
limit for the size of these xdp rings.

PS: the initial number of entries is 0x20000000 in syzkaller repro:
syscall(__NR_setsockopt, (intptr_t)r[0], 0x11b, 3, 0x20000040, 0x20);

Link: https://syzkaller.appspot.com/text?tag=ReproC&x=10910f18280000

net/xdp/xsk_queue.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/net/xdp/xsk_queue.c b/net/xdp/xsk_queue.c
index f8905400ee07..1bc7fb1f14ae 100644
--- a/net/xdp/xsk_queue.c
+++ b/net/xdp/xsk_queue.c
@@ -34,6 +34,9 @@ struct xsk_queue *xskq_create(u32 nentries, bool umem_queue)
q->ring_mask = nentries - 1;

size = xskq_get_ring_size(q, umem_queue);
+ if (size == SIZE_MAX)
+ return NULL;
+
size = PAGE_ALIGN(size);

q->ring = vmalloc_user(size);
--
2.39.3

2023-10-02 19:15:09

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH net-next v1] net/xdp: fix zero-size allocation warning in xskq_create()

From: Andrew Kanner <[email protected]>
Date: Thu, 28 Sep 2023 23:44:40 +0300

> Syzkaller reported the following issue:

[...]

> PS: the initial number of entries is 0x20000000 in syzkaller repro:
> syscall(__NR_setsockopt, (intptr_t)r[0], 0x11b, 3, 0x20000040, 0x20);
>
> Link: https://syzkaller.appspot.com/text?tag=ReproC&x=10910f18280000
>
> net/xdp/xsk_queue.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/xdp/xsk_queue.c b/net/xdp/xsk_queue.c
> index f8905400ee07..1bc7fb1f14ae 100644
> --- a/net/xdp/xsk_queue.c
> +++ b/net/xdp/xsk_queue.c
> @@ -34,6 +34,9 @@ struct xsk_queue *xskq_create(u32 nentries, bool umem_queue)
> q->ring_mask = nentries - 1;
>
> size = xskq_get_ring_size(q, umem_queue);
> + if (size == SIZE_MAX)

unlikely().

> + return NULL;
> +
> size = PAGE_ALIGN(size);
>
> q->ring = vmalloc_user(size);

Thanks,
Olek

2023-10-02 22:04:29

by Andrew Kanner

[permalink] [raw]
Subject: Re: [PATCH net-next v1] net/xdp: fix zero-size allocation warning in xskq_create()

On Mon, Oct 02, 2023 at 03:52:44PM +0200, Alexander Lobakin wrote:
> From: Andrew Kanner <[email protected]>
> Date: Thu, 28 Sep 2023 23:44:40 +0300
>
> > Syzkaller reported the following issue:
>
> [...]
>
> > PS: the initial number of entries is 0x20000000 in syzkaller repro:
> > syscall(__NR_setsockopt, (intptr_t)r[0], 0x11b, 3, 0x20000040, 0x20);
> >
> > Link: https://syzkaller.appspot.com/text?tag=ReproC&x=10910f18280000
> >
> > net/xdp/xsk_queue.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/net/xdp/xsk_queue.c b/net/xdp/xsk_queue.c
> > index f8905400ee07..1bc7fb1f14ae 100644
> > --- a/net/xdp/xsk_queue.c
> > +++ b/net/xdp/xsk_queue.c
> > @@ -34,6 +34,9 @@ struct xsk_queue *xskq_create(u32 nentries, bool umem_queue)
> > q->ring_mask = nentries - 1;
> >
> > size = xskq_get_ring_size(q, umem_queue);
> > + if (size == SIZE_MAX)
>
> unlikely().
>
> > + return NULL;
> > +
> > size = PAGE_ALIGN(size);
> >
> > q->ring = vmalloc_user(size);
>
> Thanks,
> Olek

Thanks, Olek.
That is a reasonable optimization, I'll add it in v2.

--
pw-bot: cr

Andrew Kanner