All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"'linux-mm@kvack.org'" <linux-mm@kvack.org>
Cc: 'Kees Cook' <keescook@chromium.org>,
	'Vlastimil Babka' <vbabka@suse.cz>,
	'Christoph Lameter' <cl@linux.com>,
	'Pekka Enberg' <penberg@kernel.org>,
	'David Rientjes' <rientjes@google.com>,
	'Joonsoo Kim' <iamjoonsoo.kim@lge.com>,
	'Andrew Morton' <akpm@linux-foundation.org>,
	"'Eric Dumazet'" <edumazet@google.com>
Subject: [PATCH] slab: kmalloc_size_roundup() must not return 0 for non-zero size
Date: Wed, 6 Sep 2023 08:18:21 +0000	[thread overview]
Message-ID: <fcfee37ead054de19871139167aca787@AcuMS.aculab.com> (raw)

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@aculab.com>
Fixes: 05a940656e1eb ("slab: Introduce kmalloc_size_roundup()")
---
The 'sanity limit' value doesn't really matter (even if too small)
It could be 'MAX_ORDER + PAGE_SHIFT' but one ppc64 has MAX_ORDER 16
and I don't know if that also has large pages.
Maybe it could be 1ul << 30 on 64bit, but it really doesn't matter
if it is too big.

The original patch also added kmalloc_size_roundup() to mm/slob.c
that can also round up a value to zero - but has since been removed.

 mm/slab_common.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index cd71f9581e67..8418eccda8cf 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -747,22 +747,21 @@ 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;
-	/* Above the smaller buckets, size is a multiple of page size. */
-	if (size > KMALLOC_MAX_CACHE_SIZE)
-		return PAGE_SIZE << get_order(size);
+	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;
+	}
 
-	/*
-	 * 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. */
+	if (unlikely((size - 1) >> (sizeof (long) == 8 ? 34 : 30)))
+		return size;
+
+	/* Above the smaller buckets, size is a multiple of page size. */
+	return PAGE_SIZE << get_order(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)


             reply	other threads:[~2023-09-06  8:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-06  8:18 David Laight [this message]
2023-09-06  8:47 ` [PATCH] slab: kmalloc_size_roundup() must not return 0 for non-zero size Vlastimil Babka
2023-09-06  9:14   ` David Laight
2023-09-06 18:16 ` Kees Cook
2023-09-07  8:55   ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fcfee37ead054de19871139167aca787@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=edumazet@google.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.