[syzbot] WARNING: kmalloc bug in xdp_umem_create (2)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: a51e3ac43ddb Merge tag 'net-5.16-rc4' of git://git.kernel...
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=17f04ebeb00000
kernel config: https://syzkaller.appspot.com/x/.config?x=5b0eee8ab3ea1839
dashboard link: https://syzkaller.appspot.com/bug?extid=11421fbbff99b989670e
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

Unfortunately, I don't have any reproducer for this issue yet.

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+11421f...@syzkaller.appspotmail.com

------------[ cut here ]------------
WARNING: CPU: 0 PID: 20253 at mm/util.c:597 kvmalloc_node+0x111/0x120 mm/util.c:597
Modules linked in:
CPU: 0 PID: 20253 Comm: syz-executor.1 Not tainted 5.16.0-rc3-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:kvmalloc_node+0x111/0x120 mm/util.c:597
Code: 01 00 00 00 4c 89 e7 e8 3d f7 0c 00 49 89 c5 e9 69 ff ff ff e8 30 1e d1 ff 41 89 ed 41 81 cd 00 20 01 00 eb 95 e8 1f 1e d1 ff <0f> 0b e9 4c ff ff ff 0f 1f 84 00 00 00 00 00 55 48 89 fd 53 e8 06
RSP: 0018:ffffc900029a7c48 EFLAGS: 00010212
RAX: 00000000000000ff RBX: 0000000000000001 RCX: ffffc9000ac13000
RDX: 0000000000040000 RSI: ffffffff81a68ca1 RDI: 0000000000000003
RBP: 0000000000002dc0 R08: 000000007fffffff R09: 00000000ffffffff
R10: ffffffff81a68c5e R11: 0000000000000000 R12: 0000000708001000
R13: 0000000000000000 R14: 00000000ffffffff R15: 0000000000000f00
FS: 00007f1582971700(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000100 CR3: 000000002a1b1000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
kvmalloc include/linux/slab.h:741 [inline]
kvmalloc_array include/linux/slab.h:759 [inline]
kvcalloc include/linux/slab.h:764 [inline]
xdp_umem_pin_pages net/xdp/xdp_umem.c:102 [inline]
xdp_umem_reg net/xdp/xdp_umem.c:219 [inline]
xdp_umem_create+0x563/0x1180 net/xdp/xdp_umem.c:252
xsk_setsockopt+0x73e/0x9e0 net/xdp/xsk.c:1053
__sys_setsockopt+0x2db/0x610 net/socket.c:2176
__do_sys_setsockopt net/socket.c:2187 [inline]
__se_sys_setsockopt net/socket.c:2184 [inline]
__x64_sys_setsockopt+0xba/0x150 net/socket.c:2184
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f158541cae9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f1582971188 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 00007f1585530020 RCX: 00007f158541cae9
RDX: 0000000000000004 RSI: 000000000000011b RDI: 0000000000000005
RBP: 00007f1585476f6d R08: 0000000000000020 R09: 0000000000000000
R10: 00000000200000c0 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ffe483898ef R14: 00007f1582971300 R15: 0000000000022000
</TASK>


---
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 syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

[Björn Töpel]

On Mon, 6 Dec 2021 at 11:55, syzbot
<syzbot+11421f...@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: a51e3ac43ddb Merge tag 'net-5.16-rc4' of git://git.kernel...
> git tree: net
> console output: https://syzkaller.appspot.com/x/log.txt?x=17f04ebeb00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=5b0eee8ab3ea1839
> dashboard link: https://syzkaller.appspot.com/bug?extid=11421fbbff99b989670e
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
>
> Unfortunately, I don't have any reproducer for this issue yet.
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+11421f...@syzkaller.appspotmail.com
>

This warning stems from mm/utils.c:
/* Don't even allow crazy sizes */
if (WARN_ON_ONCE(size > INT_MAX))
return NULL;

The structure that is being allocated is the page-pinning accounting.
AF_XDP has an internal limit of U32_MAX pages, which is *a lot*, but
still fewer than what memcg allows (PAGE_COUNTER_MAX is a
LONG_MAX/PAGE_SIZE on 64b systems).

The (imo hacky) workaround to silence the warning is to decrease the
U32_MAX limit to something that is less than "sizeof householding
struct".

Note that this is a warning, and not an oops/bug.

Thoughts?


Björn

[Daniel Borkmann]

[ +Andrew ]

This is coming from 7661809d493b ("mm: don't allow oversized kvmalloc() calls").
There was a recent discussion on this topic here [0]; this adds another instance.

Iff removal would not be an option, could we maybe add a __GFP_LARGE flag to tag
these instances that it is indeed intended that large allocs are allowed (and they
would thus bypass this warning)?

Thanks,
Daniel

[0] https://lore.kernel.org/bpf/20211201202905.b989...@linux-foundation.org/

[syzbot]

syzbot has found a reproducer for the following issue on:

HEAD commit: f4bc5bbb5fef Merge tag 'nfsd-5.17-2' of git://git.kernel.o..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12073c74700000
kernel config: https://syzkaller.appspot.com/x/.config?x=5707221760c00a20

dashboard link: https://syzkaller.appspot.com/bug?extid=11421fbbff99b989670e
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12e514a4700000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15fcdf8a700000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+11421f...@syzkaller.appspotmail.com

------------[ cut here ]------------

WARNING: CPU: 1 PID: 3590 at mm/util.c:590 kvmalloc_node+0xf5/0x100 mm/util.c:590
Modules linked in:
CPU: 0 PID: 3590 Comm: syz-executor153 Not tainted 5.17.0-rc3-syzkaller-00043-gf4bc5bbb5fef #0

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011

RIP: 0010:kvmalloc_node+0xf5/0x100 mm/util.c:590
Code: 01 00 00 00 48 89 ef e8 c9 0d 0d 00 49 89 c5 e9 62 ff ff ff e8 ec d3 d0 ff 45 89 e5 41 81 cd 00 20 01 00 eb 8e e8 db d3 d0 ff <0f> 0b e9 45 ff ff ff 0f 1f 40 00 55 48 89 fd 53 e8 c6 d3 d0 ff 48
RSP: 0018:ffffc90002957c48 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000000
RDX: ffff88807297e2c0 RSI: ffffffff81a6da65 RDI: 0000000000000003
RBP: 00000007ff810000 R08: 000000007fffffff R09: 0000000000000001
R10: ffffffff81a6da21 R11: 0000000000000000 R12: 0000000000002dc0
R13: 0000000000000000 R14: 00000000ffffffff R15: 0000000000000700
FS: 000055555577a300(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000

CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033

CR2: 00007f31855463b0 CR3: 000000001d0ed000 CR4: 00000000003506f0

DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>

kvmalloc include/linux/slab.h:732 [inline]
kvmalloc_array include/linux/slab.h:750 [inline]
kvcalloc include/linux/slab.h:755 [inline]

xdp_umem_pin_pages net/xdp/xdp_umem.c:102 [inline]
xdp_umem_reg net/xdp/xdp_umem.c:219 [inline]
xdp_umem_create+0x563/0x1180 net/xdp/xdp_umem.c:252

xsk_setsockopt+0x73e/0x9e0 net/xdp/xsk.c:1051
__sys_setsockopt+0x2db/0x610 net/socket.c:2180
__do_sys_setsockopt net/socket.c:2191 [inline]
__se_sys_setsockopt net/socket.c:2188 [inline]
__x64_sys_setsockopt+0xba/0x150 net/socket.c:2188

do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae

RIP: 0033:0x7f3185535009
Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fff78e9c498 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f3185535009
RDX: 0000000000000004 RSI: 000000000000011b RDI: 0000000000000003
RBP: 00007f31854f8ff0 R08: 0000000000000020 R09: 0000000000000000
R10: 0000000020000080 R11: 0000000000000246 R12: 00007f31854f9080
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
</TASK>

[syzbot]

syzbot has bisected this issue to:

commit 7661809d493b426e979f39ab512e3adf41fbcc69
Author: Linus Torvalds <torv...@linux-foundation.org>
Date: Wed Jul 14 16:45:49 2021 +0000

mm: don't allow oversized kvmalloc() calls

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=13bc74c2700000
start commit: f4bc5bbb5fef Merge tag 'nfsd-5.17-2' of git://git.kernel.o..
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=107c74c2700000
console output: https://syzkaller.appspot.com/x/log.txt?x=17bc74c2700000

kernel config: https://syzkaller.appspot.com/x/.config?x=5707221760c00a20
dashboard link: https://syzkaller.appspot.com/bug?extid=11421fbbff99b989670e

syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12e514a4700000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15fcdf8a700000

Reported-by: syzbot+11421f...@syzkaller.appspotmail.com
Fixes: 7661809d493b ("mm: don't allow oversized kvmalloc() calls")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

[Willy Tarreau]

Interesting, so in fact syzkaller has shown that the aforementioned
patch does its job well and has spotted a call path by which a single
userland setsockopt() can request more than 2 GB allocation in the
kernel. Most likely that's in fact what needs to be addressed.

FWIW the call trace at the URL above is:

Call Trace:
kvmalloc include/linux/mm.h:806 [inline]
kvmalloc_array include/linux/mm.h:824 [inline]
kvcalloc include/linux/mm.h:829 [inline]

xdp_umem_pin_pages net/xdp/xdp_umem.c:102 [inline]
xdp_umem_reg net/xdp/xdp_umem.c:219 [inline]

xdp_umem_create+0x6a5/0xf00 net/xdp/xdp_umem.c:252
xsk_setsockopt+0x604/0x790 net/xdp/xsk.c:1068
__sys_setsockopt+0x1fd/0x4e0 net/socket.c:2176

__do_sys_setsockopt net/socket.c:2187 [inline]
__se_sys_setsockopt net/socket.c:2184 [inline]

__x64_sys_setsockopt+0xb5/0x150 net/socket.c:2184

do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae

and the meaningful part of the repro is:

syscall(__NR_mmap, 0x1ffff000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
syscall(__NR_mmap, 0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
syscall(__NR_mmap, 0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
intptr_t res = 0;
res = syscall(__NR_socket, 0x2cul, 3ul, 0);
if (res != -1)
r[0] = res;
*(uint64_t*)0x20000080 = 0;
*(uint64_t*)0x20000088 = 0xfff02000000;
*(uint32_t*)0x20000090 = 0x800;
*(uint32_t*)0x20000094 = 0;
*(uint32_t*)0x20000098 = 0;
syscall(__NR_setsockopt, r[0], 0x11b, 4, 0x20000080ul, 0x20ul);

Willy

[Daniel Borkmann]

Bjorn had a comment back then when the issue was first raised here:

https://lore.kernel.org/bpf/3f854ca9-f5d6-4065...@iogearbox.net/

There was earlier discussion from Andrew to potentially retire the warning:

https://lore.kernel.org/bpf/20211201202905.b989...@linux-foundation.org/

Bjorn / Magnus / Andrew, anyone planning to follow-up on this issue?

Thanks,
Daniel

[Björn Töpel]

Honestly, I would need some guidance on how to progress. I could just
change from U32_MAX to INT_MAX, but as I stated earlier (lore-link
above), that has a hacky feeling to it. Andrew's mail didn't really
land in a consensus. From my perspective, the code isn't broken, with
the memcg limits in consideration. Introducing a LARGE flag or a new
"_yes_this_can_be_huge_but_it_is_ok()" version would make sense if
this problem is applicable to more users in the kernel.

So, thoughts? ;-)


Björn

[Dan Carpenter]

It would have to be lower than that. The limit is on "npgs" but we are
allocating npgs * sizeof(struct page *) so it would have to:

if (npgs > INT_MAX / sizeof(void *))
return -EINVAL;

Is it normally going to huge? You could call vmalloc() instead of
kvmalloc().

When Linus added the WARN_ON() for huge kvmalloc sizes, it was as a
reaction to a security bug where the size which was more than UINT_MAX
but not everything was prepared to handle ulong sizes. He wanted
people who allocated large amounts of RAM to do it in a deliberate way
instead of by accident.

regards,
dan carpenter

[Dan Carpenter]

On Thu, Feb 10, 2022 at 08:45:08PM +0300, Dan Carpenter wrote:
> Is it normally going to huge? You could call vmalloc() instead of
> kvmalloc().

Wait that would make the allocation succeed... We don't want that.
That was a dumb idea. Forget I said that.

regards,
dan carpenter

[Linus Torvalds]

On Thu, Feb 10, 2022 at 9:57 AM Dan Carpenter <dan.ca...@oracle.com> wrote:
>
> Wait that would make the allocation succeed... We don't want that.
> That was a dumb idea. Forget I said that.

Well, sometimes that _can_ be the right model.

That said, pretty much every time this has come up, the kernel warning
has shown that yes, the code was broken and there really wasn't a
reason for doing allocations that big.

Of course, some people would be perfectly fine with the allocation
failing, they just don't want the warning. I didn't want __GFP_NOWARN
to shut it up originally because I wanted people to see all those
cases, but these days I think we can just say "yeah, people can shut
it up explicitly by saying 'go ahead and fail this allocation, don't
warn about it'".

So enough time has passed that by now I'd certainly be ok with something like

--- a/mm/util.c
+++ b/mm/util.c
@@ -587,8 +587,10 @@ void *kvmalloc_node(size_t size,
return ret;

/* Don't even allow crazy sizes */

- if (WARN_ON_ONCE(size > INT_MAX))
+ if (unlikely(size > INT_MAX)) {
+ WARN_ON_ONCE(!(flags & __GFP_NOWARN));
return NULL;
+ }

return __vmalloc_node(size, 1, flags, node,
__builtin_return_address(0));

(which is obviously COMPLETELY UNTESTED as well as being
whitespace-damaged, but you get the idea).

If somebody tests that patch and verifies it works, and writes a
little commit blurb for it, I'll apply it.

Linus