2019-06-26 12:25:01

by Marco Elver

[permalink] [raw]
Subject: [PATCH v2 0/4] mm/kasan: Add object validation in ksize()

This patch series adds proper validation of an object in ksize() --
ksize() has been unconditionally unpoisoning the entire memory region
associated with an allocation. This can lead to various undetected bugs.

To correctly address this for all allocators, and a requirement that we
still need access to an unchecked ksize(), we introduce __ksize(), and
then refactor the common logic in ksize() to slab_common.c.

Furthermore, we introduce __kasan_check_{read,write}, which can be used
even if KASAN is disabled in a compilation unit (as is the case for
slab_common.c). See inline comment for why __kasan_check_read() is
chosen to check validity of an object inside ksize().

Previous version:
http://lkml.kernel.org/r/[email protected]

v2:
* Complete rewrite of patch, refactoring ksize() and relying on
kasan_check_read for validation.

Marco Elver (4):
mm/kasan: Introduce __kasan_check_{read,write}
lib/test_kasan: Add test for double-kzfree detection
mm/slab: Refactor common ksize KASAN logic into slab_common.c
mm/kasan: Add object validation in ksize()

include/linux/kasan-checks.h | 35 ++++++++++++++++++++++------
include/linux/kasan.h | 7 ++++--
include/linux/slab.h | 1 +
lib/test_kasan.c | 17 ++++++++++++++
mm/kasan/common.c | 14 +++++------
mm/kasan/generic.c | 13 ++++++-----
mm/kasan/kasan.h | 10 +++++++-
mm/kasan/tags.c | 12 ++++++----
mm/slab.c | 28 +++++-----------------
mm/slab_common.c | 45 ++++++++++++++++++++++++++++++++++++
mm/slob.c | 4 ++--
mm/slub.c | 14 ++---------
12 files changed, 135 insertions(+), 65 deletions(-)

--
2.22.0.410.gd8fdbe21b5-goog


2019-06-26 12:25:58

by Marco Elver

[permalink] [raw]
Subject: [PATCH v2 4/4] mm/kasan: Add object validation in ksize()

ksize() has been unconditionally unpoisoning the whole shadow memory region
associated with an allocation. This can lead to various undetected bugs,
for example, double-kzfree().

Specifically, kzfree() uses ksize() to determine the actual allocation
size, and subsequently zeroes the memory. Since ksize() used to just
unpoison the whole shadow memory region, no invalid free was detected.

This patch addresses this as follows:

1. Add a check in ksize(), and only then unpoison the memory region.

2. Preserve kasan_unpoison_slab() semantics by explicitly unpoisoning
the shadow memory region using the size obtained from __ksize().

Tested:
1. With SLAB allocator: a) normal boot without warnings; b) verified the
added double-kzfree() is detected.
2. With SLUB allocator: a) normal boot without warnings; b) verified the
added double-kzfree() is detected.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199359
Signed-off-by: Marco Elver <[email protected]>
Cc: Andrey Ryabinin <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: Alexander Potapenko <[email protected]>
Cc: Andrey Konovalov <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
include/linux/kasan.h | 7 +++++--
mm/slab_common.c | 21 ++++++++++++++++++++-
2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index b40ea104dd36..cc8a03cc9674 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -76,8 +76,11 @@ void kasan_free_shadow(const struct vm_struct *vm);
int kasan_add_zero_shadow(void *start, unsigned long size);
void kasan_remove_zero_shadow(void *start, unsigned long size);

-size_t ksize(const void *);
-static inline void kasan_unpoison_slab(const void *ptr) { ksize(ptr); }
+size_t __ksize(const void *);
+static inline void kasan_unpoison_slab(const void *ptr)
+{
+ kasan_unpoison_shadow(ptr, __ksize(ptr));
+}
size_t kasan_metadata_size(struct kmem_cache *cache);

bool kasan_save_enable_multi_shot(void);
diff --git a/mm/slab_common.c b/mm/slab_common.c
index b7c6a40e436a..ba4a859261d5 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1613,7 +1613,26 @@ EXPORT_SYMBOL(kzfree);
*/
size_t ksize(const void *objp)
{
- size_t size = __ksize(objp);
+ size_t size;
+
+ BUG_ON(!objp);
+ /*
+ * We need to check that the pointed to object is valid, and only then
+ * unpoison the shadow memory below. We use __kasan_check_read(), to
+ * generate a more useful report at the time ksize() is called (rather
+ * than later where behaviour is undefined due to potential
+ * use-after-free or double-free).
+ *
+ * If the pointed to memory is invalid we return 0, to avoid users of
+ * ksize() writing to and potentially corrupting the memory region.
+ *
+ * We want to perform the check before __ksize(), to avoid potentially
+ * crashing in __ksize() due to accessing invalid metadata.
+ */
+ if (unlikely(objp == ZERO_SIZE_PTR) || !__kasan_check_read(objp, 1))
+ return 0;
+
+ size = __ksize(objp);
/*
* We assume that ksize callers could use whole allocated area,
* so we need to unpoison this area.
--
2.22.0.410.gd8fdbe21b5-goog