[PATCH v2] kasan: infer the requested size by scanning shadow memory

* [PATCH v2] kasan: infer the requested size by scanning shadow memory
@ 2023-01-18  9:38 ` Kuan-Ying Lee
  0 siblings, 0 replies; 6+ messages in thread
From: Kuan-Ying Lee @ 2023-01-18  9:38 UTC (permalink / raw)
  To: Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, Vincenzo Frascino, Andrew Morton,
	Matthias Brugger
  Cc: chinwen.chang, qun-wei.lin, Kuan-Ying Lee, kasan-dev, linux-mm,
	linux-kernel, linux-arm-kernel, linux-mediatek

We scan the shadow memory to infer the requested size instead of
printing cache->object_size directly.

This patch will fix the confusing kasan slab-out-of-bounds
report like below. [1]
Report shows "cache kmalloc-192 of size 192", but user
actually kmalloc(184).

==================================================================
BUG: KASAN: slab-out-of-bounds in _find_next_bit+0x143/0x160 lib/find_bit.c:109
Read of size 8 at addr ffff8880175766b8 by task kworker/1:1/26
...
The buggy address belongs to the object at ffff888017576600
 which belongs to the cache kmalloc-192 of size 192
The buggy address is located 184 bytes inside of
 192-byte region [ffff888017576600, ffff8880175766c0)
...
Memory state around the buggy address:
 ffff888017576580: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ffff888017576600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff888017576680: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc
                                        ^
 ffff888017576700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888017576780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================

After this patch, slab-out-of-bounds report will show as below.
==================================================================
...
The buggy address belongs to the object at ffff888017576600
 which belongs to the cache kmalloc-192 of size 192
The buggy address is located 0 bytes right of
 allocated 184-byte region [ffff888017576600, ffff8880175766b8)
...
==================================================================

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216457 [1]

Signed-off-by: Kuan-Ying Lee <Kuan-Ying.Lee@mediatek.com>
---
V1 -> V2:
 - Implement getting allocated size of object for tag-based kasan.
 - Refine the kasan report.
 - Check if it is slab-out-of-bounds report type.
 - Thanks for Andrey and Dmitry suggestion.

 mm/kasan/kasan.h          |  2 ++
 mm/kasan/report.c         | 20 +++++++++++++-------
 mm/kasan/report_generic.c | 24 ++++++++++++++++++++++++
 mm/kasan/report_hw_tags.c | 18 ++++++++++++++++++
 mm/kasan/report_sw_tags.c | 17 +++++++++++++++++
 mm/kasan/report_tags.c    |  8 ++++++++
 6 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index abbcc1b0eec5..15ffd46fec6a 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -185,6 +185,7 @@ struct kasan_report_info {
 	const char *bug_type;
 	struct kasan_track alloc_track;
 	struct kasan_track free_track;
+	int obj_size;
 };
 
 /* Do not change the struct layout: compiler ABI. */
@@ -306,6 +307,7 @@ static inline bool addr_has_metadata(const void *addr)
 void *kasan_find_first_bad_addr(void *addr, size_t size);
 void kasan_complete_mode_report_info(struct kasan_report_info *info);
 void kasan_metadata_fetch_row(char *buffer, void *row);
+int kasan_get_alloc_size(void *object_addr, struct kmem_cache *cache);
 
 #if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
 void kasan_print_tags(u8 addr_tag, const void *addr);
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index df3602062bfd..dae0d4ae8fe9 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -210,12 +210,13 @@ static inline struct page *addr_to_page(const void *addr)
 }
 
 static void describe_object_addr(const void *addr, struct kmem_cache *cache,
-				 void *object)
+				 void *object, int obj_size, const char *bug_type)
 {
 	unsigned long access_addr = (unsigned long)addr;
 	unsigned long object_addr = (unsigned long)object;
 	const char *rel_type;
 	int rel_bytes;
+	bool slab_oob = false;
 
 	pr_err("The buggy address belongs to the object at %px\n"
 	       " which belongs to the cache %s of size %d\n",
@@ -224,18 +225,22 @@ static void describe_object_addr(const void *addr, struct kmem_cache *cache,
 	if (access_addr < object_addr) {
 		rel_type = "to the left";
 		rel_bytes = object_addr - access_addr;
-	} else if (access_addr >= object_addr + cache->object_size) {
+	} else if (access_addr >= object_addr + obj_size) {
 		rel_type = "to the right";
-		rel_bytes = access_addr - (object_addr + cache->object_size);
+		rel_bytes = access_addr - (object_addr + obj_size);
 	} else {
 		rel_type = "inside";
 		rel_bytes = access_addr - object_addr;
 	}
 
+	if (strcmp(bug_type, "slab-out-of-bounds") == 0)
+		slab_oob = true;
+
 	pr_err("The buggy address is located %d bytes %s of\n"
-	       " %d-byte region [%px, %px)\n",
-		rel_bytes, rel_type, cache->object_size, (void *)object_addr,
-		(void *)(object_addr + cache->object_size));
+	       " %s%d-byte region [%px, %px)\n",
+	       rel_bytes, rel_type, slab_oob ? "allocated " : "",
+	       obj_size, (void *)object_addr,
+	       (void *)(object_addr + obj_size));
 }
 
 static void describe_object_stacks(struct kasan_report_info *info)
@@ -257,7 +262,8 @@ static void describe_object(const void *addr, struct kasan_report_info *info)
 {
 	if (kasan_stack_collection_enabled())
 		describe_object_stacks(info);
-	describe_object_addr(addr, info->cache, info->object);
+	describe_object_addr(addr, info->cache, info->object, info->obj_size,
+			info->bug_type);
 }
 
 static inline bool kernel_or_module_addr(const void *addr)
diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
index 043c94b04605..7b4bec9e6d1a 100644
--- a/mm/kasan/report_generic.c
+++ b/mm/kasan/report_generic.c
@@ -43,6 +43,25 @@ void *kasan_find_first_bad_addr(void *addr, size_t size)
 	return p;
 }
 
+int kasan_get_alloc_size(void *addr, struct kmem_cache *cache)
+{
+	int size = 0;
+	u8 *shadow;
+
+	shadow = (u8 *)kasan_mem_to_shadow(addr);
+	while (size < cache->object_size) {
+		if (*shadow == 0)
+			size += KASAN_GRANULE_SIZE;
+		else if (*shadow >= 1 && *shadow <= KASAN_GRANULE_SIZE - 1)
+			size += *shadow;
+		else
+			return size;
+		shadow++;
+	}
+
+	return cache->object_size;
+}
+
 static const char *get_shadow_bug_type(struct kasan_report_info *info)
 {
 	const char *bug_type = "unknown-crash";
@@ -149,6 +168,11 @@ void kasan_complete_mode_report_info(struct kasan_report_info *info)
 		memcpy(&info->free_track, &free_meta->free_track,
 		       sizeof(info->free_track));
 	}
+
+	if (strcmp(info->bug_type, "slab-out-of-bounds") == 0)
+		info->obj_size = kasan_get_alloc_size(info->object, info->cache);
+	else
+		info->obj_size = info->cache->object_size;
 }
 
 void kasan_metadata_fetch_row(char *buffer, void *row)
diff --git a/mm/kasan/report_hw_tags.c b/mm/kasan/report_hw_tags.c
index f3d3be614e4b..e462dd750fe2 100644
--- a/mm/kasan/report_hw_tags.c
+++ b/mm/kasan/report_hw_tags.c
@@ -21,6 +21,24 @@ void *kasan_find_first_bad_addr(void *addr, size_t size)
 	return kasan_reset_tag(addr);
 }
 
+int kasan_get_alloc_size(void *addr, struct kmem_cache *cache)
+{
+	int size = 0, i = 0;
+	u8 memory_tag;
+
+	while (size < cache->object_size) {
+		memory_tag = hw_get_mem_tag(addr + i * KASAN_GRANULE_SIZE);
+
+		if (memory_tag != KASAN_TAG_INVALID)
+			size += KASAN_GRANULE_SIZE;
+		else
+			return size;
+		i++;
+	}
+
+	return cache->object_size;
+}
+
 void kasan_metadata_fetch_row(char *buffer, void *row)
 {
 	int i;
diff --git a/mm/kasan/report_sw_tags.c b/mm/kasan/report_sw_tags.c
index 7a26397297ed..d50caefd7fd5 100644
--- a/mm/kasan/report_sw_tags.c
+++ b/mm/kasan/report_sw_tags.c
@@ -45,6 +45,23 @@ void *kasan_find_first_bad_addr(void *addr, size_t size)
 	return p;
 }
 
+int kasan_get_alloc_size(void *addr, struct kmem_cache *cache)
+{
+	int size = 0;
+	u8 *shadow;
+
+	shadow = (u8 *)kasan_mem_to_shadow(addr);
+	while (size < cache->object_size) {
+		if (*shadow != KASAN_TAG_INVALID)
+			size += KASAN_GRANULE_SIZE;
+		else
+			return size;
+		shadow++;
+	}
+
+	return cache->object_size;
+}
+
 void kasan_metadata_fetch_row(char *buffer, void *row)
 {
 	memcpy(buffer, kasan_mem_to_shadow(row), META_BYTES_PER_ROW);
diff --git a/mm/kasan/report_tags.c b/mm/kasan/report_tags.c
index ecede06ef374..b349a0ae1b83 100644
--- a/mm/kasan/report_tags.c
+++ b/mm/kasan/report_tags.c
@@ -7,6 +7,7 @@
 #include <linux/atomic.h>
 
 #include "kasan.h"
+#include "../slab.h"
 
 extern struct kasan_stack_ring stack_ring;
 
@@ -113,4 +114,11 @@ void kasan_complete_mode_report_info(struct kasan_report_info *info)
 	/* Assign the common bug type if no entries were found. */
 	if (!info->bug_type)
 		info->bug_type = get_common_bug_type(info);
+
+	if (info->object && info->cache) {
+		if (strcmp(info->bug_type, "slab-out-of-bounds") == 0)
+			info->obj_size = kasan_get_alloc_size(info->object, info->cache);
+		else
+			info->obj_size = info->cache->object_size;
+	}
 }
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread