Commit 7661809d493b ("mm: don't allow oversized kvmalloc() calls")
restricted memory allocation with 'kvmalloc()' to sizes that fit
in an 'int', to protect against trivial integer conversion issues.
However, the WARN triggers with KVM, when it allocates ancillary page
data whose size essentially depends on whatever userspace has passed to
the KVM_SET_USER_MEMORY_REGION ioctl. The warnings are easily raised by
syzkaller, but the largest allocation that KVM can do is 8 bytes per page
of guest memory; therefore, a 1 TiB memslot will cause a warning even
outside fuzzing, and those allocations are known to happen in the wild.
Google for example already has VMs that create 1.5tb memslots (12tb of
total guest memory spread across 8 virtual NUMA nodes).
Use memcg accounting as evidence that the crazy large allocations are
expected---in which case, it is indeed a good idea to have them
properly accounted---and exempt them from the warning.
Cc: Willy Tarreau <w@xxxxxx>
Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Reported-by: syzbot+e0de2333cbf95ea473e8@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
---
Linus, what do you think of this? It is a bit of a hack,
but the reasoning in the commit message does make at least
some sense.
The alternative would be to just use __vmalloc in KVM, and add
__vcalloc too. The two underscores would suggest that something
"different" is going on, but I wonder what you prefer between
this and having a __vcalloc with 2-3 uses in the whole source.
mm/util.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/util.c b/mm/util.c
index 499b6b5767ed..31fca4a999c6 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -593,8 +593,12 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
if (ret || size <= PAGE_SIZE)
return ret;
- /* Don't even allow crazy sizes */
- if (WARN_ON_ONCE(size > INT_MAX))
+ /*
+ * Don't even allow crazy sizes unless memcg accounting is
+ * request. We take that as a sign that huge allocations
+ * are indeed expected.
+ */
+ if (likely(!(flags & __GFP_ACCOUNT)) && WARN_ON_ONCE(size > INT_MAX))
return NULL;
return __vmalloc_node(size, 1, flags, node,
--
2.27.0
