kasan: catch invalid free before SLUB reinitializes the object

Currently, when KASAN is combined with init-on-free behavior, the
initialization happens before KASAN's "invalid free" checks.

More importantly, a subsequent commit will want to RCU-delay the actual
SLUB freeing of an object, and we'd like KASAN to still validate
synchronously that freeing the object is permitted. (Otherwise this
change will make the existing testcase kmem_cache_invalid_free fail.)

So add a new KASAN hook that allows KASAN to pre-validate a
kmem_cache_free() operation before SLUB actually starts modifying the
object or its metadata.

Inside KASAN, this:

 - moves checks from poison_slab_object() into check_slab_allocation()
 - moves kasan_arch_is_ready() up into callers of poison_slab_object()
 - removes "ip" argument of poison_slab_object() and __kasan_slab_free()
   (since those functions no longer do any reporting)

Acked-by: Vlastimil Babka <vbabka@suse.cz> #slub
Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 70d6a8f6e25dae8c4ecc8b10f7ea69be23a4513d..1570c719117664a25541077b6d22f92579459742 100644 (file)
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -175,13 +175,55 @@ static __always_inline void * __must_check kasan_init_slab_obj(
        return (void *)object;
 }
 
-bool __kasan_slab_free(struct kmem_cache *s, void *object,
-                       unsigned long ip, bool init);
+bool __kasan_slab_pre_free(struct kmem_cache *s, void *object,
+                       unsigned long ip);
+/**
+ * kasan_slab_pre_free - Check whether freeing a slab object is safe.
+ * @object: Object to be freed.
+ *
+ * This function checks whether freeing the given object is safe. It may
+ * check for double-free and invalid-free bugs and report them.
+ *
+ * This function is intended only for use by the slab allocator.
+ *
+ * @Return true if freeing the object is unsafe; false otherwise.
+ */
+static __always_inline bool kasan_slab_pre_free(struct kmem_cache *s,
+                                               void *object)
+{
+       if (kasan_enabled())
+               return __kasan_slab_pre_free(s, object, _RET_IP_);
+       return false;
+}
+
+bool __kasan_slab_free(struct kmem_cache *s, void *object, bool init);
+/**
+ * kasan_slab_free - Poison, initialize, and quarantine a slab object.
+ * @object: Object to be freed.
+ * @init: Whether to initialize the object.
+ *
+ * This function informs that a slab object has been freed and is not
+ * supposed to be accessed anymore, except for objects in
+ * SLAB_TYPESAFE_BY_RCU caches.
+ *
+ * For KASAN modes that have integrated memory initialization
+ * (kasan_has_integrated_init() == true), this function also initializes
+ * the object's memory. For other modes, the @init argument is ignored.
+ *
+ * This function might also take ownership of the object to quarantine it.
+ * When this happens, KASAN will defer freeing the object to a later
+ * stage and handle it internally until then. The return value indicates
+ * whether KASAN took ownership of the object.
+ *
+ * This function is intended only for use by the slab allocator.
+ *
+ * @Return true if KASAN took ownership of the object; false otherwise.
+ */
 static __always_inline bool kasan_slab_free(struct kmem_cache *s,
                                                void *object, bool init)
 {
        if (kasan_enabled())
-               return __kasan_slab_free(s, object, _RET_IP_, init);
+               return __kasan_slab_free(s, object, init);
        return false;
 }
 
@@ -371,6 +413,12 @@ static inline void *kasan_init_slab_obj(struct kmem_cache *cache,
 {
        return (void *)object;
 }
+
+static inline bool kasan_slab_pre_free(struct kmem_cache *s, void *object)
+{
+       return false;
+}
+
 static inline bool kasan_slab_free(struct kmem_cache *s, void *object, bool init)
 {
        return false;

diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 85e7c6b4575c217aa9bdb5f587d47e4db7b10105..f26bbc087b3b1fe6872744afb058a02e13779d9a 100644 (file)
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -208,15 +208,12 @@ void * __must_check __kasan_init_slab_obj(struct kmem_cache *cache,
        return (void *)object;
 }
 
-static inline bool poison_slab_object(struct kmem_cache *cache, void *object,
-                                     unsigned long ip, bool init)
+/* Returns true when freeing the object is not safe. */
+static bool check_slab_allocation(struct kmem_cache *cache, void *object,
+                                 unsigned long ip)
 {
-       void *tagged_object;
-
-       if (!kasan_arch_is_ready())
-               return false;
+       void *tagged_object = object;
 
-       tagged_object = object;
        object = kasan_reset_tag(object);
 
        if (unlikely(nearest_obj(cache, virt_to_slab(object), object) != object)) {
@@ -224,37 +221,46 @@ static inline bool poison_slab_object(struct kmem_cache *cache, void *object,
                return true;
        }
 
-       /* RCU slabs could be legally used after free within the RCU period. */
-       if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU))
-               return false;
-
        if (!kasan_byte_accessible(tagged_object)) {
                kasan_report_invalid_free(tagged_object, ip, KASAN_REPORT_DOUBLE_FREE);
                return true;
        }
 
+       return false;
+}
+
+static inline void poison_slab_object(struct kmem_cache *cache, void *object,
+                                     bool init)
+{
+       void *tagged_object = object;
+
+       object = kasan_reset_tag(object);
+
+       /* RCU slabs could be legally used after free within the RCU period. */
+       if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU))
+               return;
+
        kasan_poison(object, round_up(cache->object_size, KASAN_GRANULE_SIZE),
                        KASAN_SLAB_FREE, init);
 
        if (kasan_stack_collection_enabled())
                kasan_save_free_info(cache, tagged_object);
+}
 
-       return false;
+bool __kasan_slab_pre_free(struct kmem_cache *cache, void *object,
+                               unsigned long ip)
+{
+       if (!kasan_arch_is_ready() || is_kfence_address(object))
+               return false;
+       return check_slab_allocation(cache, object, ip);
 }
 
-bool __kasan_slab_free(struct kmem_cache *cache, void *object,
-                               unsigned long ip, bool init)
+bool __kasan_slab_free(struct kmem_cache *cache, void *object, bool init)
 {
-       if (is_kfence_address(object))
+       if (!kasan_arch_is_ready() || is_kfence_address(object))
                return false;
 
-       /*
-        * If the object is buggy, do not let slab put the object onto the
-        * freelist. The object will thus never be allocated again and its
-        * metadata will never get released.
-        */
-       if (poison_slab_object(cache, object, ip, init))
-               return true;
+       poison_slab_object(cache, object, init);
 
        /*
         * If the object is put into quarantine, do not let slab put the object
@@ -504,11 +510,16 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip)
                return true;
        }
 
-       if (is_kfence_address(ptr))
-               return false;
+       if (is_kfence_address(ptr) || !kasan_arch_is_ready())
+               return true;
 
        slab = folio_slab(folio);
-       return !poison_slab_object(slab->slab_cache, ptr, ip, false);
+
+       if (check_slab_allocation(slab->slab_cache, ptr, ip))
+               return false;
+
+       poison_slab_object(slab->slab_cache, ptr, false);
+       return true;
 }
 
 void __kasan_mempool_unpoison_object(void *ptr, size_t size, unsigned long ip)

diff --git a/mm/slub.c b/mm/slub.c
index c9d8a2497fd65e1f48cd933a67bf75ab8f9c117b..4946488cb5a76b656a9bb70e9d50df0ce3fb0e90 100644 (file)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2226,6 +2226,13 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init)
        if (kfence_free(x))
                return false;
 
+       /*
+        * Give KASAN a chance to notice an invalid free operation before we
+        * modify the object.
+        */
+       if (kasan_slab_pre_free(s, x))
+               return false;
+
        /*
         * As memory initialization might be integrated into KASAN,
         * kasan_slab_free and initialization memset's must be