Subject: [PATCH v2] slab: kmalloc_size_roundup() must not return 0 for non-zero size

From: David Laight
Date: Thu Sep 07 2023 - 12:09:09 EST


The typical use of kmalloc_size_roundup() is:
ptr = kmalloc(sz = kmalloc_size_roundup(size), ...);
if (!ptr) return -ENOMEM.
This means it is vitally important that the returned value isn't
less than the argument even if the argument is insane.
In particular if kmalloc_slab() fails or the value is above
(MAX_ULONG - PAGE_SIZE) zero is returned and kmalloc() will return
it's single zero-length buffer.

Fix by returning the input size on error or if the size exceeds
a 'sanity' limit.
kmalloc() will then return NULL is the size really is too big.


Signed-off-by: David Laight <david.laight@xxxxxxxxxx>
Fixes: 05a940656e1eb ("slab: Introduce kmalloc_size_roundup()")
---
v2:
- Use KMALLOC_MAX_SIZE for upper limit.
(KMALLOC_MAX_SIZE + 1 may give better code on some archs!)
- Invert test for overlarge for consistency.
- Put a likely() on result of kmalloc_slab().

mm/slab_common.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index cd71f9581e67..0fb7c7e19bad 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -747,22 +747,22 @@ size_t kmalloc_size_roundup(size_t size)
{
struct kmem_cache *c;

- /* Short-circuit the 0 size case. */
- if (unlikely(size == 0))
- return 0;
- /* Short-circuit saturated "too-large" case. */
- if (unlikely(size == SIZE_MAX))
- return SIZE_MAX;
+ if (size && size <= KMALLOC_MAX_CACHE_SIZE) {
+ /*
+ * The flags don't matter since size_index is common to all.
+ * Neither does the caller for just getting ->object_size.
+ */
+ c = kmalloc_slab(size, GFP_KERNEL, 0);
+ return likely(c) ? c->object_size : size;
+ }
+
/* Above the smaller buckets, size is a multiple of page size. */
- if (size > KMALLOC_MAX_CACHE_SIZE)
+ if (size && size <= KMALLOC_MAX_SIZE)
return PAGE_SIZE << get_order(size);

- /*
- * The flags don't matter since size_index is common to all.
- * Neither does the caller for just getting ->object_size.
- */
- c = kmalloc_slab(size, GFP_KERNEL, 0);
- return c ? c->object_size : 0;
+ /* Return 'size' for 0 and very large - kmalloc() may fail. */
+ return size;
+
}
EXPORT_SYMBOL(kmalloc_size_roundup);

--
2.17.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)