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
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?
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.b9892171e3f5b9a60f9da251@linux-foundation.org/
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
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);
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);
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
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);
Honestly, I would need some guidance on how to progress. I could just
change from U32_MAX to INT_MAX
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
On Thu, Feb 10, 2022 at 9:57 AM Dan Carpenter [off-list ref] 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