linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Feng Tang <feng.tang@intel.com>
To: Andrew Morton <akpm@linux-foundation.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>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Hyeonggon Yoo <42.hyeyoo@gmail.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Andrey Konovalov <andreyknvl@gmail.com>
Cc: Dave Hansen <dave.hansen@intel.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kasan-dev@googlegroups.com, Feng Tang <feng.tang@intel.com>
Subject: [PATCH v6 4/4] mm/slub: extend redzone check to extra allocated kmalloc space than requested
Date: Tue, 13 Sep 2022 14:54:23 +0800	[thread overview]
Message-ID: <20220913065423.520159-5-feng.tang@intel.com> (raw)
In-Reply-To: <20220913065423.520159-1-feng.tang@intel.com>

kmalloc will round up the request size to a fixed size (mostly power
of 2), so there could be a extra space than what is requested, whose
size is the actual buffer size minus original request size.

To better detect out of bound access or abuse of this space, add
redzone sanity check for it.

And in current kernel, some kmalloc user already knows the existence
of the space and utilizes it after calling 'ksize()' to know the real
size of the allocated buffer. So we skip the sanity check for objects
which have been called with ksize(), as treating them as legitimate
users.

In some cases, the free pointer could be saved inside the latter
part of object data area, which may overlap the redzone part(for
small sizes of kmalloc objects). As suggested by Hyeonggon Yoo,
force the free pointer to be in meta data area when kmalloc redzone
debug is enabled, to make all kmalloc objects covered by redzone
check.

Suggested-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 mm/slab.h        |  4 ++++
 mm/slab_common.c |  4 ++++
 mm/slub.c        | 51 ++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/mm/slab.h b/mm/slab.h
index 3cf5adf63f48..5ca04d9c8bf5 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -881,4 +881,8 @@ void __check_heap_object(const void *ptr, unsigned long n,
 }
 #endif
 
+#ifdef CONFIG_SLUB_DEBUG
+void skip_orig_size_check(struct kmem_cache *s, const void *object);
+#endif
+
 #endif /* MM_SLAB_H */
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 8e13e3aac53f..5106667d6adb 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1001,6 +1001,10 @@ size_t __ksize(const void *object)
 		return folio_size(folio);
 	}
 
+#ifdef CONFIG_SLUB_DEBUG
+	skip_orig_size_check(folio_slab(folio)->slab_cache, object);
+#endif
+
 	return slab_ksize(folio_slab(folio)->slab_cache);
 }
 
diff --git a/mm/slub.c b/mm/slub.c
index 6f823e99d8b4..546b30ed5afd 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -812,12 +812,28 @@ static inline void set_orig_size(struct kmem_cache *s,
 	if (!slub_debug_orig_size(s))
 		return;
 
+#ifdef CONFIG_KASAN_GENERIC
+	/*
+	 * KASAN could save its free meta data in object's data area at
+	 * offset 0, if the size is larger than 'orig_size', it could
+	 * overlap the data redzone(from 'orig_size+1' to 'object_size'),
+	 * where the check should be skipped.
+	 */
+	if (s->kasan_info.free_meta_size > orig_size)
+		orig_size = s->object_size;
+#endif
+
 	p += get_info_end(s);
 	p += sizeof(struct track) * 2;
 
 	*(unsigned int *)p = orig_size;
 }
 
+void skip_orig_size_check(struct kmem_cache *s, const void *object)
+{
+	set_orig_size(s, (void *)object, s->object_size);
+}
+
 static unsigned int get_orig_size(struct kmem_cache *s, void *object)
 {
 	void *p = kasan_reset_tag(object);
@@ -949,13 +965,27 @@ static __printf(3, 4) void slab_err(struct kmem_cache *s, struct slab *slab,
 static void init_object(struct kmem_cache *s, void *object, u8 val)
 {
 	u8 *p = kasan_reset_tag(object);
+	unsigned int orig_size = s->object_size;
 
-	if (s->flags & SLAB_RED_ZONE)
+	if (s->flags & SLAB_RED_ZONE) {
 		memset(p - s->red_left_pad, val, s->red_left_pad);
 
+		if (slub_debug_orig_size(s) && val == SLUB_RED_ACTIVE) {
+			orig_size = get_orig_size(s, object);
+
+			/*
+			 * Redzone the extra allocated space by kmalloc
+			 * than requested.
+			 */
+			if (orig_size < s->object_size)
+				memset(p + orig_size, val,
+				       s->object_size - orig_size);
+		}
+	}
+
 	if (s->flags & __OBJECT_POISON) {
-		memset(p, POISON_FREE, s->object_size - 1);
-		p[s->object_size - 1] = POISON_END;
+		memset(p, POISON_FREE, orig_size - 1);
+		p[orig_size - 1] = POISON_END;
 	}
 
 	if (s->flags & SLAB_RED_ZONE)
@@ -1103,6 +1133,7 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
 {
 	u8 *p = object;
 	u8 *endobject = object + s->object_size;
+	unsigned int orig_size;
 
 	if (s->flags & SLAB_RED_ZONE) {
 		if (!check_bytes_and_report(s, slab, object, "Left Redzone",
@@ -1112,6 +1143,17 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
 		if (!check_bytes_and_report(s, slab, object, "Right Redzone",
 			endobject, val, s->inuse - s->object_size))
 			return 0;
+
+		if (slub_debug_orig_size(s) && val == SLUB_RED_ACTIVE) {
+			orig_size = get_orig_size(s, object);
+
+			if (s->object_size > orig_size  &&
+				!check_bytes_and_report(s, slab, object,
+					"kmalloc Redzone", p + orig_size,
+					val, s->object_size - orig_size)) {
+				return 0;
+			}
+		}
 	} else {
 		if ((s->flags & SLAB_POISON) && s->object_size < s->inuse) {
 			check_bytes_and_report(s, slab, p, "Alignment padding",
@@ -4187,7 +4229,8 @@ static int calculate_sizes(struct kmem_cache *s)
 	 */
 	s->inuse = size;
 
-	if ((flags & (SLAB_TYPESAFE_BY_RCU | SLAB_POISON)) ||
+	if (slub_debug_orig_size(s) ||
+	    (flags & (SLAB_TYPESAFE_BY_RCU | SLAB_POISON)) ||
 	    ((flags & SLAB_RED_ZONE) && s->object_size < sizeof(void *)) ||
 	    s->ctor) {
 		/*
-- 
2.34.1


  parent reply	other threads:[~2022-09-13  6:55 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-13  6:54 [PATCH v6 0/4] mm/slub: some debug enhancements for kmalloc Feng Tang
2022-09-13  6:54 ` [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc Feng Tang
2022-09-23 11:43   ` Vlastimil Babka
2022-09-24  7:08     ` Feng Tang
2022-10-30 19:23   ` John Thomson
2022-10-30 21:30     ` Vlastimil Babka
2022-10-31  2:36       ` Feng Tang
2022-10-31 10:05         ` John Thomson
2022-10-31 11:36           ` Hyeonggon Yoo
2022-10-31 11:42           ` Feng Tang
2022-11-01  0:18             ` John Thomson
2022-11-01  2:41               ` John Thomson
2022-11-01  7:57               ` Feng Tang
2022-11-01  9:20                 ` John Thomson
2022-11-01  9:31                   ` Hyeonggon Yoo
2022-11-01 10:33                     ` John Thomson
2022-11-01 10:42                       ` Hyeonggon Yoo
2022-11-01 13:55                         ` Feng Tang
2022-11-01 19:39                           ` John Thomson
2022-11-02  6:08                             ` Feng Tang
2022-11-02  7:16                               ` Hyeonggon Yoo
2022-11-03  7:18                                 ` Feng Tang
2022-11-03  7:45                                   ` John Thomson
2022-11-03  8:16                                     ` Feng Tang
2022-11-02  8:22                       ` Vlastimil Babka
2022-11-03  5:54                         ` Feng Tang
2022-11-03  8:33                           ` Vlastimil Babka
2022-11-03 14:16                             ` Feng Tang
2022-11-03 14:36                               ` Hyeonggon Yoo
2022-11-03 16:57                                 ` Vlastimil Babka
2022-11-03 17:35                                   ` Vlastimil Babka
2022-11-04  3:52                                     ` Feng Tang
2022-09-13  6:54 ` [PATCH v6 2/4] mm/slub: only zero the requested size of buffer for kzalloc Feng Tang
2022-09-26 19:11   ` Andrey Konovalov
2022-09-26 20:15     ` Kees Cook
2022-09-27  1:22       ` Feng Tang
2022-09-27  2:42     ` Feng Tang
2022-10-13 14:00       ` Andrey Konovalov
2022-10-14  5:59         ` Feng Tang
2022-09-13  6:54 ` [PATCH v6 3/4] mm: kasan: Add free_meta size info in struct kasan_cache Feng Tang
2022-09-20 19:20   ` Andrey Konovalov
2022-09-21 12:02     ` Feng Tang
2022-09-24 18:05       ` Andrey Konovalov
2022-09-25 11:26         ` Feng Tang
2022-09-25 16:31           ` Andrey Konovalov
2022-09-27  3:03             ` Feng Tang
2022-09-13  6:54 ` Feng Tang [this message]
2022-09-13  8:53   ` [PATCH v6 4/4] mm/slub: extend redzone check to extra allocated kmalloc space than requested 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=20220913065423.520159-5-feng.tang@intel.com \
    --to=feng.tang@intel.com \
    --cc=42.hyeyoo@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@gmail.com \
    --cc=cl@linux.com \
    --cc=corbet@lwn.net \
    --cc=dave.hansen@intel.com \
    --cc=dvyukov@google.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=kasan-dev@googlegroups.com \
    --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;
as well as URLs for NNTP newsgroup(s).