[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230825211426.3798691-1-jannh@google.com>
Date: Fri, 25 Aug 2023 23:14:26 +0200
From: Jann Horn <jannh@...gle.com>
To: Andrey Ryabinin <ryabinin.a.a@...il.com>,
Christoph Lameter <cl@...ux.com>,
Pekka Enberg <penberg@...nel.org>,
David Rientjes <rientjes@...gle.com>,
Joonsoo Kim <iamjoonsoo.kim@....com>,
Vlastimil Babka <vbabka@...e.cz>
Cc: Alexander Potapenko <glider@...gle.com>,
Andrey Konovalov <andreyknvl@...il.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Vincenzo Frascino <vincenzo.frascino@....com>,
Andrew Morton <akpm@...ux-foundation.org>,
Roman Gushchin <roman.gushchin@...ux.dev>,
Hyeonggon Yoo <42.hyeyoo@...il.com>,
kasan-dev@...glegroups.com,
linux-kernel@...r.kernel.org,
linux-mm@...ck.org,
linux-hardening@...r.kernel.org,
kernel-hardening@...ts.openwall.com
Subject: [PATCH] slub: Introduce CONFIG_SLUB_RCU_DEBUG
Currently, KASAN is unable to catch use-after-free in SLAB_TYPESAFE_BY_RCU
slabs because use-after-free is allowed within the RCU grace period by
design.
Add a SLUB debugging feature which RCU-delays every individual
kmem_cache_free() before either actually freeing the object or handing it
off to KASAN, and change KASAN to poison freed objects as normal when this
option is enabled.
Note that this creates a 16-byte unpoisoned area in the middle of the
slab metadata area, which kinda sucks but seems to be necessary in order
to be able to store an rcu_head in there without triggering an ASAN
splat during RCU callback processing.
For now I've configured Kconfig.kasan to always enable this feature in the
GENERIC and SW_TAGS modes; I'm not forcibly enabling it in HW_TAGS mode
because I'm not sure if it might have unwanted performance degradation
effects there.
Signed-off-by: Jann Horn <jannh@...gle.com>
---
can I get a review from the KASAN folks of this?
I have been running it on my laptop for a bit and it seems to be working
fine.
Notes:
With this patch, a UAF on a TYPESAFE_BY_RCU will splat with an error
like this (tested by reverting a security bugfix).
Note that, in the ASAN memory state dump, we can see the little
unpoisoned 16-byte areas storing the rcu_head.
BUG: KASAN: slab-use-after-free in folio_lock_anon_vma_read+0x129/0x4c0
Read of size 8 at addr ffff888004e85b00 by task forkforkfork/592
CPU: 0 PID: 592 Comm: forkforkfork Not tainted 6.5.0-rc7-00105-gae70c1e1f6f5-dirty #334
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x4a/0x80
print_report+0xcf/0x660
kasan_report+0xd4/0x110
folio_lock_anon_vma_read+0x129/0x4c0
rmap_walk_anon+0x1cc/0x290
folio_referenced+0x277/0x2a0
shrink_folio_list+0xb8c/0x1680
reclaim_folio_list+0xdc/0x1f0
reclaim_pages+0x211/0x280
madvise_cold_or_pageout_pte_range+0x812/0xb70
walk_pgd_range+0x70b/0xce0
__walk_page_range+0x343/0x360
walk_page_range+0x227/0x280
madvise_pageout+0x1cd/0x2d0
do_madvise+0x552/0x15a0
__x64_sys_madvise+0x62/0x70
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
[...]
</TASK>
Allocated by task 574:
kasan_save_stack+0x33/0x60
kasan_set_track+0x25/0x30
__kasan_slab_alloc+0x6e/0x70
kmem_cache_alloc+0xfd/0x2b0
anon_vma_fork+0x88/0x270
dup_mmap+0x87c/0xc10
copy_process+0x3399/0x3590
kernel_clone+0x10e/0x480
__do_sys_clone+0xa1/0xe0
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
Freed by task 0:
kasan_save_stack+0x33/0x60
kasan_set_track+0x25/0x30
kasan_save_free_info+0x2b/0x50
__kasan_slab_free+0xfe/0x180
slab_free_after_rcu_debug+0xad/0x200
rcu_core+0x638/0x1620
__do_softirq+0x14c/0x581
Last potentially related work creation:
kasan_save_stack+0x33/0x60
__kasan_record_aux_stack+0x94/0xa0
__call_rcu_common.constprop.0+0x47/0x730
__put_anon_vma+0x6e/0x150
unlink_anon_vmas+0x277/0x2e0
vma_complete+0x341/0x580
vma_merge+0x613/0xff0
mprotect_fixup+0x1c0/0x510
do_mprotect_pkey+0x5a7/0x710
__x64_sys_mprotect+0x47/0x60
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
Second to last potentially related work creation:
[...]
The buggy address belongs to the object at ffff888004e85b00
which belongs to the cache anon_vma of size 192
The buggy address is located 0 bytes inside of
freed 192-byte region [ffff888004e85b00, ffff888004e85bc0)
The buggy address belongs to the physical page:
[...]
Memory state around the buggy address:
ffff888004e85a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff888004e85a80: 00 00 00 00 00 00 00 00 fc 00 00 fc fc fc fc fc
>ffff888004e85b00: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff888004e85b80: fb fb fb fb fb fb fb fb fc 00 00 fc fc fc fc fc
ffff888004e85c00: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
include/linux/kasan.h | 6 ++++
include/linux/slub_def.h | 3 ++
lib/Kconfig.kasan | 2 ++
mm/Kconfig.debug | 21 +++++++++++++
mm/kasan/common.c | 15 ++++++++-
mm/slub.c | 66 +++++++++++++++++++++++++++++++++++++---
6 files changed, 107 insertions(+), 6 deletions(-)
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 819b6bc8ac08..45e07caf4704 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -229,6 +229,8 @@ static __always_inline bool kasan_check_byte(const void *addr)
return true;
}
+size_t kasan_align(size_t size);
+
#else /* CONFIG_KASAN */
static inline void kasan_unpoison_range(const void *address, size_t size) {}
@@ -278,6 +280,10 @@ static inline bool kasan_check_byte(const void *address)
{
return true;
}
+static inline size_t kasan_align(size_t size)
+{
+ return size;
+}
#endif /* CONFIG_KASAN */
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index deb90cf4bffb..b87be8fce64a 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -120,6 +120,9 @@ struct kmem_cache {
int refcount; /* Refcount for slab cache destroy */
void (*ctor)(void *);
unsigned int inuse; /* Offset to metadata */
+#ifdef CONFIG_SLUB_RCU_DEBUG
+ unsigned int debug_rcu_head_offset;
+#endif
unsigned int align; /* Alignment */
unsigned int red_left_pad; /* Left redzone padding size */
const char *name; /* Name (only for display!) */
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index fdca89c05745..7ff7de96c6e4 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -79,6 +79,7 @@ config KASAN_GENERIC
depends on HAVE_ARCH_KASAN && CC_HAS_KASAN_GENERIC
depends on CC_HAS_WORKING_NOSANITIZE_ADDRESS
select SLUB_DEBUG if SLUB
+ select SLUB_RCU_DEBUG if SLUB_DEBUG
select CONSTRUCTORS
help
Enables Generic KASAN.
@@ -96,6 +97,7 @@ config KASAN_SW_TAGS
depends on HAVE_ARCH_KASAN_SW_TAGS && CC_HAS_KASAN_SW_TAGS
depends on CC_HAS_WORKING_NOSANITIZE_ADDRESS
select SLUB_DEBUG if SLUB
+ select SLUB_RCU_DEBUG if SLUB_DEBUG
select CONSTRUCTORS
help
Enables Software Tag-Based KASAN.
diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
index 018a5bd2f576..99cce7f0fbef 100644
--- a/mm/Kconfig.debug
+++ b/mm/Kconfig.debug
@@ -78,6 +78,27 @@ config SLUB_DEBUG_ON
off in a kernel built with CONFIG_SLUB_DEBUG_ON by specifying
"slub_debug=-".
+config SLUB_RCU_DEBUG
+ bool "Make use-after-free detection possible in TYPESAFE_BY_RCU caches"
+ depends on SLUB && SLUB_DEBUG
+ default n
+ help
+ Make SLAB_TYPESAFE_BY_RCU caches behave approximately as if the cache
+ was not marked as SLAB_TYPESAFE_BY_RCU and every caller used
+ kfree_rcu() instead.
+
+ This is intended for use in combination with KASAN, to enable KASAN to
+ detect use-after-free accesses in such caches.
+ (KFENCE is able to do that independent of this flag.)
+
+ This might degrade performance.
+
+ If you're using this for testing bugs / fuzzing and care about
+ catching all the bugs WAY more than performance, you might want to
+ also turn on CONFIG_RCU_STRICT_GRACE_PERIOD.
+
+ If unsure, say N.
+
config PAGE_OWNER
bool "Track page owner"
depends on DEBUG_KERNEL && STACKTRACE_SUPPORT
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 256930da578a..b4a3504f9f5e 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -191,6 +191,13 @@ void * __must_check __kasan_init_slab_obj(struct kmem_cache *cache,
if (kasan_requires_meta())
kasan_init_object_meta(cache, object);
+#ifdef CONFIG_SLUB_RCU_DEBUG
+ if (cache->flags & SLAB_TYPESAFE_BY_RCU) {
+ kasan_unpoison(object + cache->debug_rcu_head_offset,
+ sizeof(struct rcu_head), false);
+ }
+#endif /* CONFIG_SLUB_RCU_DEBUG */
+
/* Tag is ignored in set_tag() without CONFIG_KASAN_SW/HW_TAGS */
object = set_tag(object, assign_tag(cache, object, true));
@@ -218,7 +225,8 @@ static inline bool ____kasan_slab_free(struct kmem_cache *cache, void *object,
}
/* RCU slabs could be legally used after free within the RCU period */
- if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU))
+ if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU) &&
+ !IS_ENABLED(CONFIG_SLUB_RCU_DEBUG))
return false;
if (!kasan_byte_accessible(tagged_object)) {
@@ -450,3 +458,8 @@ bool __kasan_check_byte(const void *address, unsigned long ip)
}
return true;
}
+
+size_t kasan_align(size_t size)
+{
+ return round_up(size, KASAN_GRANULE_SIZE);
+}
diff --git a/mm/slub.c b/mm/slub.c
index e3b5d5c0eb3a..bae6c2bc1e5f 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1108,7 +1108,8 @@ static int check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
* A. Free pointer (if we cannot overwrite object on free)
* B. Tracking data for SLAB_STORE_USER
* C. Original request size for kmalloc object (SLAB_STORE_USER enabled)
- * D. Padding to reach required alignment boundary or at minimum
+ * D. RCU head for CONFIG_SLUB_RCU_DEBUG (with padding around it)
+ * E. Padding to reach required alignment boundary or at minimum
* one word if debugging is on to be able to detect writes
* before the word boundary.
*
@@ -1134,6 +1135,11 @@ static int check_pad_bytes(struct kmem_cache *s, struct slab *slab, u8 *p)
off += sizeof(unsigned int);
}
+#ifdef CONFIG_SLUB_RCU_DEBUG
+ if (s->flags & SLAB_TYPESAFE_BY_RCU)
+ off = kasan_align(s->debug_rcu_head_offset + sizeof(struct rcu_head));
+#endif /* CONFIG_SLUB_RCU_DEBUG */
+
off += kasan_metadata_size(s, false);
if (size_from_object(s) == off)
@@ -1751,12 +1757,17 @@ static bool freelist_corrupted(struct kmem_cache *s, struct slab *slab,
#endif
#endif /* CONFIG_SLUB_DEBUG */
+#ifdef CONFIG_SLUB_RCU_DEBUG
+static void slab_free_after_rcu_debug(struct rcu_head *rcu_head);
+#endif
+
/*
* Hooks for other subsystems that check memory allocations. In a typical
* production configuration these hooks all should produce no code at all.
*/
static __always_inline bool slab_free_hook(struct kmem_cache *s,
- void *x, bool init)
+ void *x, bool init,
+ bool after_rcu_delay)
{
kmemleak_free_recursive(x, s->flags);
kmsan_slab_free(s, x);
@@ -1766,8 +1777,18 @@ static __always_inline bool slab_free_hook(struct kmem_cache *s,
if (!(s->flags & SLAB_DEBUG_OBJECTS))
debug_check_no_obj_freed(x, s->object_size);
+#ifdef CONFIG_SLUB_RCU_DEBUG
+ /* kfence does its own RCU delay */
+ if ((s->flags & SLAB_TYPESAFE_BY_RCU) && !after_rcu_delay &&
+ !is_kfence_address(x)) {
+ call_rcu(kasan_reset_tag(x) + s->debug_rcu_head_offset,
+ slab_free_after_rcu_debug);
+ return true;
+ }
+#endif /* CONFIG_SLUB_RCU_DEBUG */
+
/* Use KCSAN to help debug racy use-after-free. */
- if (!(s->flags & SLAB_TYPESAFE_BY_RCU))
+ if (!(s->flags & SLAB_TYPESAFE_BY_RCU) || after_rcu_delay)
__kcsan_check_access(x, s->object_size,
KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT);
@@ -1802,7 +1823,7 @@ static inline bool slab_free_freelist_hook(struct kmem_cache *s,
void *old_tail = *tail ? *tail : *head;
if (is_kfence_address(next)) {
- slab_free_hook(s, next, false);
+ slab_free_hook(s, next, false, false);
return true;
}
@@ -1815,7 +1836,7 @@ static inline bool slab_free_freelist_hook(struct kmem_cache *s,
next = get_freepointer(s, object);
/* If object's reuse doesn't have to be delayed */
- if (!slab_free_hook(s, object, slab_want_init_on_free(s))) {
+ if (!slab_free_hook(s, object, slab_want_init_on_free(s), false)) {
/* Move object to the new freelist */
set_freepointer(s, object, *head);
*head = object;
@@ -3802,6 +3823,31 @@ static __fastpath_inline void slab_free(struct kmem_cache *s, struct slab *slab,
do_slab_free(s, slab, head, tail, cnt, addr);
}
+#ifdef CONFIG_SLUB_RCU_DEBUG
+static void slab_free_after_rcu_debug(struct rcu_head *rcu_head)
+{
+ struct slab *slab = virt_to_slab(rcu_head);
+ struct kmem_cache *s;
+ void *object;
+
+ if (WARN_ON(is_kfence_address(rcu_head)))
+ return;
+
+ /* find the object and the cache again */
+ if (WARN_ON(!slab))
+ return;
+ s = slab->slab_cache;
+ if (WARN_ON(!(s->flags & SLAB_TYPESAFE_BY_RCU)))
+ return;
+ object = (void *)rcu_head - s->debug_rcu_head_offset;
+
+ /* resume freeing */
+ if (slab_free_hook(s, object, slab_want_init_on_free(s), true))
+ return;
+ do_slab_free(s, slab, object, NULL, 1, _THIS_IP_);
+}
+#endif /* CONFIG_SLUB_RCU_DEBUG */
+
#ifdef CONFIG_KASAN_GENERIC
void ___cache_free(struct kmem_cache *cache, void *x, unsigned long addr)
{
@@ -4443,6 +4489,16 @@ static int calculate_sizes(struct kmem_cache *s)
if (flags & SLAB_KMALLOC)
size += sizeof(unsigned int);
}
+
+#ifdef CONFIG_SLUB_RCU_DEBUG
+ if (flags & SLAB_TYPESAFE_BY_RCU) {
+ size = kasan_align(size);
+ size = ALIGN(size, __alignof__(struct rcu_head));
+ s->debug_rcu_head_offset = size;
+ size += sizeof(struct rcu_head);
+ size = kasan_align(size);
+ }
+#endif /* CONFIG_SLUB_RCU_DEBUG */
#endif
kasan_cache_create(s, &size, &s->flags);
base-commit: 4f9e7fabf8643003afefc172e62dd276686f016e
--
2.42.0.rc1.204.g551eb34607-goog
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
