linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Vlastimil Babka' <vbabka@suse.cz>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"'linux-mm@kvack.org'" <linux-mm@kvack.org>
Cc: 'Kees Cook' <keescook@chromium.org>,
	'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>,
	"Hyeonggon Yoo" <42.hyeyoo@gmail.com>,
	Roman Gushchin <roman.gushchin@linux.dev>
Subject: Subject: [PATCH v2] slab: kmalloc_size_roundup() must not return 0 for non-zero size
Date: Thu, 7 Sep 2023 12:42:20 +0000	[thread overview]
Message-ID: <4d31a2bf7eb544749023cf491c0eccc8@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()")
---
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)



             reply	other threads:[~2023-09-07 12:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-07 12:42 David Laight [this message]
2023-09-07 19:38 ` Kees Cook
2023-09-08  8:26   ` David Laight
2023-09-11 15:54     ` Vlastimil Babka
2023-09-11 16:12       ` David Laight
2023-09-11 16:23         ` Vlastimil Babka
2023-09-11 16:38           ` David Laight
2023-09-20  9:58             ` Vlastimil Babka
2023-09-20 10:06               ` David Laight
2023-09-20 12:48                 ` Vlastimil Babka
2023-09-30 13:53                   ` Hyeonggon Yoo

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=4d31a2bf7eb544749023cf491c0eccc8@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=42.hyeyoo@gmail.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=roman.gushchin@linux.dev \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox