Add object validation in ksize()
Subsystems (4)
v5 cover Add object validation in ksize()
This version fixes several build issues --
Previous version here:
http://lkml.kernel.org/r/[email protected]
Marco Elver (5):
mm/kasan: Introduce __kasan_check_{read,write} mm/kasan: Change kasan_check_{read,write} to return boolean 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 | 43 +++++++++++++++++++++++++++------ 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 | 46 ++++++++++++++++++++++++++++++++++++ mm/slob.c | 4 ++-- mm/slub.c | 14 ++--------- 12 files changed, 144 insertions(+), 65 deletions(-)
Signature · 1 line
2.22.0.410.gd8fdbe21b5-goog
PATCH v5 1/5 mm/kasan: Introduce __kasan_check_{read,write}
This introduces __kasan_check_{read,write}. __kasan_check functions may be used from anywhere, even compilation units that disable instrumentation selectively.
This change eliminates the need for the __KASAN_INTERNAL definition.
---
v5:
* Use #define for kasan_check_* in the __SANITIZE_ADDRESS__ case, as the inline functions conflict with the __no_sanitize_address attribute.
v3:
* Fix Formatting and split introduction of __kasan_check_* and returning bool into 2 patches.
--- include/linux/kasan-checks.h | 25 ++++++++++++++++++++++--- mm/kasan/common.c | 10 ++++------ 2 files changed, 26 insertions(+), 9 deletions(-)
include/linux/kasan-checks.h +22 -3
@@ -2,9 +2,28 @@#ifndef _LINUX_KASAN_CHECKS_H#define _LINUX_KASAN_CHECKS_H -#if defined(__SANITIZE_ADDRESS__) || defined(__KASAN_INTERNAL) -void kasan_check_read (const volatile void *p,unsigned int size); -void kasan_check_write (const volatile void *p,unsigned int size); +/* +* __kasan_check_*: Always available when KASAN is enabled. This may be used +* even in compilation units that selectively disable KASAN, but must use KASAN +* to validate access to an address. Never use these in header files! +*/ +#ifdef CONFIG_KASAN +void __kasan_check_read (const volatile void *p,unsigned int size); +void __kasan_check_write (const volatile void *p,unsigned int size); +#else +static inline void __kasan_check_read (const volatile void *p,unsigned int size) +{ } +static inline void __kasan_check_write (const volatile void *p,unsigned int size) +{ } +#endif + +/* +* kasan_check_*: Only available when the particular compilation unit has KASAN +* instrumentation enabled. May be used in header files. +*/ +#ifdef __SANITIZE_ADDRESS__ +#define kasan_check_read __kasan_check_read +#define kasan_check_write __kasan_check_write#else static inline void kasan_check_read (const volatile void *p,unsigned int size) { }
mm/kasan/common.c +4 -6
@@ -14,8 +14,6 @@* */ -#define __KASAN_INTERNAL -#include <linux/export.h> #include <linux/interrupt.h> #include <linux/init.h>
@@ -89,17 +87,17 @@ void kasan_disable_current(void) current->kasan_depth--; } -void kasan_check_read (const volatile void *p,unsigned int size) +void __kasan_check_read (const volatile void *p,unsigned int size) {check_memory_region ((unsigned long )p, size, false, _RET_IP_); } -EXPORT_SYMBOL (kasan_check_read); +EXPORT_SYMBOL (__kasan_check_read); -void kasan_check_write (const volatile void *p,unsigned int size) +void __kasan_check_write (const volatile void *p,unsigned int size) {check_memory_region ((unsigned long )p, size, true, _RET_IP_); } -EXPORT_SYMBOL (kasan_check_write); +EXPORT_SYMBOL (__kasan_check_write);#undef memsetvoid *memset (void *addr,int c,size_t len)
Signature · 1 line
2.22.0.410.gd8fdbe21b5-goog
PATCH v5 2/5 mm/kasan: Change kasan_check_{read,write} to return boolean
In reply to Marco ElverThis changes {,__}kasan_check_{read,write} functions to return a boolean denoting if the access was valid or not.
---
v5:
* Rebase on top of v5 of preceding patch.
* Include types.h for bool.
v3:
* Fix Formatting and split introduction of __kasan_check_* and returning bool into 2 patches.
--- include/linux/kasan-checks.h | 30 ++++++++++++++++++++---------- mm/kasan/common.c | 8 ++++---- mm/kasan/generic.c | 13 +++++++------ mm/kasan/kasan.h | 10 +++++++++- mm/kasan/tags.c | 12 +++++++----- 5 files changed, 47 insertions(+), 26 deletions(-)
include/linux/kasan-checks.h +20 -10
@@ -2,19 +2,25 @@#ifndef _LINUX_KASAN_CHECKS_H#define _LINUX_KASAN_CHECKS_H +#include <linux/types.h> +/* * __kasan_check_*: Always available when KASAN is enabled. This may be used * even in compilation units that selectively disable KASAN, but must use KASAN * to validate access to an address. Never use these in header files! */ #ifdef CONFIG_KASAN -void __kasan_check_read (const volatile void *p,unsigned int size); -void __kasan_check_write (const volatile void *p,unsigned int size); +bool __kasan_check_read (const volatile void *p,unsigned int size); +bool __kasan_check_write (const volatile void *p,unsigned int size);#else -static inline void __kasan_check_read (const volatile void *p,unsigned int size) -{ } -static inline void __kasan_check_write (const volatile void *p,unsigned int size) -{ } +static inline bool __kasan_check_read (const volatile void *p,unsigned int size) +{ +return true; +} +static inline bool __kasan_check_write (const volatile void *p,unsigned int size) +{ +return true; +}#endif /*
@@ -25,10 +31,14 @@ static inline void __kasan_check_write(const volatile void *p, unsigned int size#define kasan_check_read __kasan_check_read#define kasan_check_write __kasan_check_write#else -static inline void kasan_check_read (const volatile void *p,unsigned int size) -{ } -static inline void kasan_check_write (const volatile void *p,unsigned int size) -{ } +static inline bool kasan_check_read (const volatile void *p,unsigned int size) +{ +return true; +} +static inline bool kasan_check_write (const volatile void *p,unsigned int size) +{ +return true; +}#endif #endif
mm/kasan/common.c +4 -4
@@ -87,15 +87,15 @@ void kasan_disable_current(void) current->kasan_depth--; } -void __kasan_check_read (const volatile void *p,unsigned int size) +bool __kasan_check_read (const volatile void *p,unsigned int size) { -check_memory_region ((unsigned long )p, size, false, _RET_IP_); +return check_memory_region ((unsigned long )p, size, false, _RET_IP_); }EXPORT_SYMBOL (__kasan_check_read); -void __kasan_check_write (const volatile void *p,unsigned int size) +bool __kasan_check_write (const volatile void *p,unsigned int size) { -check_memory_region ((unsigned long )p, size, true, _RET_IP_); +return check_memory_region ((unsigned long )p, size, true, _RET_IP_); }EXPORT_SYMBOL (__kasan_check_write);
mm/kasan/generic.c +7 -6
@@ -166,29 +166,30 @@ static __always_inline bool memory_is_poisoned(unsigned long addr, size_t size)return memory_is_poisoned_n (addr, size); } -static __always_inline void check_memory_region_inline(unsigned long addr, +static __always_inline bool check_memory_region_inline(unsigned long addr,size_t size,bool write,unsigned long ret_ip) {if (unlikely (size ==0 )) -return ; +return true;if (unlikely ((void *)addr <kasan_shadow_to_mem ((void *)KASAN_SHADOW_START ))) {kasan_report (addr, size, write, ret_ip); -return ; +return false; }if (likely (!memory_is_poisoned (addr, size))) -return ; +return true;kasan_report (addr, size, write, ret_ip); +return false; } -void check_memory_region (unsigned long addr,size_t size,bool write, +bool check_memory_region (unsigned long addr,size_t size,bool write,unsigned long ret_ip) { -check_memory_region_inline (addr, size, write, ret_ip); +return check_memory_region_inline (addr, size, write, ret_ip); }void kasan_cache_shrink (struct kmem_cache *cache)
mm/kasan/kasan.h +9 -1
@@ -123,7 +123,15 @@ static inline bool addr_has_shadow(const void *addr)void kasan_poison_shadow (const void *address,size_t size,u8 value); -void check_memory_region (unsigned long addr,size_t size,bool write, +/** +* check_memory_region - Check memory region, and report if invalid access. +* @addr: the accessed address +* @size: the accessed size +* @write: true if access is a write access +* @ret_ip: return address +* @return: true if access was valid, false if invalid +*/ +bool check_memory_region (unsigned long addr,size_t size,bool write,unsigned long ret_ip);void *find_first_bad_addr (void *addr,size_t size);
mm/kasan/tags.c +7 -5
@@ -76,7 +76,7 @@ void *kasan_reset_tag(const void *addr)return reset_tag (addr); } -void check_memory_region (unsigned long addr,size_t size,bool write, +bool check_memory_region (unsigned long addr,size_t size,bool write,unsigned long ret_ip) {u8 tag;
@@ -84,7 +84,7 @@ void check_memory_region(unsigned long addr, size_t size, bool write,void *untagged_addr;if (unlikely (size ==0 )) -return ; +return true; tag =get_tag ((const void *)addr);
@@ -106,22 +106,24 @@ void check_memory_region(unsigned long addr, size_t size, bool write,* set to KASAN_TAG_KERNEL (0xFF)). */ if (tag ==KASAN_TAG_KERNEL ) -return ; +return true; untagged_addr =reset_tag ((const void *)addr);if (unlikely (untagged_addr <kasan_shadow_to_mem ((void *)KASAN_SHADOW_START ))) {kasan_report (addr, size, write, ret_ip); -return ; +return false; } shadow_first =kasan_mem_to_shadow (untagged_addr); shadow_last =kasan_mem_to_shadow (untagged_addr + size -1 );for (shadow = shadow_first; shadow <= shadow_last; shadow++) {if (*shadow != tag) {kasan_report (addr, size, write, ret_ip); -return ; +return false; } } + +return true; }#define DEFINE_HWASAN_LOAD_STORE (size) \
Signature · 1 line
2.22.0.410.gd8fdbe21b5-goog
PATCH v5 3/5 lib/test_kasan: Add test for double-kzfree detection
In reply to Marco ElverAdds a simple test that checks if double-kzfree is being detected correctly.
--- lib/test_kasan.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
lib/test_kasan.c +17 -0
@@ -619,6 +619,22 @@ static noinline void __init kasan_strings(void)strnlen (ptr,1 ); } +static noinline void __init kmalloc_double_kzfree(void) +{ +char *ptr; +size_t size =16 ; + +pr_info ("double-free (kzfree)\n" ); + ptr =kmalloc (size,GFP_KERNEL ); +if (!ptr) { +pr_err ("Allocation failed\n" ); +return ; + } + +kzfree (ptr); +kzfree (ptr); +} +static int __init kmalloc_tests_init (void ) {/*
@@ -660,6 +676,7 @@ static int __init kmalloc_tests_init(void)kasan_memchr ();kasan_memcmp ();kasan_strings (); +kmalloc_double_kzfree ();kasan_restore_multi_shot (multishot);
Signature · 1 line
2.22.0.410.gd8fdbe21b5-goog
PATCH v5 4/5 mm/slab: Refactor common ksize KASAN logic into slab_common.c
In reply to Marco ElverThis refactors common code of ksize() between the various allocators into slab_common.c: __ksize() is the allocator-specific implementation without instrumentation, whereas ksize() includes the required KASAN logic.
--- include/linux/slab.h | 1 + mm/slab.c | 28 ++++++---------------------- mm/slab_common.c | 26 ++++++++++++++++++++++++++ mm/slob.c | 4 ++-- mm/slub.c | 14 ++------------ 5 files changed, 37 insertions(+), 36 deletions(-)
include/linux/slab.h +1 -0
@@ -184,6 +184,7 @@ void * __must_check __krealloc(const void *, size_t, gfp_t);void *__must_check krealloc (const void *,size_t ,gfp_t );void kfree (const void *);void kzfree (const void *); +size_t __ksize (const void *);size_t ksize (const void *);#ifdef CONFIG_HAVE_HARDENED_USERCOPY_ALLOCATOR
mm/slab.c +6 -22
@@ -4204,33 +4204,17 @@ void __check_heap_object(const void *ptr, unsigned long n, struct page *page,#endif /* CONFIG_HARDENED_USERCOPY */ /** -* ksize - get the actual amount of memory allocated for a given object -* @objp: Pointer to the object +* __ksize -- Uninstrumented ksize. * -* kmalloc may internally round up allocations and return more memory -* than requested. ksize() can be used to determine the actual amount of -* memory allocated. The caller may use this additional memory, even though -* a smaller amount of memory was initially specified with the kmalloc call. -* The caller must guarantee that objp points to a valid object previously -* allocated with either kmalloc() or kmem_cache_alloc(). The object -* must not be freed during the duration of the call. -* -* Return: size of the actual memory used by @objp in bytes +* Unlike ksize(), __ksize() is uninstrumented, and does not provide the same +* safety checks as ksize() with KASAN instrumentation enabled. */ -size_t ksize (const void *objp) +size_t __ksize (const void *objp) { -size_t size; -BUG_ON (!objp);if (unlikely (objp ==ZERO_SIZE_PTR ))return 0 ; - size =virt_to_cache (objp)->object_size; -/* We assume that ksize callers could use the whole allocated area, -* so we need to unpoison this area. -*/ -kasan_unpoison_shadow (objp, size); - -return size; +return virt_to_cache (objp)->object_size; } -EXPORT_SYMBOL (ksize); +EXPORT_SYMBOL (__ksize);
mm/slab_common.c +26 -0
@@ -1597,6 +1597,32 @@ void kzfree(const void *p) }EXPORT_SYMBOL (kzfree); +/** +* ksize - get the actual amount of memory allocated for a given object +* @objp: Pointer to the object +* +* kmalloc may internally round up allocations and return more memory +* than requested. ksize() can be used to determine the actual amount of +* memory allocated. The caller may use this additional memory, even though +* a smaller amount of memory was initially specified with the kmalloc call. +* The caller must guarantee that objp points to a valid object previously +* allocated with either kmalloc() or kmem_cache_alloc(). The object +* must not be freed during the duration of the call. +* +* Return: size of the actual memory used by @objp in bytes +*/ +size_t ksize (const void *objp) +{ +size_t size =__ksize (objp); +/* +* We assume that ksize callers could use whole allocated area, +* so we need to unpoison this area. +*/ +kasan_unpoison_shadow (objp, size); +return size; +} +EXPORT_SYMBOL (ksize); +/* Tracepoints definitions. */ EXPORT_TRACEPOINT_SYMBOL (kmalloc);EXPORT_TRACEPOINT_SYMBOL (kmem_cache_alloc);
mm/slob.c +2 -2
@@ -527,7 +527,7 @@ void kfree(const void *block)EXPORT_SYMBOL (kfree);/* can't use ksize for kmem_cache_alloc memory, only kmalloc */ -size_t ksize (const void *block) +size_t __ksize (const void *block) {struct page *sp;int align;
@@ -545,7 +545,7 @@ size_t ksize(const void *block) m = (unsigned int *)(block - align);return SLOB_UNITS (*m) *SLOB_UNIT ; } -EXPORT_SYMBOL (ksize); +EXPORT_SYMBOL (__ksize);int __kmem_cache_create (struct kmem_cache *c,slab_flags_t flags) {
mm/slub.c +2 -12
@@ -3901,7 +3901,7 @@ void __check_heap_object(const void *ptr, unsigned long n, struct page *page, }#endif /* CONFIG_HARDENED_USERCOPY */ -static size_t __ksize (const void *object) +size_t __ksize (const void *object) {struct page *page;
@@ -3917,17 +3917,7 @@ static size_t __ksize(const void *object)return slab_ksize (page->slab_cache); } - -size_t ksize (const void *object) -{ -size_t size =__ksize (object); -/* We assume that ksize callers could use whole allocated area, -* so we need to unpoison this area. -*/ -kasan_unpoison_shadow (object, size); -return size; -} -EXPORT_SYMBOL (ksize); +EXPORT_SYMBOL (__ksize);void kfree (const void *x) {
Signature · 1 line
2.22.0.410.gd8fdbe21b5-goog
Re: [PATCH v5 4/5] mm/slab: Refactor common ksize KASAN logic into slab_common.c
Quoted 4 lines · Marco Elver
This refactors common code of
ksize()between the various allocators intoslab_common.c:__ksize()is the allocator-specific implementation without instrumentation, whereasksize()includes the required KASAN logic.
PATCH v5 5/5 mm/kasan: Add object validation in ksize()
In reply to Marco Elverksize() 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
---
v4:
* Prefer WARN_ON_ONCE() instead of BUG_ON().
--- include/linux/kasan.h | 7 +++++-- mm/slab_common.c | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-)
include/linux/kasan.h +5 -2
@@ -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 );
mm/slab_common.c +21 -1
@@ -1613,7 +1613,27 @@ EXPORT_SYMBOL(kzfree);*/ size_t ksize (const void *objp) { -size_t size =__ksize (objp); +size_t size; + +if (WARN_ON_ONCE (!objp)) +return 0 ; +/* +* 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.
Signature · 1 line
2.22.0.410.gd8fdbe21b5-goog
Quoted 6 lines · Marco Elver
This version fixes several build issues --
Reported-by kbuild test robot <[email protected]>Previous version here:
http://lkml.kernel.org/r/[email protected]
Quoted 7 lines
Marco Elver (5):
mm/kasan: Introduce __kasan_check_{read,write} mm/kasan: Change kasan_check_{read,write} to return booleanlib/test_kasan: Add test for double-kzfree detection mm/slab: Refactor common ksize KASAN logic intoslab_common.cmm/kasan: Add object validation inksize()