kasan: HW_TAGS tests support and fixes
Subsystems (8)
cover kasan: HW_TAGS tests support and fixes
This patchset adds support for running KASAN-KUnit tests with the hardware tag-based mode and also contains a few fixes.
Andrey Konovalov (11):
kasan: prefix exported functions with kasan_
kasan: clarify HW_TAGS impact on TBI
kasan: clean up comments in tests
kasan: add match-all tag tests
kasan, arm64: allow using KUnit tests with HW_TAGS mode
kasan: rename CONFIG_TEST_KASAN_MODULE
kasan: add compiler barriers to KUNIT_EXPECT_KASAN_FAIL
kasan: adopt kmalloc_uaf2 test to HW_TAGS mode
kasan: fix memory corruption in kasan_bitops_tags test
kasan: fix bug detection via ksize for HW_TAGS mode
kasan: add proper page allocator tests
Documentation/dev-tools/kasan.rst | 22 +- arch/arm64/include/asm/memory.h | 1 + arch/arm64/include/asm/mte-kasan.h | 12 ++ arch/arm64/kernel/mte.c | 12 ++ arch/arm64/mm/fault.c | 16 +- include/linux/kasan-checks.h | 6 + include/linux/kasan.h | 13 ++ lib/Kconfig.kasan | 6 +- lib/Makefile | 2 +- lib/test_kasan.c | 312 +++++++++++++++++++++++------ lib/test_kasan_module.c | 5 +- mm/kasan/common.c | 56 +++--- mm/kasan/generic.c | 38 ++-- mm/kasan/kasan.h | 69 ++++--- mm/kasan/quarantine.c | 22 +- mm/kasan/report.c | 13 +- mm/kasan/report_generic.c | 8 +- mm/kasan/report_hw_tags.c | 8 +- mm/kasan/report_sw_tags.c | 8 +- mm/kasan/shadow.c | 26 +-- mm/kasan/sw_tags.c | 20 +- mm/slab_common.c | 15 +- tools/objtool/check.c | 2 +- 23 files changed, 484 insertions(+), 208 deletions(-)
Signature · 1 line
2.29.2.729.g45daf8777d-goog
PATCH 1/11 kasan: prefix exported functions with kasan_
There's a number of internal KASAN functions that are used across multiple source code files and therefore aren't marked as static inline. To avoid littering the kernel function names list with generic functions, prefix all such KASAN functions with kasan_.
As a part of this change:
- Rename internal (un)poison_range() to kasan_(un)poison() (no _range) to avoid name collision with a public kasan_unpoison_range().
- Rename check_memory_region() to kasan_check_range(), as it seems to be a more fitting name.
--- mm/kasan/common.c | 47 +++++++++++++++++++------------------- mm/kasan/generic.c | 36 ++++++++++++++--------------- mm/kasan/kasan.h | 48 +++++++++++++++++++-------------------- mm/kasan/quarantine.c | 22 +++++++++--------- mm/kasan/report.c | 13 ++++++----- mm/kasan/report_generic.c | 8 +++---- mm/kasan/report_hw_tags.c | 8 +++---- mm/kasan/report_sw_tags.c | 8 +++---- mm/kasan/shadow.c | 26 ++++++++++----------- mm/kasan/sw_tags.c | 16 ++++++------- tools/objtool/check.c | 2 +- 11 files changed, 117 insertions(+), 117 deletions(-)
mm/kasan/common.c +23 -24
@@ -60,7 +60,7 @@ void kasan_disable_current(void)void __kasan_unpoison_range (const void *address,size_t size) { -unpoison_range (address, size); +kasan_unpoison (address, size); }#if CONFIG_KASAN_STACK
@@ -69,7 +69,7 @@ void kasan_unpoison_task_stack(struct task_struct *task) {void *base =task_stack_page (task); -unpoison_range (base,THREAD_SIZE ); +kasan_unpoison (base,THREAD_SIZE ); }/* Unpoison the stack for the current task beyond a watermark sp value. */
@@ -82,7 +82,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)*/ void *base = (void *)((unsigned long )watermark & ~(THREAD_SIZE -1 )); - unpoison_range(base, watermark - base); + kasan_unpoison(base, watermark - base); }#endif /* CONFIG_KASAN_STACK */
@@ -105,18 +105,17 @@ void __kasan_alloc_pages(struct page *page, unsigned int order)if (unlikely (PageHighMem (page)))return ; - tag =random_tag (); + tag =kasan_random_tag ();for (i =0 ; i < (1 << order); i++)page_kasan_tag_set (page + i, tag); -unpoison_range (page_address (page),PAGE_SIZE << order); +kasan_unpoison (page_address (page),PAGE_SIZE << order); }void __kasan_free_pages (struct page *page,unsigned int order) {if (likely (!PageHighMem (page))) -poison_range (page_address (page), -PAGE_SIZE << order, -KASAN_FREE_PAGE ); +kasan_poison (page_address (page),PAGE_SIZE << order, +KASAN_FREE_PAGE ); }/*
@@ -246,18 +245,18 @@ void __kasan_poison_slab(struct page *page)for (i =0 ; i <compound_nr (page); i++)page_kasan_tag_reset (page + i); -poison_range (page_address (page),page_size (page), +kasan_poison (page_address (page),page_size (page),KASAN_KMALLOC_REDZONE ); }void __kasan_unpoison_object_data (struct kmem_cache *cache,void *object) { -unpoison_range (object, cache->object_size); +kasan_unpoison (object, cache->object_size); }void __kasan_poison_object_data (struct kmem_cache *cache,void *object) { -poison_range (object, cache->object_size,KASAN_KMALLOC_REDZONE ); +kasan_poison (object, cache->object_size,KASAN_KMALLOC_REDZONE ); }/*
@@ -294,7 +293,7 @@ static u8 assign_tag(struct kmem_cache *cache, const void *object,* set, assign a tag when the object is being allocated (init == false). */ if (!cache->ctor && !(cache->flags &SLAB_TYPESAFE_BY_RCU )) -return init ?KASAN_TAG_KERNEL :random_tag (); +return init ?KASAN_TAG_KERNEL :kasan_random_tag ();/* For caches that either have a constructor or SLAB_TYPESAFE_BY_RCU: */ #ifdef CONFIG_SLAB
@@ -305,7 +304,7 @@ static u8 assign_tag(struct kmem_cache *cache, const void *object,* For SLUB assign a random tag during slab creation, otherwise reuse * the already assigned tag. */ -return init ?random_tag () :get_tag (object); +return init ?kasan_random_tag () :get_tag (object);#endif }
@@ -346,12 +345,12 @@ static bool ____kasan_slab_free(struct kmem_cache *cache, void *object,if (unlikely (cache->flags &SLAB_TYPESAFE_BY_RCU ))return false; -if (check_invalid_free (tagged_object)) { +if (kasan_check_invalid_free (tagged_object)) {kasan_report_invalid_free (tagged_object, ip);return true; } -poison_range (object, cache->object_size,KASAN_KMALLOC_FREE ); +kasan_poison (object, cache->object_size,KASAN_KMALLOC_FREE );if (!kasan_stack_collection_enabled ())return false;
@@ -361,7 +360,7 @@ static bool ____kasan_slab_free(struct kmem_cache *cache, void *object,kasan_set_free_info (cache, object, tag); -return quarantine_put (cache, object); +return kasan_quarantine_put (cache, object); }bool __kasan_slab_free (struct kmem_cache *cache,void *object,unsigned long ip)
@@ -386,7 +385,7 @@ void __kasan_slab_free_mempool(void *ptr, unsigned long ip)kasan_report_invalid_free (ptr, ip);return ; } -poison_range (ptr,page_size (page),KASAN_FREE_PAGE ); +kasan_poison (ptr,page_size (page),KASAN_FREE_PAGE ); }else {____kasan_slab_free (page->slab_cache, ptr, ip, false); }
@@ -409,7 +408,7 @@ static void *____kasan_kmalloc(struct kmem_cache *cache, const void *object,u8 tag;if (gfpflags_allow_blocking (flags)) -quarantine_reduce (); +kasan_quarantine_reduce ();if (unlikely (object ==NULL ))return NULL ;
@@ -421,9 +420,9 @@ static void *____kasan_kmalloc(struct kmem_cache *cache, const void *object, tag =assign_tag (cache, object, false, keep_tag);/* Tag is ignored in set_tag without CONFIG_KASAN_SW/HW_TAGS */ -unpoison_range (set_tag (object, tag), size); -poison_range ((void *)redzone_start, redzone_end - redzone_start, -KASAN_KMALLOC_REDZONE ); +kasan_unpoison (set_tag (object, tag), size); +kasan_poison ((void *)redzone_start, redzone_end - redzone_start, +KASAN_KMALLOC_REDZONE );if (kasan_stack_collection_enabled ())set_alloc_info (cache, (void *)object, flags);
@@ -452,7 +451,7 @@ void * __must_check __kasan_kmalloc_large(const void *ptr, size_t size,unsigned long redzone_end;if (gfpflags_allow_blocking (flags)) -quarantine_reduce (); +kasan_quarantine_reduce ();if (unlikely (ptr ==NULL ))return NULL ;
@@ -462,8 +461,8 @@ void * __must_check __kasan_kmalloc_large(const void *ptr, size_t size, KASAN_GRANULE_SIZE); redzone_end = (unsigned long )ptr +page_size (page); -unpoison_range (ptr, size); -poison_range ((void *)redzone_start, redzone_end - redzone_start, +kasan_unpoison (ptr, size); +kasan_poison ((void *)redzone_start, redzone_end - redzone_start,KASAN_PAGE_REDZONE );return (void *)ptr;
mm/kasan/generic.c +18 -18
@@ -158,7 +158,7 @@ static __always_inline bool memory_is_poisoned(unsigned long addr, size_t size)return memory_is_poisoned_n (addr, size); } -static __always_inline bool check_memory_region_inline(unsigned long addr, +static __always_inline bool check_region_inline(unsigned long addr,size_t size,bool write,unsigned long ret_ip) {
@@ -179,13 +179,13 @@ static __always_inline bool check_memory_region_inline(unsigned long addr,return !kasan_report (addr, size, write, ret_ip); } -bool check_memory_region (unsigned long addr,size_t size,bool write, -unsigned long ret_ip) +bool kasan_check_range (unsigned long addr,size_t size,bool write, +unsigned long ret_ip) { -return check_memory_region_inline (addr, size, write, ret_ip); +return check_region_inline (addr, size, write, ret_ip); } -bool check_invalid_free (void *addr) +bool kasan_check_invalid_free (void *addr) {s8 shadow_byte =READ_ONCE (*(s8 *)kasan_mem_to_shadow (addr));
@@ -194,22 +194,22 @@ bool check_invalid_free(void *addr)void kasan_cache_shrink (struct kmem_cache *cache) { -quarantine_remove_cache (cache); +kasan_quarantine_remove_cache (cache); }void kasan_cache_shutdown (struct kmem_cache *cache) {if (!__kmem_cache_empty (cache)) -quarantine_remove_cache (cache); +kasan_quarantine_remove_cache (cache); }static void register_global (struct kasan_global *global) {size_t aligned_size =round_up (global->size,KASAN_GRANULE_SIZE ); -unpoison_range (global->beg, global->size); +kasan_unpoison (global->beg, global->size); -poison_range (global->beg + aligned_size, +kasan_poison (global->beg + aligned_size, global->size_with_redzone - aligned_size,KASAN_GLOBAL_REDZONE ); }
@@ -231,7 +231,7 @@ EXPORT_SYMBOL(__asan_unregister_globals);#define DEFINE_ASAN_LOAD_STORE (size) \ void __asan_load##size(unsigned long addr) \ { \ - check_memory_region_inline(addr, size, false, _RET_IP_);\ + check_region_inline(addr, size, false, _RET_IP_); \ } \ EXPORT_SYMBOL(__asan_load##size); \ __alias(__asan_load##size) \
@@ -239,7 +239,7 @@ EXPORT_SYMBOL(__asan_unregister_globals); EXPORT_SYMBOL(__asan_load##size##_noabort); \ void __asan_store##size(unsigned long addr) \ { \ - check_memory_region_inline(addr, size, true, _RET_IP_); \ + check_region_inline(addr, size, true, _RET_IP_); \ } \ EXPORT_SYMBOL(__asan_store##size); \ __alias(__asan_store##size) \
@@ -254,7 +254,7 @@ DEFINE_ASAN_LOAD_STORE(16);void __asan_loadN (unsigned long addr,size_t size) { -check_memory_region (addr, size, false, _RET_IP_); +kasan_check_range (addr, size, false, _RET_IP_); }EXPORT_SYMBOL (__asan_loadN);
@@ -264,7 +264,7 @@ EXPORT_SYMBOL(__asan_loadN_noabort);void __asan_storeN (unsigned long addr,size_t size) { -check_memory_region (addr, size, true, _RET_IP_); +kasan_check_range (addr, size, true, _RET_IP_); }EXPORT_SYMBOL (__asan_storeN);
@@ -290,11 +290,11 @@ void __asan_alloca_poison(unsigned long addr, size_t size)WARN_ON (!IS_ALIGNED (addr,KASAN_ALLOCA_REDZONE_SIZE )); -unpoison_range ((const void *)(addr + rounded_down_size), - size - rounded_down_size); -poison_range (left_redzone,KASAN_ALLOCA_REDZONE_SIZE , +kasan_unpoison ((const void *)(addr + rounded_down_size), + size - rounded_down_size); +kasan_poison (left_redzone,KASAN_ALLOCA_REDZONE_SIZE ,KASAN_ALLOCA_LEFT ); -poison_range (right_redzone, padding_size +KASAN_ALLOCA_REDZONE_SIZE , +kasan_poison (right_redzone, padding_size +KASAN_ALLOCA_REDZONE_SIZE ,KASAN_ALLOCA_RIGHT ); }EXPORT_SYMBOL (__asan_alloca_poison);
@@ -305,7 +305,7 @@ void __asan_allocas_unpoison(const void *stack_top, const void *stack_bottom)if (unlikely (!stack_top || stack_top > stack_bottom))return ; -unpoison_range (stack_top, stack_bottom - stack_top); +kasan_unpoison (stack_top, stack_bottom - stack_top); }EXPORT_SYMBOL (__asan_allocas_unpoison);
mm/kasan/kasan.h +24 -24
107 lines not on this page · raw message
mm/kasan/quarantine.c +11 -11
@@ -168,7 +168,7 @@ static void qlist_free_all(struct qlist_head *q, struct kmem_cache *cache)qlist_init (q); } -bool quarantine_put (struct kmem_cache *cache,void *object) +bool kasan_quarantine_put (struct kmem_cache *cache,void *object) {unsigned long flags;struct qlist_head *q;
@@ -184,11 +184,11 @@ bool quarantine_put(struct kmem_cache *cache, void *object)/* * Note: irq must be disabled until after we move the batch to the -* global quarantine. Otherwise quarantine_remove_cache() can miss -* some objects belonging to the cache if they are in our local temp -* list. quarantine_remove_cache() executes on_each_cpu() at the -* beginning which ensures that it either sees the objects in per-cpu -* lists or in the global quarantine. +* global quarantine. Otherwise kasan_quarantine_remove_cache() can +* miss some objects belonging to the cache if they are in our local +* temp list. kasan_quarantine_remove_cache() executes on_each_cpu() +* at the beginning which ensures that it either sees the objects in +* per-cpu lists or in the global quarantine. */ local_irq_save (flags);
@@ -222,7 +222,7 @@ bool quarantine_put(struct kmem_cache *cache, void *object)return true; } -void quarantine_reduce (void ) +void kasan_quarantine_reduce (void ) {size_t total_size, new_quarantine_size, percpu_quarantines;unsigned long flags;
@@ -234,7 +234,7 @@ void quarantine_reduce(void)return ;/* -* srcu critical section ensures that quarantine_remove_cache() +* srcu critical section ensures that kasan_quarantine_remove_cache() * will not miss objects belonging to the cache while they are in our * local to_free list. srcu is chosen because (1) it gives us private * grace period domain that does not interfere with anything else,
@@ -309,15 +309,15 @@ static void per_cpu_remove_cache(void *arg) }/* Free all quarantined objects belonging to cache. */ -void quarantine_remove_cache (struct kmem_cache *cache) +void kasan_quarantine_remove_cache (struct kmem_cache *cache) {unsigned long flags, i;struct qlist_head to_free =QLIST_INIT ;/* * Must be careful to not miss any objects that are being moved from -* per-cpu list to the global quarantine in quarantine_put(), -* nor objects being freed in quarantine_reduce(). on_each_cpu() +* per-cpu list to the global quarantine in kasan_quarantine_put(), +* nor objects being freed in kasan_quarantine_reduce(). on_each_cpu() * achieves the first goal, while synchronize_srcu() achieves the * second. */
mm/kasan/report.c +7 -6
@@ -61,7 +61,7 @@ __setup("kasan_multi_shot", kasan_set_multi_shot);static void print_error_description (struct kasan_access_info *info) {pr_err ("BUG: KASAN: %s in %pS\n" , -get_bug_type (info), (void *)info->ip); +kasan_get_bug_type (info), (void *)info->ip);if (info->access_size) pr_err("%s of size %zu at addr %px by task %s/%d\n", info->is_write ? "Write" : "Read", info->access_size,
@@ -247,7 +247,7 @@ static void print_address_description(void *addr, u8 tag)dump_page (page,"kasan: bad access detected" ); } -print_address_stack_frame (addr); +kasan_print_address_stack_frame (addr); }static bool meta_row_is_guilty (const void *row,const void *addr)
@@ -293,7 +293,7 @@ static void print_memory_metadata(const void *addr)* function, because generic functions may try to * access kasan mapping for the passed address. */ -metadata_fetch_row (&metadata[0 ], row); +kasan_metadata_fetch_row (&metadata[0 ], row); print_hex_dump(KERN_ERR, buffer, DUMP_PREFIX_NONE, META_BYTES_PER_ROW, 1,
@@ -350,7 +350,7 @@ void kasan_report_invalid_free(void *object, unsigned long ip)start_report (&flags);pr_err ("BUG: KASAN: double-free or invalid-free in %pS\n" , (void *)ip); -print_tags (tag, object); +kasan_print_tags (tag, object);pr_err ("\n" );print_address_description (object, tag);pr_err ("\n" );
@@ -378,7 +378,8 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write, info.access_addr = tagged_addr;if (addr_has_metadata (untagged_addr)) - info.first_bad_addr =find_first_bad_addr (tagged_addr, size); + info.first_bad_addr = +kasan_find_first_bad_addr (tagged_addr, size);else info.first_bad_addr = untagged_addr; info.access_size = size;
@@ -389,7 +390,7 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,print_error_description (&info);if (addr_has_metadata (untagged_addr)) -print_tags (get_tag (tagged_addr), info.first_bad_addr); +kasan_print_tags (get_tag (tagged_addr), info.first_bad_addr);pr_err ("\n" );if (addr_has_metadata (untagged_addr)) {
mm/kasan/report_generic.c +4 -4
@@ -30,7 +30,7 @@#include "kasan.h" #include "../slab.h" -void *find_first_bad_addr (void *addr,size_t size) +void *kasan_find_first_bad_addr (void *addr,size_t size) {void *p = addr;
@@ -105,7 +105,7 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)return bug_type; } -const char *get_bug_type (struct kasan_access_info *info) +const char *kasan_get_bug_type (struct kasan_access_info *info) {/* * If access_size is a negative number, then it has reason to be
@@ -123,7 +123,7 @@ const char *get_bug_type(struct kasan_access_info *info)return get_wild_bug_type (info); } -void metadata_fetch_row (char *buffer,void *row) +void kasan_metadata_fetch_row (char *buffer,void *row) {memcpy (buffer,kasan_mem_to_shadow (row),META_BYTES_PER_ROW ); }
@@ -263,7 +263,7 @@ static bool __must_check get_address_stack_frame_info(const void *addr,return true; } -void print_address_stack_frame (const void *addr) +void kasan_print_address_stack_frame (const void *addr) {unsigned long offset;const char *frame_descr;
mm/kasan/report_hw_tags.c +4 -4
@@ -15,17 +15,17 @@#include "kasan.h" -const char *get_bug_type (struct kasan_access_info *info) +const char *kasan_get_bug_type (struct kasan_access_info *info) {return "invalid-access" ; } -void *find_first_bad_addr (void *addr,size_t size) +void *kasan_find_first_bad_addr (void *addr,size_t size) {return kasan_reset_tag (addr); } -void metadata_fetch_row (char *buffer,void *row) +void kasan_metadata_fetch_row (char *buffer,void *row) {int i;
@@ -33,7 +33,7 @@ void metadata_fetch_row(char *buffer, void *row) buffer[i] =hw_get_mem_tag (row + i *KASAN_GRANULE_SIZE ); } -void print_tags (u8 addr_tag,const void *addr) +void kasan_print_tags (u8 addr_tag,const void *addr) {u8 memory_tag =hw_get_mem_tag ((void *)addr);
mm/kasan/report_sw_tags.c +4 -4
@@ -29,7 +29,7 @@#include "kasan.h" #include "../slab.h" -const char *get_bug_type (struct kasan_access_info *info) +const char *kasan_get_bug_type (struct kasan_access_info *info) {#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY struct kasan_alloc_meta *alloc_meta;
@@ -72,7 +72,7 @@ const char *get_bug_type(struct kasan_access_info *info)return "invalid-access" ; } -void *find_first_bad_addr (void *addr,size_t size) +void *kasan_find_first_bad_addr (void *addr,size_t size) {u8 tag =get_tag (addr);void *p =kasan_reset_tag (addr);
@@ -83,12 +83,12 @@ void *find_first_bad_addr(void *addr, size_t size)return p; } -void metadata_fetch_row (char *buffer,void *row) +void kasan_metadata_fetch_row (char *buffer,void *row) {memcpy (buffer,kasan_mem_to_shadow (row),META_BYTES_PER_ROW ); } -void print_tags (u8 addr_tag,const void *addr) +void kasan_print_tags (u8 addr_tag,const void *addr) {u8 *shadow = (u8 *)kasan_mem_to_shadow (addr);
mm/kasan/shadow.c +13 -13
@@ -27,20 +27,20 @@bool __kasan_check_read (const volatile void *p,unsigned int size) { -return check_memory_region ((unsigned long )p, size, false, _RET_IP_); +return kasan_check_range ((unsigned long )p, size, false, _RET_IP_); }EXPORT_SYMBOL (__kasan_check_read);bool __kasan_check_write (const volatile void *p,unsigned int size) { -return check_memory_region ((unsigned long )p, size, true, _RET_IP_); +return kasan_check_range ((unsigned long )p, size, true, _RET_IP_); }EXPORT_SYMBOL (__kasan_check_write);#undef memsetvoid *memset (void *addr,int c,size_t len) { -if (!check_memory_region ((unsigned long )addr, len, true, _RET_IP_)) +if (!kasan_check_range ((unsigned long )addr, len, true, _RET_IP_))return NULL ;return __memset (addr, c, len);
@@ -50,8 +50,8 @@ void *memset(void *addr, int c, size_t len)#undef memmovevoid *memmove (void *dest,const void *src,size_t len) { -if (!check_memory_region ((unsigned long )src, len, false, _RET_IP_) || - !check_memory_region ((unsigned long )dest, len, true, _RET_IP_)) +if (!kasan_check_range ((unsigned long )src, len, false, _RET_IP_) || + !kasan_check_range ((unsigned long )dest, len, true, _RET_IP_))return NULL ;return __memmove (dest, src, len);
@@ -61,8 +61,8 @@ void *memmove(void *dest, const void *src, size_t len)#undef memcpyvoid *memcpy (void *dest,const void *src,size_t len) { -if (!check_memory_region ((unsigned long )src, len, false, _RET_IP_) || - !check_memory_region ((unsigned long )dest, len, true, _RET_IP_)) +if (!kasan_check_range ((unsigned long )src, len, false, _RET_IP_) || + !kasan_check_range ((unsigned long )dest, len, true, _RET_IP_))return NULL ;return __memcpy (dest, src, len);
@@ -72,7 +72,7 @@ void *memcpy(void *dest, const void *src, size_t len)* Poisons the shadow memory for 'size' bytes starting from 'addr'. * Memory addresses should be aligned to KASAN_GRANULE_SIZE. */ -void poison_range (const void *address,size_t size,u8 value) +void kasan_poison (const void *address,size_t size,u8 value) {void *shadow_start, *shadow_end;
@@ -90,7 +90,7 @@ void poison_range(const void *address, size_t size, u8 value)__memset (shadow_start, value, shadow_end - shadow_start); } -void unpoison_range (const void *address,size_t size) +void kasan_unpoison (const void *address,size_t size) {u8 tag =get_tag (address);
@@ -101,7 +101,7 @@ void unpoison_range(const void *address, size_t size)*/ address =kasan_reset_tag (address); -poison_range (address, size, tag); +kasan_poison (address, size, tag);if (size &KASAN_GRANULE_MASK ) {u8 *shadow = (u8 *)kasan_mem_to_shadow (address + size);
@@ -286,7 +286,7 @@ int kasan_populate_vmalloc(unsigned long addr, unsigned long size)* // vmalloc() allocates memory * // let a = area->addr * // we reach kasan_populate_vmalloc -* // and call unpoison_range: +* // and call kasan_unpoison: * STORE shadow(a), unpoison_val * ... * STORE shadow(a+99), unpoison_val x = LOAD p
@@ -321,7 +321,7 @@ void kasan_poison_vmalloc(const void *start, unsigned long size)return ; size =round_up (size,KASAN_GRANULE_SIZE ); -poison_range (start, size,KASAN_VMALLOC_INVALID ); +kasan_poison (start, size,KASAN_VMALLOC_INVALID ); }void kasan_unpoison_vmalloc (const void *start,unsigned long size)
@@ -329,7 +329,7 @@ void kasan_unpoison_vmalloc(const void *start, unsigned long size)if (!is_vmalloc_or_module_addr (start))return ; -unpoison_range (start, size); +kasan_unpoison (start, size); } static int kasan_depopulate_vmalloc_pte(pte_t *ptep, unsigned long addr,
mm/kasan/sw_tags.c +8 -8
@@ -57,7 +57,7 @@ void __init kasan_init_sw_tags(void)* sequence has in fact positive effect, since interrupts that randomly skew * PRNG at unpredictable points do only good. */ -u8 random_tag (void ) +u8 kasan_random_tag (void ) {u32 state =this_cpu_read (prng_state);
@@ -67,7 +67,7 @@ u8 random_tag(void)return (u8)(state % (KASAN_TAG_MAX +1 )); } -bool check_memory_region (unsigned long addr,size_t size,bool write, +bool kasan_check_range (unsigned long addr,size_t size,bool write,unsigned long ret_ip) {u8 tag;
@@ -118,7 +118,7 @@ bool check_memory_region(unsigned long addr, size_t size, bool write,return true; } -bool check_invalid_free (void *addr) +bool kasan_check_invalid_free (void *addr) {u8 tag =get_tag (addr);u8 shadow_byte =READ_ONCE (*(u8 *)kasan_mem_to_shadow (kasan_reset_tag (addr)));
@@ -130,12 +130,12 @@ bool check_invalid_free(void *addr)#define DEFINE_HWASAN_LOAD_STORE (size) \ void __hwasan_load##size##_noabort(unsigned long addr) \ { \ - check_memory_region(addr, size, false, _RET_IP_); \ + kasan_check_range(addr, size, false, _RET_IP_); \ } \ EXPORT_SYMBOL(__hwasan_load##size##_noabort); \ void __hwasan_store##size##_noabort(unsigned long addr) \ { \ - check_memory_region(addr, size, true, _RET_IP_); \ + kasan_check_range(addr, size, true, _RET_IP_); \ } \ EXPORT_SYMBOL(__hwasan_store##size##_noabort)
@@ -147,19 +147,19 @@ DEFINE_HWASAN_LOAD_STORE(16);void __hwasan_loadN_noabort (unsigned long addr,unsigned long size) { -check_memory_region (addr, size, false, _RET_IP_); +kasan_check_range (addr, size, false, _RET_IP_); }EXPORT_SYMBOL (__hwasan_loadN_noabort);void __hwasan_storeN_noabort (unsigned long addr,unsigned long size) { -check_memory_region (addr, size, true, _RET_IP_); +kasan_check_range (addr, size, true, _RET_IP_); }EXPORT_SYMBOL (__hwasan_storeN_noabort);void __hwasan_tag_memory (unsigned long addr,u8 tag,unsigned long size) { -poison_range ((void *)addr, size, tag); +kasan_poison ((void *)addr, size, tag); }EXPORT_SYMBOL (__hwasan_tag_memory);
tools/objtool/check.c +1 -1
@@ -576,7 +576,7 @@ static void add_ignores(struct objtool_file *file)static const char *uaccess_safe_builtin[] = {/* KASAN */ "kasan_report" , -"check_memory_region" , +"kasan_check_range" ,/* KASAN out-of-line */ "__asan_loadN_noabort" ,"__asan_load1_noabort" ,
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Re: [PATCH 01/11] kasan: prefix exported functions with kasan_
Quoted 11 lines · Andrey Konovalov
There's a number of internal KASAN functions that are used across multiple source code files and therefore aren't marked as static inline. To avoid littering the kernel function names list with generic functions, prefix all such KASAN functions with
kasan_.As a part of this change:
- Rename internal (un)poison_range() to kasan_(un)poison() (no
_range) to avoid name collision with a publickasan_unpoison_range().
Quoted 6 lines
- Rename
check_memory_region()tokasan_check_range(), as it seems to be a more fitting name.
Quoted 844 lines
--- mm/kasan/common.c | 47 +++++++++++++++++++------------------- mm/kasan/generic.c | 36 ++++++++++++++--------------- mm/kasan/kasan.h | 48 +++++++++++++++++++-------------------- mm/kasan/quarantine.c | 22 +++++++++--------- mm/kasan/report.c | 13 ++++++----- mm/kasan/report_generic.c | 8 +++---- mm/kasan/report_hw_tags.c | 8 +++---- mm/kasan/report_sw_tags.c | 8 +++---- mm/kasan/shadow.c | 26 ++++++++++----------- mm/kasan/sw_tags.c | 16 ++++++------- tools/objtool/check.c | 2 +- 11 files changed, 117 insertions(+), 117 deletions(-)mm/kasan/common.c +23
-24@@ -60,7 +60,7 @@ void kasan_disable_current(void)void __kasan_unpoison_range (const void *address,size_t size) { -unpoison_range (address, size); +kasan_unpoison (address, size); }#if CONFIG_KASAN_STACK @@ -69,7 +69,7 @@ void kasan_unpoison_task_stack(struct task_struct *task) {void *base =task_stack_page (task); -unpoison_range (base,THREAD_SIZE ); +kasan_unpoison (base,THREAD_SIZE ); }/* Unpoison the stack for the current task beyond a watermark sp value. */ @@ -82,7 +82,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)*/ void *base = (void *)((unsigned long )watermark & ~(THREAD_SIZE -1 )); - unpoison_range(base, watermark - base); + kasan_unpoison(base, watermark - base); }#endif /* CONFIG_KASAN_STACK */ @@ -105,18 +105,17 @@ void __kasan_alloc_pages(struct page *page, unsigned int order)if (unlikely (PageHighMem (page)))return ; - tag =random_tag (); + tag =kasan_random_tag ();for (i =0 ; i < (1 << order); i++)page_kasan_tag_set (page + i, tag); -unpoison_range (page_address (page),PAGE_SIZE << order); +kasan_unpoison (page_address (page),PAGE_SIZE << order); }void __kasan_free_pages (struct page *page,unsigned int order) {if (likely (!PageHighMem (page))) -poison_range (page_address (page), -PAGE_SIZE << order, -KASAN_FREE_PAGE ); +kasan_poison (page_address (page),PAGE_SIZE << order, +KASAN_FREE_PAGE ); }/* @@ -246,18 +245,18 @@ void __kasan_poison_slab(struct page *page)for (i =0 ; i <compound_nr (page); i++)page_kasan_tag_reset (page + i); -poison_range (page_address (page),page_size (page), +kasan_poison (page_address (page),page_size (page),KASAN_KMALLOC_REDZONE ); }void __kasan_unpoison_object_data (struct kmem_cache *cache,void *object) { -unpoison_range (object, cache->object_size); +kasan_unpoison (object, cache->object_size); }void __kasan_poison_object_data (struct kmem_cache *cache,void *object) { -poison_range (object, cache->object_size,KASAN_KMALLOC_REDZONE ); +kasan_poison (object, cache->object_size,KASAN_KMALLOC_REDZONE ); }/* @@ -294,7 +293,7 @@ static u8 assign_tag(struct kmem_cache *cache, const void *object,* set, assign a tag when the object is being allocated (init == false). */ if (!cache->ctor && !(cache->flags &SLAB_TYPESAFE_BY_RCU )) -return init ?KASAN_TAG_KERNEL :random_tag (); +return init ?KASAN_TAG_KERNEL :kasan_random_tag ();/* For caches that either have a constructor or SLAB_TYPESAFE_BY_RCU: */ #ifdef CONFIG_SLAB @@ -305,7 +304,7 @@ static u8 assign_tag(struct kmem_cache *cache, const void *object,* For SLUB assign a random tag during slab creation, otherwise reuse * the already assigned tag. */ -return init ?random_tag () :get_tag (object); +return init ?kasan_random_tag () :get_tag (object);#endif }@@ -346,12 +345,12 @@ static bool ____kasan_slab_free(struct kmem_cache *cache, void *object,if (unlikely (cache->flags &SLAB_TYPESAFE_BY_RCU ))return false; -if (check_invalid_free (tagged_object)) { +if (kasan_check_invalid_free (tagged_object)) {kasan_report_invalid_free (tagged_object, ip);return true; } -poison_range (object, cache->object_size,KASAN_KMALLOC_FREE ); +kasan_poison (object, cache->object_size,KASAN_KMALLOC_FREE );if (!kasan_stack_collection_enabled ())return false;@@ -361,7 +360,7 @@ static bool ____kasan_slab_free(struct kmem_cache *cache, void *object,kasan_set_free_info (cache, object, tag); -return quarantine_put (cache, object); +return kasan_quarantine_put (cache, object); }bool __kasan_slab_free (struct kmem_cache *cache,void *object,unsigned long ip)@@ -386,7 +385,7 @@ void __kasan_slab_free_mempool(void *ptr, unsigned long ip)kasan_report_invalid_free (ptr, ip);return ; } -poison_range (ptr,page_size (page),KASAN_FREE_PAGE ); +kasan_poison (ptr,page_size (page),KASAN_FREE_PAGE ); }else {____kasan_slab_free (page->slab_cache, ptr, ip, false); }@@ -409,7 +408,7 @@ static void *____kasan_kmalloc(struct kmem_cache *cache, const void *object,u8 tag;if (gfpflags_allow_blocking (flags)) -quarantine_reduce (); +kasan_quarantine_reduce ();if (unlikely (object ==NULL ))return NULL ;@@ -421,9 +420,9 @@ static void *____kasan_kmalloc(struct kmem_cache *cache, const void *object, tag =assign_tag (cache, object, false, keep_tag);/* Tag is ignored in set_tag without CONFIG_KASAN_SW/HW_TAGS */ -unpoison_range (set_tag (object, tag), size); -poison_range ((void *)redzone_start, redzone_end - redzone_start, -KASAN_KMALLOC_REDZONE ); +kasan_unpoison (set_tag (object, tag), size); +kasan_poison ((void *)redzone_start, redzone_end - redzone_start, +KASAN_KMALLOC_REDZONE );if (kasan_stack_collection_enabled ())set_alloc_info (cache, (void *)object, flags);@@ -452,7 +451,7 @@ void * __must_check __kasan_kmalloc_large(const void *ptr, size_t size,unsigned long redzone_end;if (gfpflags_allow_blocking (flags)) -quarantine_reduce (); +kasan_quarantine_reduce ();if (unlikely (ptr ==NULL ))return NULL ;@@ -462,8 +461,8 @@ void * __must_check __kasan_kmalloc_large(const void *ptr, size_t size, KASAN_GRANULE_SIZE); redzone_end = (unsigned long )ptr +page_size (page); -unpoison_range (ptr, size); -poison_range ((void *)redzone_start, redzone_end - redzone_start, +kasan_unpoison (ptr, size); +kasan_poison ((void *)redzone_start, redzone_end - redzone_start,KASAN_PAGE_REDZONE );return (void *)ptr;mm/kasan/generic.c +18
-18@@ -158,7 +158,7 @@ static __always_inline bool memory_is_poisoned(unsigned long addr, size_t size)return memory_is_poisoned_n (addr, size); } -static __always_inline bool check_memory_region_inline(unsigned long addr, +static __always_inline bool check_region_inline(unsigned long addr,size_t size,bool write,unsigned long ret_ip) {@@ -179,13 +179,13 @@ static __always_inline bool check_memory_region_inline(unsigned long addr,return !kasan_report (addr, size, write, ret_ip); } -bool check_memory_region (unsigned long addr,size_t size,bool write, -unsigned long ret_ip) +bool kasan_check_range (unsigned long addr,size_t size,bool write, +unsigned long ret_ip) { -return check_memory_region_inline (addr, size, write, ret_ip); +return check_region_inline (addr, size, write, ret_ip); } -bool check_invalid_free (void *addr) +bool kasan_check_invalid_free (void *addr) {s8 shadow_byte =READ_ONCE (*(s8 *)kasan_mem_to_shadow (addr));@@ -194,22 +194,22 @@ bool check_invalid_free(void *addr)void kasan_cache_shrink (struct kmem_cache *cache) { -quarantine_remove_cache (cache); +kasan_quarantine_remove_cache (cache); }void kasan_cache_shutdown (struct kmem_cache *cache) {if (!__kmem_cache_empty (cache)) -quarantine_remove_cache (cache); +kasan_quarantine_remove_cache (cache); }static void register_global (struct kasan_global *global) {size_t aligned_size =round_up (global->size,KASAN_GRANULE_SIZE ); -unpoison_range (global->beg, global->size); +kasan_unpoison (global->beg, global->size); -poison_range (global->beg + aligned_size, +kasan_poison (global->beg + aligned_size, global->size_with_redzone - aligned_size,KASAN_GLOBAL_REDZONE ); }@@ -231,7 +231,7 @@ EXPORT_SYMBOL(__asan_unregister_globals);#define DEFINE_ASAN_LOAD_STORE (size) \ void __asan_load##size(unsigned long addr) \ { \ - check_memory_region_inline(addr, size, false, _RET_IP_);\ + check_region_inline(addr, size, false, _RET_IP_); \ } \ EXPORT_SYMBOL(__asan_load##size); \ __alias(__asan_load##size) \@@ -239,7 +239,7 @@ EXPORT_SYMBOL(__asan_unregister_globals); EXPORT_SYMBOL(__asan_load##size##_noabort); \ void __asan_store##size(unsigned long addr) \ { \ - check_memory_region_inline(addr, size, true, _RET_IP_); \ + check_region_inline(addr, size, true, _RET_IP_); \ } \ EXPORT_SYMBOL(__asan_store##size); \ __alias(__asan_store##size) \@@ -254,7 +254,7 @@ DEFINE_ASAN_LOAD_STORE(16);void __asan_loadN (unsigned long addr,size_t size) { -check_memory_region (addr, size, false, _RET_IP_); +kasan_check_range (addr, size, false, _RET_IP_); }EXPORT_SYMBOL (__asan_loadN);@@ -264,7 +264,7 @@ EXPORT_SYMBOL(__asan_loadN_noabort);void __asan_storeN (unsigned long addr,size_t size) { -check_memory_region (addr, size, true, _RET_IP_); +kasan_check_range (addr, size, true, _RET_IP_); }EXPORT_SYMBOL (__asan_storeN);@@ -290,11 +290,11 @@ void __asan_alloca_poison(unsigned long addr, size_t size)WARN_ON (!IS_ALIGNED (addr,KASAN_ALLOCA_REDZONE_SIZE )); -unpoison_range ((const void *)(addr + rounded_down_size), - size - rounded_down_size); -poison_range (left_redzone,KASAN_ALLOCA_REDZONE_SIZE , +kasan_unpoison ((const void *)(addr + rounded_down_size), + size - rounded_down_size); +kasan_poison (left_redzone,KASAN_ALLOCA_REDZONE_SIZE ,KASAN_ALLOCA_LEFT ); -poison_range (right_redzone, padding_size +KASAN_ALLOCA_REDZONE_SIZE , +kasan_poison (right_redzone, padding_size +KASAN_ALLOCA_REDZONE_SIZE ,KASAN_ALLOCA_RIGHT ); }EXPORT_SYMBOL (__asan_alloca_poison);@@ -305,7 +305,7 @@ void __asan_allocas_unpoison(const void *stack_top, const void *stack_bottom)if (unlikely (!stack_top || stack_top > stack_bottom))return ; -unpoison_range (stack_top, stack_bottom - stack_top); +kasan_unpoison (stack_top, stack_bottom - stack_top); }EXPORT_SYMBOL (__asan_allocas_unpoison);mm/kasan/kasan.h +24
-24@@ -195,14 +195,14 @@ static inline bool addr_has_metadata(const void *addr) }/** -* check_memory_region - Check memory region, and report if invalid access. +* kasan_check_range - 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, +bool kasan_check_range (unsigned long addr,size_t size,bool write,unsigned long ret_ip);#else /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */ @@ -215,19 +215,19 @@ static inline bool addr_has_metadata(const void *addr)#endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */ #if defined(CONFIG_KASAN_SW_TAGS ) || defined(CONFIG_KASAN_HW_TAGS ) -void print_tags (u8 addr_tag,const void *addr); +void kasan_print_tags (u8 addr_tag,const void *addr);#else -static inline void print_tags (u8 addr_tag,const void *addr) { } +static inline void kasan_print_tags (u8 addr_tag,const void *addr) { }#endif -void *find_first_bad_addr (void *addr,size_t size); -const char *get_bug_type (struct kasan_access_info *info); -void metadata_fetch_row (char *buffer,void *row); +void *kasan_find_first_bad_addr (void *addr,size_t size); +const char *kasan_get_bug_type (struct kasan_access_info *info); +void kasan_metadata_fetch_row (char *buffer,void *row);#if defined(CONFIG_KASAN_GENERIC ) &&CONFIG_KASAN_STACK -void print_address_stack_frame (const void *addr); +void kasan_print_address_stack_frame (const void *addr);#else -static inline void print_address_stack_frame (const void *addr) { } +static inline void kasan_print_address_stack_frame (const void *addr) { }#endif bool kasan_report(unsigned long addr, size_t size,@@ -244,13 +244,13 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,#if defined(CONFIG_KASAN_GENERIC ) && \ (defined(CONFIG_SLAB ) || defined(CONFIG_SLUB )) -bool quarantine_put (struct kmem_cache *cache,void *object); -void quarantine_reduce (void ); -void quarantine_remove_cache (struct kmem_cache *cache); +bool kasan_quarantine_put (struct kmem_cache *cache,void *object); +void kasan_quarantine_reduce (void ); +void kasan_quarantine_remove_cache (struct kmem_cache *cache);#else -static inline bool quarantine_put (struct kmem_cache *cache,void *object) {return false; } -static inline void quarantine_reduce (void ) { } -static inline void quarantine_remove_cache (struct kmem_cache *cache) { } +static inline bool kasan_quarantine_put (struct kmem_cache *cache,void *object) {return false; } +static inline void kasan_quarantine_reduce (void ) { } +static inline void kasan_quarantine_remove_cache (struct kmem_cache *cache) { }#endif #ifndef arch_kasan_set_tag@@ -293,28 +293,28 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)#endif /* CONFIG_KASAN_HW_TAGS */ #ifdef CONFIG_KASAN_SW_TAGS -u8 random_tag (void ); +u8 kasan_random_tag (void );#elif defined(CONFIG_KASAN_HW_TAGS ) -static inline u8 random_tag (void ) {return hw_get_random_tag (); } +static inline u8 kasan_random_tag (void ) {return hw_get_random_tag (); }#else -static inline u8 random_tag (void ) {return 0 ; } +static inline u8 kasan_random_tag (void ) {return 0 ; }#endif #ifdef CONFIG_KASAN_HW_TAGS -static inline void poison_range (const void *address,size_t size,u8 value) +static inline void kasan_poison (const void *address,size_t size,u8 value) {hw_set_mem_tag_range (kasan_reset_tag (address),round_up (size,KASAN_GRANULE_SIZE ), value); } -static inline void unpoison_range (const void *address,size_t size) +static inline void kasan_unpoison (const void *address,size_t size) {hw_set_mem_tag_range (kasan_reset_tag (address),round_up (size,KASAN_GRANULE_SIZE ),get_tag (address)); } -static inline bool check_invalid_free (void *addr) +static inline bool kasan_check_invalid_free (void *addr) {u8 ptr_tag =get_tag (addr);u8 mem_tag =hw_get_mem_tag (addr);@@ -325,9 +325,9 @@ static inline bool check_invalid_free(void *addr)#else /* CONFIG_KASAN_HW_TAGS */ -void poison_range (const void *address,size_t size,u8 value); -void unpoison_range (const void *address,size_t size); -bool check_invalid_free (void *addr); +void kasan_poison (const void *address,size_t size,u8 value); +void kasan_unpoison (const void *address,size_t size); +bool kasan_check_invalid_free (void *addr);#endif /* CONFIG_KASAN_HW_TAGS */ mm/kasan/quarantine.c +11
-11@@ -168,7 +168,7 @@ static void qlist_free_all(struct qlist_head *q, struct kmem_cache *cache)qlist_init (q); } -bool quarantine_put (struct kmem_cache *cache,void *object) +bool kasan_quarantine_put (struct kmem_cache *cache,void *object) {unsigned long flags;struct qlist_head *q;@@ -184,11 +184,11 @@ bool quarantine_put(struct kmem_cache *cache, void *object)/* * Note: irq must be disabled until after we move the batch to the -* global quarantine. Otherwise quarantine_remove_cache() can miss -* some objects belonging to the cache if they are in our local temp -* list. quarantine_remove_cache() executes on_each_cpu() at the -* beginning which ensures that it either sees the objects in per-cpu -* lists or in the global quarantine. +* global quarantine. Otherwise kasan_quarantine_remove_cache() can +* miss some objects belonging to the cache if they are in our local +* temp list. kasan_quarantine_remove_cache() executes on_each_cpu() +* at the beginning which ensures that it either sees the objects in +* per-cpu lists or in the global quarantine. */ local_irq_save (flags);@@ -222,7 +222,7 @@ bool quarantine_put(struct kmem_cache *cache, void *object)return true; } -void quarantine_reduce (void ) +void kasan_quarantine_reduce (void ) {size_t total_size, new_quarantine_size, percpu_quarantines;unsigned long flags;@@ -234,7 +234,7 @@ void quarantine_reduce(void)return ;/* -* srcu critical section ensures that quarantine_remove_cache() +* srcu critical section ensures that kasan_quarantine_remove_cache() * will not miss objects belonging to the cache while they are in our * local to_free list. srcu is chosen because (1) it gives us private * grace period domain that does not interfere with anything else, @@ -309,15 +309,15 @@ static void per_cpu_remove_cache(void *arg) }/* Free all quarantined objects belonging to cache. */ -void quarantine_remove_cache (struct kmem_cache *cache) +void kasan_quarantine_remove_cache (struct kmem_cache *cache) {unsigned long flags, i;struct qlist_head to_free =QLIST_INIT ;/* * Must be careful to not miss any objects that are being moved from -* per-cpu list to the global quarantine in quarantine_put(), -* nor objects being freed in quarantine_reduce(). on_each_cpu() +* per-cpu list to the global quarantine in kasan_quarantine_put(), +* nor objects being freed in kasan_quarantine_reduce(). on_each_cpu() * achieves the first goal, while synchronize_srcu() achieves the * second. */ mm/kasan/report.c +7
-6@@ -61,7 +61,7 @@ __setup("kasan_multi_shot", kasan_set_multi_shot);static void print_error_description (struct kasan_access_info *info) {pr_err ("BUG: KASAN: %s in %pS\n" , -get_bug_type (info), (void *)info->ip); +kasan_get_bug_type (info), (void *)info->ip);if (info->access_size) pr_err("%s of size %zu at addr %px by task %s/%d\n", info->is_write ? "Write" : "Read", info->access_size,@@ -247,7 +247,7 @@ static void print_address_description(void *addr, u8 tag)dump_page (page,"kasan: bad access detected" ); } -print_address_stack_frame (addr); +kasan_print_address_stack_frame (addr); }static bool meta_row_is_guilty (const void *row,const void *addr)@@ -293,7 +293,7 @@ static void print_memory_metadata(const void *addr)* function, because generic functions may try to * access kasan mapping for the passed address. */ -metadata_fetch_row (&metadata[0 ], row); +kasan_metadata_fetch_row (&metadata[0 ], row); print_hex_dump(KERN_ERR, buffer, DUMP_PREFIX_NONE, META_BYTES_PER_ROW, 1,@@ -350,7 +350,7 @@ void kasan_report_invalid_free(void *object, unsigned long ip)start_report (&flags);pr_err ("BUG: KASAN: double-free or invalid-free in %pS\n" , (void *)ip); -print_tags (tag, object); +kasan_print_tags (tag, object);pr_err ("\n" );print_address_description (object, tag);pr_err ("\n" );@@ -378,7 +378,8 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write, info.access_addr = tagged_addr;if (addr_has_metadata (untagged_addr)) - info.first_bad_addr =find_first_bad_addr (tagged_addr, size); + info.first_bad_addr = +kasan_find_first_bad_addr (tagged_addr, size);else info.first_bad_addr = untagged_addr; info.access_size = size;@@ -389,7 +390,7 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,print_error_description (&info);if (addr_has_metadata (untagged_addr)) -print_tags (get_tag (tagged_addr), info.first_bad_addr); +kasan_print_tags (get_tag (tagged_addr), info.first_bad_addr);pr_err ("\n" );if (addr_has_metadata (untagged_addr)) {mm/kasan/report_generic.c +4
-4@@ -30,7 +30,7 @@#include "kasan.h" #include "../slab.h" -void *find_first_bad_addr (void *addr,size_t size) +void *kasan_find_first_bad_addr (void *addr,size_t size) {void *p = addr;@@ -105,7 +105,7 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)return bug_type; } -const char *get_bug_type (struct kasan_access_info *info) +const char *kasan_get_bug_type (struct kasan_access_info *info) {/* * If access_size is a negative number, then it has reason to be @@ -123,7 +123,7 @@ const char *get_bug_type(struct kasan_access_info *info)return get_wild_bug_type (info); } -void metadata_fetch_row (char *buffer,void *row) +void kasan_metadata_fetch_row (char *buffer,void *row) {memcpy (buffer,kasan_mem_to_shadow (row),META_BYTES_PER_ROW ); }@@ -263,7 +263,7 @@ static bool __must_check get_address_stack_frame_info(const void *addr,return true; } -void print_address_stack_frame (const void *addr) +void kasan_print_address_stack_frame (const void *addr) {unsigned long offset;const char *frame_descr;mm/kasan/report_hw_tags.c +4
-4@@ -15,17 +15,17 @@#include "kasan.h" -const char *get_bug_type (struct kasan_access_info *info) +const char *kasan_get_bug_type (struct kasan_access_info *info) {return "invalid-access" ; } -void *find_first_bad_addr (void *addr,size_t size) +void *kasan_find_first_bad_addr (void *addr,size_t size) {return kasan_reset_tag (addr); } -void metadata_fetch_row (char *buffer,void *row) +void kasan_metadata_fetch_row (char *buffer,void *row) {int i;@@ -33,7 +33,7 @@ void metadata_fetch_row(char *buffer, void *row) buffer[i] =hw_get_mem_tag (row + i *KASAN_GRANULE_SIZE ); } -void print_tags (u8 addr_tag,const void *addr) +void kasan_print_tags (u8 addr_tag,const void *addr) {u8 memory_tag =hw_get_mem_tag ((void *)addr);mm/kasan/report_sw_tags.c +4
-4@@ -29,7 +29,7 @@#include "kasan.h" #include "../slab.h" -const char *get_bug_type (struct kasan_access_info *info) +const char *kasan_get_bug_type (struct kasan_access_info *info) {#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY struct kasan_alloc_meta *alloc_meta;@@ -72,7 +72,7 @@ const char *get_bug_type(struct kasan_access_info *info)return "invalid-access" ; } -void *find_first_bad_addr (void *addr,size_t size) +void *kasan_find_first_bad_addr (void *addr,size_t size) {u8 tag =get_tag (addr);void *p =kasan_reset_tag (addr);@@ -83,12 +83,12 @@ void *find_first_bad_addr(void *addr, size_t size)return p; } -void metadata_fetch_row (char *buffer,void *row) +void kasan_metadata_fetch_row (char *buffer,void *row) {memcpy (buffer,kasan_mem_to_shadow (row),META_BYTES_PER_ROW ); } -void print_tags (u8 addr_tag,const void *addr) +void kasan_print_tags (u8 addr_tag,const void *addr) {u8 *shadow = (u8 *)kasan_mem_to_shadow (addr);mm/kasan/shadow.c +13
-13@@ -27,20 +27,20 @@bool __kasan_check_read (const volatile void *p,unsigned int size) { -return check_memory_region ((unsigned long )p, size, false, _RET_IP_); +return kasan_check_range ((unsigned long )p, size, false, _RET_IP_); }EXPORT_SYMBOL (__kasan_check_read);bool __kasan_check_write (const volatile void *p,unsigned int size) { -return check_memory_region ((unsigned long )p, size, true, _RET_IP_); +return kasan_check_range ((unsigned long )p, size, true, _RET_IP_); }EXPORT_SYMBOL (__kasan_check_write);#undef memsetvoid *memset (void *addr,int c,size_t len) { -if (!check_memory_region ((unsigned long )addr, len, true, _RET_IP_)) +if (!kasan_check_range ((unsigned long )addr, len, true, _RET_IP_))return NULL ;return __memset (addr, c, len);@@ -50,8 +50,8 @@ void *memset(void *addr, int c, size_t len)#undef memmovevoid *memmove (void *dest,const void *src,size_t len) { -if (!check_memory_region ((unsigned long )src, len, false, _RET_IP_) || - !check_memory_region ((unsigned long )dest, len, true, _RET_IP_)) +if (!kasan_check_range ((unsigned long )src, len, false, _RET_IP_) || + !kasan_check_range ((unsigned long )dest, len, true, _RET_IP_))return NULL ;return __memmove (dest, src, len);@@ -61,8 +61,8 @@ void *memmove(void *dest, const void *src, size_t len)#undef memcpyvoid *memcpy (void *dest,const void *src,size_t len) { -if (!check_memory_region ((unsigned long )src, len, false, _RET_IP_) || - !check_memory_region ((unsigned long )dest, len, true, _RET_IP_)) +if (!kasan_check_range ((unsigned long )src, len, false, _RET_IP_) || + !kasan_check_range ((unsigned long )dest, len, true, _RET_IP_))return NULL ;return __memcpy (dest, src, len);@@ -72,7 +72,7 @@ void *memcpy(void *dest, const void *src, size_t len)* Poisons the shadow memory for 'size' bytes starting from 'addr'. * Memory addresses should be aligned to KASAN_GRANULE_SIZE. */ -void poison_range (const void *address,size_t size,u8 value) +void kasan_poison (const void *address,size_t size,u8 value) {void *shadow_start, *shadow_end;@@ -90,7 +90,7 @@ void poison_range(const void *address, size_t size, u8 value)__memset (shadow_start, value, shadow_end - shadow_start); } -void unpoison_range (const void *address,size_t size) +void kasan_unpoison (const void *address,size_t size) {u8 tag =get_tag (address);@@ -101,7 +101,7 @@ void unpoison_range(const void *address, size_t size)*/ address =kasan_reset_tag (address); -poison_range (address, size, tag); +kasan_poison (address, size, tag);if (size &KASAN_GRANULE_MASK ) {u8 *shadow = (u8 *)kasan_mem_to_shadow (address + size);@@ -286,7 +286,7 @@ int kasan_populate_vmalloc(unsigned long addr, unsigned long size)* // vmalloc() allocates memory * // let a = area->addr * // we reach kasan_populate_vmalloc -* // and call unpoison_range: +* // and call kasan_unpoison: * STORE shadow(a), unpoison_val * ... * STORE shadow(a+99), unpoison_val x = LOAD p @@ -321,7 +321,7 @@ void kasan_poison_vmalloc(const void *start, unsigned long size)return ; size =round_up (size,KASAN_GRANULE_SIZE ); -poison_range (start, size,KASAN_VMALLOC_INVALID ); +kasan_poison (start, size,KASAN_VMALLOC_INVALID ); }void kasan_unpoison_vmalloc (const void *start,unsigned long size)@@ -329,7 +329,7 @@ void kasan_unpoison_vmalloc(const void *start, unsigned long size)if (!is_vmalloc_or_module_addr (start))return ; -unpoison_range (start, size); +kasan_unpoison (start, size); } static int kasan_depopulate_vmalloc_pte(pte_t *ptep, unsigned long addr,mm/kasan/sw_tags.c +8
-8@@ -57,7 +57,7 @@ void __init kasan_init_sw_tags(void)* sequence has in fact positive effect, since interrupts that randomly skew * PRNG at unpredictable points do only good. */ -u8 random_tag (void ) +u8 kasan_random_tag (void ) {u32 state =this_cpu_read (prng_state);@@ -67,7 +67,7 @@ u8 random_tag(void)return (u8)(state % (KASAN_TAG_MAX +1 )); } -bool check_memory_region (unsigned long addr,size_t size,bool write, +bool kasan_check_range (unsigned long addr,size_t size,bool write,unsigned long ret_ip) {u8 tag;@@ -118,7 +118,7 @@ bool check_memory_region(unsigned long addr, size_t size, bool write,return true; } -bool check_invalid_free (void *addr) +bool kasan_check_invalid_free (void *addr) {u8 tag =get_tag (addr);u8 shadow_byte =READ_ONCE (*(u8 *)kasan_mem_to_shadow (kasan_reset_tag (addr)));@@ -130,12 +130,12 @@ bool check_invalid_free(void *addr)#define DEFINE_HWASAN_LOAD_STORE (size) \ void __hwasan_load##size##_noabort(unsigned long addr) \ { \ - check_memory_region(addr, size, false, _RET_IP_); \ + kasan_check_range(addr, size, false, _RET_IP_); \ } \ EXPORT_SYMBOL(__hwasan_load##size##_noabort); \ void __hwasan_store##size##_noabort(unsigned long addr) \ { \ - check_memory_region(addr, size, true, _RET_IP_); \ + kasan_check_range(addr, size, true, _RET_IP_); \ } \ EXPORT_SYMBOL(__hwasan_store##size##_noabort)@@ -147,19 +147,19 @@ DEFINE_HWASAN_LOAD_STORE(16);void __hwasan_loadN_noabort (unsigned long addr,unsigned long size) { -check_memory_region (addr, size, false, _RET_IP_); +kasan_check_range (addr, size, false, _RET_IP_); }EXPORT_SYMBOL (__hwasan_loadN_noabort);void __hwasan_storeN_noabort (unsigned long addr,unsigned long size) { -check_memory_region (addr, size, true, _RET_IP_); +kasan_check_range (addr, size, true, _RET_IP_); }EXPORT_SYMBOL (__hwasan_storeN_noabort);void __hwasan_tag_memory (unsigned long addr,u8 tag,unsigned long size) { -poison_range ((void *)addr, size, tag); +kasan_poison ((void *)addr, size, tag); }EXPORT_SYMBOL (__hwasan_tag_memory);tools/objtool/check.c +1
-2@@ -576,7 +576,7 @@ static void add_ignores(struct objtool_file *file)static const char *uaccess_safe_builtin[] = {/* KASAN */ "kasan_report" , -"check_memory_region" , +"kasan_check_range" ,/* KASAN out-of-line */ "__asan_loadN_noabort" ,"__asan_load1_noabort" , --2.29.2.729.g45daf8777d-goog
Signature · 10 lines
Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
Re: [PATCH 01/11] kasan: prefix exported functions with kasan_
In reply to Andrey KonovalovRe subject: none of these seem to be exported (but they are global).
Quoted 10 lines · Andrey Konovalov
There's a number of internal KASAN functions that are used across multiple source code files and therefore aren't marked as static inline. To avoid littering the kernel function names list with generic functions, prefix all such KASAN functions with
kasan_.As a part of this change:
- Rename internal (un)poison_range() to kasan_(un)poison() (no
_range) to avoid name collision with a publickasan_unpoison_range().
Quoted 6 lines
- Rename
check_memory_region()tokasan_check_range(), as it seems to be a more fitting name.
Thank you!
Quoted 844 lines
--- mm/kasan/common.c | 47 +++++++++++++++++++------------------- mm/kasan/generic.c | 36 ++++++++++++++--------------- mm/kasan/kasan.h | 48 +++++++++++++++++++-------------------- mm/kasan/quarantine.c | 22 +++++++++--------- mm/kasan/report.c | 13 ++++++----- mm/kasan/report_generic.c | 8 +++---- mm/kasan/report_hw_tags.c | 8 +++---- mm/kasan/report_sw_tags.c | 8 +++---- mm/kasan/shadow.c | 26 ++++++++++----------- mm/kasan/sw_tags.c | 16 ++++++------- tools/objtool/check.c | 2 +- 11 files changed, 117 insertions(+), 117 deletions(-)mm/kasan/common.c +23
-24@@ -60,7 +60,7 @@ void kasan_disable_current(void)void __kasan_unpoison_range (const void *address,size_t size) { -unpoison_range (address, size); +kasan_unpoison (address, size); }#if CONFIG_KASAN_STACK @@ -69,7 +69,7 @@ void kasan_unpoison_task_stack(struct task_struct *task) {void *base =task_stack_page (task); -unpoison_range (base,THREAD_SIZE ); +kasan_unpoison (base,THREAD_SIZE ); }/* Unpoison the stack for the current task beyond a watermark sp value. */ @@ -82,7 +82,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)*/ void *base = (void *)((unsigned long )watermark & ~(THREAD_SIZE -1 )); - unpoison_range(base, watermark - base); + kasan_unpoison(base, watermark - base); }#endif /* CONFIG_KASAN_STACK */ @@ -105,18 +105,17 @@ void __kasan_alloc_pages(struct page *page, unsigned int order)if (unlikely (PageHighMem (page)))return ; - tag =random_tag (); + tag =kasan_random_tag ();for (i =0 ; i < (1 << order); i++)page_kasan_tag_set (page + i, tag); -unpoison_range (page_address (page),PAGE_SIZE << order); +kasan_unpoison (page_address (page),PAGE_SIZE << order); }void __kasan_free_pages (struct page *page,unsigned int order) {if (likely (!PageHighMem (page))) -poison_range (page_address (page), -PAGE_SIZE << order, -KASAN_FREE_PAGE ); +kasan_poison (page_address (page),PAGE_SIZE << order, +KASAN_FREE_PAGE ); }/* @@ -246,18 +245,18 @@ void __kasan_poison_slab(struct page *page)for (i =0 ; i <compound_nr (page); i++)page_kasan_tag_reset (page + i); -poison_range (page_address (page),page_size (page), +kasan_poison (page_address (page),page_size (page),KASAN_KMALLOC_REDZONE ); }void __kasan_unpoison_object_data (struct kmem_cache *cache,void *object) { -unpoison_range (object, cache->object_size); +kasan_unpoison (object, cache->object_size); }void __kasan_poison_object_data (struct kmem_cache *cache,void *object) { -poison_range (object, cache->object_size,KASAN_KMALLOC_REDZONE ); +kasan_poison (object, cache->object_size,KASAN_KMALLOC_REDZONE ); }/* @@ -294,7 +293,7 @@ static u8 assign_tag(struct kmem_cache *cache, const void *object,* set, assign a tag when the object is being allocated (init == false). */ if (!cache->ctor && !(cache->flags &SLAB_TYPESAFE_BY_RCU )) -return init ?KASAN_TAG_KERNEL :random_tag (); +return init ?KASAN_TAG_KERNEL :kasan_random_tag ();/* For caches that either have a constructor or SLAB_TYPESAFE_BY_RCU: */ #ifdef CONFIG_SLAB @@ -305,7 +304,7 @@ static u8 assign_tag(struct kmem_cache *cache, const void *object,* For SLUB assign a random tag during slab creation, otherwise reuse * the already assigned tag. */ -return init ?random_tag () :get_tag (object); +return init ?kasan_random_tag () :get_tag (object);#endif }@@ -346,12 +345,12 @@ static bool ____kasan_slab_free(struct kmem_cache *cache, void *object,if (unlikely (cache->flags &SLAB_TYPESAFE_BY_RCU ))return false; -if (check_invalid_free (tagged_object)) { +if (kasan_check_invalid_free (tagged_object)) {kasan_report_invalid_free (tagged_object, ip);return true; } -poison_range (object, cache->object_size,KASAN_KMALLOC_FREE ); +kasan_poison (object, cache->object_size,KASAN_KMALLOC_FREE );if (!kasan_stack_collection_enabled ())return false;@@ -361,7 +360,7 @@ static bool ____kasan_slab_free(struct kmem_cache *cache, void *object,kasan_set_free_info (cache, object, tag); -return quarantine_put (cache, object); +return kasan_quarantine_put (cache, object); }bool __kasan_slab_free (struct kmem_cache *cache,void *object,unsigned long ip)@@ -386,7 +385,7 @@ void __kasan_slab_free_mempool(void *ptr, unsigned long ip)kasan_report_invalid_free (ptr, ip);return ; } -poison_range (ptr,page_size (page),KASAN_FREE_PAGE ); +kasan_poison (ptr,page_size (page),KASAN_FREE_PAGE ); }else {____kasan_slab_free (page->slab_cache, ptr, ip, false); }@@ -409,7 +408,7 @@ static void *____kasan_kmalloc(struct kmem_cache *cache, const void *object,u8 tag;if (gfpflags_allow_blocking (flags)) -quarantine_reduce (); +kasan_quarantine_reduce ();if (unlikely (object ==NULL ))return NULL ;@@ -421,9 +420,9 @@ static void *____kasan_kmalloc(struct kmem_cache *cache, const void *object, tag =assign_tag (cache, object, false, keep_tag);/* Tag is ignored in set_tag without CONFIG_KASAN_SW/HW_TAGS */ -unpoison_range (set_tag (object, tag), size); -poison_range ((void *)redzone_start, redzone_end - redzone_start, -KASAN_KMALLOC_REDZONE ); +kasan_unpoison (set_tag (object, tag), size); +kasan_poison ((void *)redzone_start, redzone_end - redzone_start, +KASAN_KMALLOC_REDZONE );if (kasan_stack_collection_enabled ())set_alloc_info (cache, (void *)object, flags);@@ -452,7 +451,7 @@ void * __must_check __kasan_kmalloc_large(const void *ptr, size_t size,unsigned long redzone_end;if (gfpflags_allow_blocking (flags)) -quarantine_reduce (); +kasan_quarantine_reduce ();if (unlikely (ptr ==NULL ))return NULL ;@@ -462,8 +461,8 @@ void * __must_check __kasan_kmalloc_large(const void *ptr, size_t size, KASAN_GRANULE_SIZE); redzone_end = (unsigned long )ptr +page_size (page); -unpoison_range (ptr, size); -poison_range ((void *)redzone_start, redzone_end - redzone_start, +kasan_unpoison (ptr, size); +kasan_poison ((void *)redzone_start, redzone_end - redzone_start,KASAN_PAGE_REDZONE );return (void *)ptr;mm/kasan/generic.c +18
-18@@ -158,7 +158,7 @@ static __always_inline bool memory_is_poisoned(unsigned long addr, size_t size)return memory_is_poisoned_n (addr, size); } -static __always_inline bool check_memory_region_inline(unsigned long addr, +static __always_inline bool check_region_inline(unsigned long addr,size_t size,bool write,unsigned long ret_ip) {@@ -179,13 +179,13 @@ static __always_inline bool check_memory_region_inline(unsigned long addr,return !kasan_report (addr, size, write, ret_ip); } -bool check_memory_region (unsigned long addr,size_t size,bool write, -unsigned long ret_ip) +bool kasan_check_range (unsigned long addr,size_t size,bool write, +unsigned long ret_ip) { -return check_memory_region_inline (addr, size, write, ret_ip); +return check_region_inline (addr, size, write, ret_ip); } -bool check_invalid_free (void *addr) +bool kasan_check_invalid_free (void *addr) {s8 shadow_byte =READ_ONCE (*(s8 *)kasan_mem_to_shadow (addr));@@ -194,22 +194,22 @@ bool check_invalid_free(void *addr)void kasan_cache_shrink (struct kmem_cache *cache) { -quarantine_remove_cache (cache); +kasan_quarantine_remove_cache (cache); }void kasan_cache_shutdown (struct kmem_cache *cache) {if (!__kmem_cache_empty (cache)) -quarantine_remove_cache (cache); +kasan_quarantine_remove_cache (cache); }static void register_global (struct kasan_global *global) {size_t aligned_size =round_up (global->size,KASAN_GRANULE_SIZE ); -unpoison_range (global->beg, global->size); +kasan_unpoison (global->beg, global->size); -poison_range (global->beg + aligned_size, +kasan_poison (global->beg + aligned_size, global->size_with_redzone - aligned_size,KASAN_GLOBAL_REDZONE ); }@@ -231,7 +231,7 @@ EXPORT_SYMBOL(__asan_unregister_globals);#define DEFINE_ASAN_LOAD_STORE (size) \ void __asan_load##size(unsigned long addr) \ { \ - check_memory_region_inline(addr, size, false, _RET_IP_);\ + check_region_inline(addr, size, false, _RET_IP_); \ } \ EXPORT_SYMBOL(__asan_load##size); \ __alias(__asan_load##size) \@@ -239,7 +239,7 @@ EXPORT_SYMBOL(__asan_unregister_globals); EXPORT_SYMBOL(__asan_load##size##_noabort); \ void __asan_store##size(unsigned long addr) \ { \ - check_memory_region_inline(addr, size, true, _RET_IP_); \ + check_region_inline(addr, size, true, _RET_IP_); \ } \ EXPORT_SYMBOL(__asan_store##size); \ __alias(__asan_store##size) \@@ -254,7 +254,7 @@ DEFINE_ASAN_LOAD_STORE(16);void __asan_loadN (unsigned long addr,size_t size) { -check_memory_region (addr, size, false, _RET_IP_); +kasan_check_range (addr, size, false, _RET_IP_); }EXPORT_SYMBOL (__asan_loadN);@@ -264,7 +264,7 @@ EXPORT_SYMBOL(__asan_loadN_noabort);void __asan_storeN (unsigned long addr,size_t size) { -check_memory_region (addr, size, true, _RET_IP_); +kasan_check_range (addr, size, true, _RET_IP_); }EXPORT_SYMBOL (__asan_storeN);@@ -290,11 +290,11 @@ void __asan_alloca_poison(unsigned long addr, size_t size)WARN_ON (!IS_ALIGNED (addr,KASAN_ALLOCA_REDZONE_SIZE )); -unpoison_range ((const void *)(addr + rounded_down_size), - size - rounded_down_size); -poison_range (left_redzone,KASAN_ALLOCA_REDZONE_SIZE , +kasan_unpoison ((const void *)(addr + rounded_down_size), + size - rounded_down_size); +kasan_poison (left_redzone,KASAN_ALLOCA_REDZONE_SIZE ,KASAN_ALLOCA_LEFT ); -poison_range (right_redzone, padding_size +KASAN_ALLOCA_REDZONE_SIZE , +kasan_poison (right_redzone, padding_size +KASAN_ALLOCA_REDZONE_SIZE ,KASAN_ALLOCA_RIGHT ); }EXPORT_SYMBOL (__asan_alloca_poison);@@ -305,7 +305,7 @@ void __asan_allocas_unpoison(const void *stack_top, const void *stack_bottom)if (unlikely (!stack_top || stack_top > stack_bottom))return ; -unpoison_range (stack_top, stack_bottom - stack_top); +kasan_unpoison (stack_top, stack_bottom - stack_top); }EXPORT_SYMBOL (__asan_allocas_unpoison);mm/kasan/kasan.h +24
-24@@ -195,14 +195,14 @@ static inline bool addr_has_metadata(const void *addr) }/** -* check_memory_region - Check memory region, and report if invalid access. +* kasan_check_range - 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, +bool kasan_check_range (unsigned long addr,size_t size,bool write,unsigned long ret_ip);#else /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */ @@ -215,19 +215,19 @@ static inline bool addr_has_metadata(const void *addr)#endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */ #if defined(CONFIG_KASAN_SW_TAGS ) || defined(CONFIG_KASAN_HW_TAGS ) -void print_tags (u8 addr_tag,const void *addr); +void kasan_print_tags (u8 addr_tag,const void *addr);#else -static inline void print_tags (u8 addr_tag,const void *addr) { } +static inline void kasan_print_tags (u8 addr_tag,const void *addr) { }#endif -void *find_first_bad_addr (void *addr,size_t size); -const char *get_bug_type (struct kasan_access_info *info); -void metadata_fetch_row (char *buffer,void *row); +void *kasan_find_first_bad_addr (void *addr,size_t size); +const char *kasan_get_bug_type (struct kasan_access_info *info); +void kasan_metadata_fetch_row (char *buffer,void *row);#if defined(CONFIG_KASAN_GENERIC ) &&CONFIG_KASAN_STACK -void print_address_stack_frame (const void *addr); +void kasan_print_address_stack_frame (const void *addr);#else -static inline void print_address_stack_frame (const void *addr) { } +static inline void kasan_print_address_stack_frame (const void *addr) { }#endif bool kasan_report(unsigned long addr, size_t size,@@ -244,13 +244,13 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,#if defined(CONFIG_KASAN_GENERIC ) && \ (defined(CONFIG_SLAB ) || defined(CONFIG_SLUB )) -bool quarantine_put (struct kmem_cache *cache,void *object); -void quarantine_reduce (void ); -void quarantine_remove_cache (struct kmem_cache *cache); +bool kasan_quarantine_put (struct kmem_cache *cache,void *object); +void kasan_quarantine_reduce (void ); +void kasan_quarantine_remove_cache (struct kmem_cache *cache);#else -static inline bool quarantine_put (struct kmem_cache *cache,void *object) {return false; } -static inline void quarantine_reduce (void ) { } -static inline void quarantine_remove_cache (struct kmem_cache *cache) { } +static inline bool kasan_quarantine_put (struct kmem_cache *cache,void *object) {return false; } +static inline void kasan_quarantine_reduce (void ) { } +static inline void kasan_quarantine_remove_cache (struct kmem_cache *cache) { }#endif #ifndef arch_kasan_set_tag@@ -293,28 +293,28 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)#endif /* CONFIG_KASAN_HW_TAGS */ #ifdef CONFIG_KASAN_SW_TAGS -u8 random_tag (void ); +u8 kasan_random_tag (void );#elif defined(CONFIG_KASAN_HW_TAGS ) -static inline u8 random_tag (void ) {return hw_get_random_tag (); } +static inline u8 kasan_random_tag (void ) {return hw_get_random_tag (); }#else -static inline u8 random_tag (void ) {return 0 ; } +static inline u8 kasan_random_tag (void ) {return 0 ; }#endif #ifdef CONFIG_KASAN_HW_TAGS -static inline void poison_range (const void *address,size_t size,u8 value) +static inline void kasan_poison (const void *address,size_t size,u8 value) {hw_set_mem_tag_range (kasan_reset_tag (address),round_up (size,KASAN_GRANULE_SIZE ), value); } -static inline void unpoison_range (const void *address,size_t size) +static inline void kasan_unpoison (const void *address,size_t size) {hw_set_mem_tag_range (kasan_reset_tag (address),round_up (size,KASAN_GRANULE_SIZE ),get_tag (address)); } -static inline bool check_invalid_free (void *addr) +static inline bool kasan_check_invalid_free (void *addr) {u8 ptr_tag =get_tag (addr);u8 mem_tag =hw_get_mem_tag (addr);@@ -325,9 +325,9 @@ static inline bool check_invalid_free(void *addr)#else /* CONFIG_KASAN_HW_TAGS */ -void poison_range (const void *address,size_t size,u8 value); -void unpoison_range (const void *address,size_t size); -bool check_invalid_free (void *addr); +void kasan_poison (const void *address,size_t size,u8 value); +void kasan_unpoison (const void *address,size_t size); +bool kasan_check_invalid_free (void *addr);#endif /* CONFIG_KASAN_HW_TAGS */ mm/kasan/quarantine.c +11
-11@@ -168,7 +168,7 @@ static void qlist_free_all(struct qlist_head *q, struct kmem_cache *cache)qlist_init (q); } -bool quarantine_put (struct kmem_cache *cache,void *object) +bool kasan_quarantine_put (struct kmem_cache *cache,void *object) {unsigned long flags;struct qlist_head *q;@@ -184,11 +184,11 @@ bool quarantine_put(struct kmem_cache *cache, void *object)/* * Note: irq must be disabled until after we move the batch to the -* global quarantine. Otherwise quarantine_remove_cache() can miss -* some objects belonging to the cache if they are in our local temp -* list. quarantine_remove_cache() executes on_each_cpu() at the -* beginning which ensures that it either sees the objects in per-cpu -* lists or in the global quarantine. +* global quarantine. Otherwise kasan_quarantine_remove_cache() can +* miss some objects belonging to the cache if they are in our local +* temp list. kasan_quarantine_remove_cache() executes on_each_cpu() +* at the beginning which ensures that it either sees the objects in +* per-cpu lists or in the global quarantine. */ local_irq_save (flags);@@ -222,7 +222,7 @@ bool quarantine_put(struct kmem_cache *cache, void *object)return true; } -void quarantine_reduce (void ) +void kasan_quarantine_reduce (void ) {size_t total_size, new_quarantine_size, percpu_quarantines;unsigned long flags;@@ -234,7 +234,7 @@ void quarantine_reduce(void)return ;/* -* srcu critical section ensures that quarantine_remove_cache() +* srcu critical section ensures that kasan_quarantine_remove_cache() * will not miss objects belonging to the cache while they are in our * local to_free list. srcu is chosen because (1) it gives us private * grace period domain that does not interfere with anything else, @@ -309,15 +309,15 @@ static void per_cpu_remove_cache(void *arg) }/* Free all quarantined objects belonging to cache. */ -void quarantine_remove_cache (struct kmem_cache *cache) +void kasan_quarantine_remove_cache (struct kmem_cache *cache) {unsigned long flags, i;struct qlist_head to_free =QLIST_INIT ;/* * Must be careful to not miss any objects that are being moved from -* per-cpu list to the global quarantine in quarantine_put(), -* nor objects being freed in quarantine_reduce(). on_each_cpu() +* per-cpu list to the global quarantine in kasan_quarantine_put(), +* nor objects being freed in kasan_quarantine_reduce(). on_each_cpu() * achieves the first goal, while synchronize_srcu() achieves the * second. */ mm/kasan/report.c +7
-6@@ -61,7 +61,7 @@ __setup("kasan_multi_shot", kasan_set_multi_shot);static void print_error_description (struct kasan_access_info *info) {pr_err ("BUG: KASAN: %s in %pS\n" , -get_bug_type (info), (void *)info->ip); +kasan_get_bug_type (info), (void *)info->ip);if (info->access_size) pr_err("%s of size %zu at addr %px by task %s/%d\n", info->is_write ? "Write" : "Read", info->access_size,@@ -247,7 +247,7 @@ static void print_address_description(void *addr, u8 tag)dump_page (page,"kasan: bad access detected" ); } -print_address_stack_frame (addr); +kasan_print_address_stack_frame (addr); }static bool meta_row_is_guilty (const void *row,const void *addr)@@ -293,7 +293,7 @@ static void print_memory_metadata(const void *addr)* function, because generic functions may try to * access kasan mapping for the passed address. */ -metadata_fetch_row (&metadata[0 ], row); +kasan_metadata_fetch_row (&metadata[0 ], row); print_hex_dump(KERN_ERR, buffer, DUMP_PREFIX_NONE, META_BYTES_PER_ROW, 1,@@ -350,7 +350,7 @@ void kasan_report_invalid_free(void *object, unsigned long ip)start_report (&flags);pr_err ("BUG: KASAN: double-free or invalid-free in %pS\n" , (void *)ip); -print_tags (tag, object); +kasan_print_tags (tag, object);pr_err ("\n" );print_address_description (object, tag);pr_err ("\n" );@@ -378,7 +378,8 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write, info.access_addr = tagged_addr;if (addr_has_metadata (untagged_addr)) - info.first_bad_addr =find_first_bad_addr (tagged_addr, size); + info.first_bad_addr = +kasan_find_first_bad_addr (tagged_addr, size);else info.first_bad_addr = untagged_addr; info.access_size = size;@@ -389,7 +390,7 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,print_error_description (&info);if (addr_has_metadata (untagged_addr)) -print_tags (get_tag (tagged_addr), info.first_bad_addr); +kasan_print_tags (get_tag (tagged_addr), info.first_bad_addr);pr_err ("\n" );if (addr_has_metadata (untagged_addr)) {mm/kasan/report_generic.c +4
-4@@ -30,7 +30,7 @@#include "kasan.h" #include "../slab.h" -void *find_first_bad_addr (void *addr,size_t size) +void *kasan_find_first_bad_addr (void *addr,size_t size) {void *p = addr;@@ -105,7 +105,7 @@ static const char *get_wild_bug_type(struct kasan_access_info *info)return bug_type; } -const char *get_bug_type (struct kasan_access_info *info) +const char *kasan_get_bug_type (struct kasan_access_info *info) {/* * If access_size is a negative number, then it has reason to be @@ -123,7 +123,7 @@ const char *get_bug_type(struct kasan_access_info *info)return get_wild_bug_type (info); } -void metadata_fetch_row (char *buffer,void *row) +void kasan_metadata_fetch_row (char *buffer,void *row) {memcpy (buffer,kasan_mem_to_shadow (row),META_BYTES_PER_ROW ); }@@ -263,7 +263,7 @@ static bool __must_check get_address_stack_frame_info(const void *addr,return true; } -void print_address_stack_frame (const void *addr) +void kasan_print_address_stack_frame (const void *addr) {unsigned long offset;const char *frame_descr;mm/kasan/report_hw_tags.c +4
-4@@ -15,17 +15,17 @@#include "kasan.h" -const char *get_bug_type (struct kasan_access_info *info) +const char *kasan_get_bug_type (struct kasan_access_info *info) {return "invalid-access" ; } -void *find_first_bad_addr (void *addr,size_t size) +void *kasan_find_first_bad_addr (void *addr,size_t size) {return kasan_reset_tag (addr); } -void metadata_fetch_row (char *buffer,void *row) +void kasan_metadata_fetch_row (char *buffer,void *row) {int i;@@ -33,7 +33,7 @@ void metadata_fetch_row(char *buffer, void *row) buffer[i] =hw_get_mem_tag (row + i *KASAN_GRANULE_SIZE ); } -void print_tags (u8 addr_tag,const void *addr) +void kasan_print_tags (u8 addr_tag,const void *addr) {u8 memory_tag =hw_get_mem_tag ((void *)addr);mm/kasan/report_sw_tags.c +4
-4@@ -29,7 +29,7 @@#include "kasan.h" #include "../slab.h" -const char *get_bug_type (struct kasan_access_info *info) +const char *kasan_get_bug_type (struct kasan_access_info *info) {#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY struct kasan_alloc_meta *alloc_meta;@@ -72,7 +72,7 @@ const char *get_bug_type(struct kasan_access_info *info)return "invalid-access" ; } -void *find_first_bad_addr (void *addr,size_t size) +void *kasan_find_first_bad_addr (void *addr,size_t size) {u8 tag =get_tag (addr);void *p =kasan_reset_tag (addr);@@ -83,12 +83,12 @@ void *find_first_bad_addr(void *addr, size_t size)return p; } -void metadata_fetch_row (char *buffer,void *row) +void kasan_metadata_fetch_row (char *buffer,void *row) {memcpy (buffer,kasan_mem_to_shadow (row),META_BYTES_PER_ROW ); } -void print_tags (u8 addr_tag,const void *addr) +void kasan_print_tags (u8 addr_tag,const void *addr) {u8 *shadow = (u8 *)kasan_mem_to_shadow (addr);mm/kasan/shadow.c +13
-13@@ -27,20 +27,20 @@bool __kasan_check_read (const volatile void *p,unsigned int size) { -return check_memory_region ((unsigned long )p, size, false, _RET_IP_); +return kasan_check_range ((unsigned long )p, size, false, _RET_IP_); }EXPORT_SYMBOL (__kasan_check_read);bool __kasan_check_write (const volatile void *p,unsigned int size) { -return check_memory_region ((unsigned long )p, size, true, _RET_IP_); +return kasan_check_range ((unsigned long )p, size, true, _RET_IP_); }EXPORT_SYMBOL (__kasan_check_write);#undef memsetvoid *memset (void *addr,int c,size_t len) { -if (!check_memory_region ((unsigned long )addr, len, true, _RET_IP_)) +if (!kasan_check_range ((unsigned long )addr, len, true, _RET_IP_))return NULL ;return __memset (addr, c, len);@@ -50,8 +50,8 @@ void *memset(void *addr, int c, size_t len)#undef memmovevoid *memmove (void *dest,const void *src,size_t len) { -if (!check_memory_region ((unsigned long )src, len, false, _RET_IP_) || - !check_memory_region ((unsigned long )dest, len, true, _RET_IP_)) +if (!kasan_check_range ((unsigned long )src, len, false, _RET_IP_) || + !kasan_check_range ((unsigned long )dest, len, true, _RET_IP_))return NULL ;return __memmove (dest, src, len);@@ -61,8 +61,8 @@ void *memmove(void *dest, const void *src, size_t len)#undef memcpyvoid *memcpy (void *dest,const void *src,size_t len) { -if (!check_memory_region ((unsigned long )src, len, false, _RET_IP_) || - !check_memory_region ((unsigned long )dest, len, true, _RET_IP_)) +if (!kasan_check_range ((unsigned long )src, len, false, _RET_IP_) || + !kasan_check_range ((unsigned long )dest, len, true, _RET_IP_))return NULL ;return __memcpy (dest, src, len);@@ -72,7 +72,7 @@ void *memcpy(void *dest, const void *src, size_t len)* Poisons the shadow memory for 'size' bytes starting from 'addr'. * Memory addresses should be aligned to KASAN_GRANULE_SIZE. */ -void poison_range (const void *address,size_t size,u8 value) +void kasan_poison (const void *address,size_t size,u8 value) {void *shadow_start, *shadow_end;@@ -90,7 +90,7 @@ void poison_range(const void *address, size_t size, u8 value)__memset (shadow_start, value, shadow_end - shadow_start); } -void unpoison_range (const void *address,size_t size) +void kasan_unpoison (const void *address,size_t size) {u8 tag =get_tag (address);@@ -101,7 +101,7 @@ void unpoison_range(const void *address, size_t size)*/ address =kasan_reset_tag (address); -poison_range (address, size, tag); +kasan_poison (address, size, tag);if (size &KASAN_GRANULE_MASK ) {u8 *shadow = (u8 *)kasan_mem_to_shadow (address + size);@@ -286,7 +286,7 @@ int kasan_populate_vmalloc(unsigned long addr, unsigned long size)* // vmalloc() allocates memory * // let a = area->addr * // we reach kasan_populate_vmalloc -* // and call unpoison_range: +* // and call kasan_unpoison: * STORE shadow(a), unpoison_val * ... * STORE shadow(a+99), unpoison_val x = LOAD p @@ -321,7 +321,7 @@ void kasan_poison_vmalloc(const void *start, unsigned long size)return ; size =round_up (size,KASAN_GRANULE_SIZE ); -poison_range (start, size,KASAN_VMALLOC_INVALID ); +kasan_poison (start, size,KASAN_VMALLOC_INVALID ); }void kasan_unpoison_vmalloc (const void *start,unsigned long size)@@ -329,7 +329,7 @@ void kasan_unpoison_vmalloc(const void *start, unsigned long size)if (!is_vmalloc_or_module_addr (start))return ; -unpoison_range (start, size); +kasan_unpoison (start, size); } static int kasan_depopulate_vmalloc_pte(pte_t *ptep, unsigned long addr,mm/kasan/sw_tags.c +8
-8@@ -57,7 +57,7 @@ void __init kasan_init_sw_tags(void)* sequence has in fact positive effect, since interrupts that randomly skew * PRNG at unpredictable points do only good. */ -u8 random_tag (void ) +u8 kasan_random_tag (void ) {u32 state =this_cpu_read (prng_state);@@ -67,7 +67,7 @@ u8 random_tag(void)return (u8)(state % (KASAN_TAG_MAX +1 )); } -bool check_memory_region (unsigned long addr,size_t size,bool write, +bool kasan_check_range (unsigned long addr,size_t size,bool write,unsigned long ret_ip) {u8 tag;@@ -118,7 +118,7 @@ bool check_memory_region(unsigned long addr, size_t size, bool write,return true; } -bool check_invalid_free (void *addr) +bool kasan_check_invalid_free (void *addr) {u8 tag =get_tag (addr);u8 shadow_byte =READ_ONCE (*(u8 *)kasan_mem_to_shadow (kasan_reset_tag (addr)));@@ -130,12 +130,12 @@ bool check_invalid_free(void *addr)#define DEFINE_HWASAN_LOAD_STORE (size) \ void __hwasan_load##size##_noabort(unsigned long addr) \ { \ - check_memory_region(addr, size, false, _RET_IP_); \ + kasan_check_range(addr, size, false, _RET_IP_); \ } \ EXPORT_SYMBOL(__hwasan_load##size##_noabort); \ void __hwasan_store##size##_noabort(unsigned long addr) \ { \ - check_memory_region(addr, size, true, _RET_IP_); \ + kasan_check_range(addr, size, true, _RET_IP_); \ } \ EXPORT_SYMBOL(__hwasan_store##size##_noabort)@@ -147,19 +147,19 @@ DEFINE_HWASAN_LOAD_STORE(16);void __hwasan_loadN_noabort (unsigned long addr,unsigned long size) { -check_memory_region (addr, size, false, _RET_IP_); +kasan_check_range (addr, size, false, _RET_IP_); }EXPORT_SYMBOL (__hwasan_loadN_noabort);void __hwasan_storeN_noabort (unsigned long addr,unsigned long size) { -check_memory_region (addr, size, true, _RET_IP_); +kasan_check_range (addr, size, true, _RET_IP_); }EXPORT_SYMBOL (__hwasan_storeN_noabort);void __hwasan_tag_memory (unsigned long addr,u8 tag,unsigned long size) { -poison_range ((void *)addr, size, tag); +kasan_poison ((void *)addr, size, tag); }EXPORT_SYMBOL (__hwasan_tag_memory);tools/objtool/check.c +1
-1@@ -576,7 +576,7 @@ static void add_ignores(struct objtool_file *file)static const char *uaccess_safe_builtin[] = {/* KASAN */ "kasan_report" , -"check_memory_region" , +"kasan_check_range" ,/* KASAN out-of-line */ "__asan_loadN_noabort" ,"__asan_load1_noabort" ,Signature · 1 line
2.29.2.729.g45daf8777d-goog
PATCH 2/11 kasan: clarify HW_TAGS impact on TBI
In reply to Andrey KonovalovMention in the documentation that enabling CONFIG_KASAN_HW_TAGS always results in in-kernel TBI (Top Byte Ignore) being enabled.
Also do a few minor documentation cleanups.
--- Documentation/dev-tools/kasan.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
Documentation/dev-tools/kasan.rst +11 -5
@@ -147,15 +147,14 @@ negative values to distinguish between different kinds of inaccessible memory like redzones or freed memory (see mm/kasan/kasan.h). In the report above the arrows point to the shadow byte 03, which means that -the accessed address is partially accessible. - -For tag-based KASAN this last report section shows the memory tags around the -accessed address (see `Implementation details`_ section). +the accessed address is partially accessible. For tag-based KASAN modes this +last report section shows the memory tags around the accessed address +(see the `Implementation details`_ section). Boot parameters ~~~~~~~~~~~~~~~ -Hardware tag-based KASAN mode (see the section about different mode below) is +Hardware tag-based KASAN mode (see the section about various modes below) is intended for use in production as a security mitigation. Therefore it supports boot parameters that allow to disable KASAN competely or otherwise control particular KASAN features.
@@ -305,6 +304,13 @@ reserved to tag freed memory regions. Hardware tag-based KASAN currently only supports tagging of kmem_cache_alloc/kmalloc and page_alloc memory. +If the hardware doesn't support MTE (pre ARMv8.5), hardware tag-based KASAN +won't be enabled. In this case all boot parameters are ignored. + +Note, that enabling CONFIG_KASAN_HW_TAGS always results in in-kernel TBI being +enabled. Even when kasan.mode=off is provided, or when the hardware doesn't +support MTE (but supports TBI). + What memory accesses are sanitised by KASAN? --------------------------------------------
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Re: [PATCH 02/11] kasan: clarify HW_TAGS impact on TBI
Quoted 8 lines · Andrey Konovalov
Mention in the documentation that enabling
CONFIG_KASAN_HW_TAGSalways results in in-kernel TBI (Top Byte Ignore) being enabled.Also do a few minor documentation cleanups.
Quoted 45 lines
--- Documentation/dev-tools/kasan.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)Documentation/dev-tools/kasan.rst +11
-6@@ -147,15 +147,14 @@ negative values to distinguish between different kinds of inaccessible memory like redzones or freed memory (see mm/kasan/kasan.h). In the report above the arrows point to the shadow byte 03, which means that -the accessed address is partially accessible. - -For tag-based KASAN this last report section shows the memory tags around the -accessed address (see `Implementation details`_ section). +the accessed address is partially accessible. For tag-based KASAN modes this +last report section shows the memory tags around the accessed address +(see the `Implementation details`_ section). Boot parameters ~~~~~~~~~~~~~~~ -Hardware tag-based KASAN mode (see the section about different mode below) is +Hardware tag-based KASAN mode (see the section about various modes below) is intended for use in production as a security mitigation. Therefore it supports boot parameters that allow to disable KASAN competely or otherwise control particular KASAN features.@@ -305,6 +304,13 @@ reserved to tag freed memory regions. Hardware tag-based KASAN currently only supports tagging of kmem_cache_alloc/kmalloc and page_alloc memory. +If the hardware doesn't support MTE (pre ARMv8.5), hardware tag-based KASAN +won't be enabled. In this case all boot parameters are ignored. + +Note, that enabling CONFIG_KASAN_HW_TAGS always results in in-kernel TBI being +enabled. Even when kasan.mode=off is provided, or when the hardware doesn't +support MTE (but supports TBI). + What memory accesses are sanitised by KASAN? -------------------------------------------- --2.29.2.729.g45daf8777d-goog
Signature · 10 lines
Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
Re: [PATCH 02/11] kasan: clarify HW_TAGS impact on TBI
In reply to Andrey KonovalovQuoted 7 lines · Andrey Konovalov
Mention in the documentation that enabling
CONFIG_KASAN_HW_TAGSalways results in in-kernel TBI (Top Byte Ignore) being enabled.Also do a few minor documentation cleanups.
Quoted 45 lines
--- Documentation/dev-tools/kasan.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)Documentation/dev-tools/kasan.rst +11
-5@@ -147,15 +147,14 @@ negative values to distinguish between different kinds of inaccessible memory like redzones or freed memory (see mm/kasan/kasan.h). In the report above the arrows point to the shadow byte 03, which means that -the accessed address is partially accessible. - -For tag-based KASAN this last report section shows the memory tags around the -accessed address (see `Implementation details`_ section). +the accessed address is partially accessible. For tag-based KASAN modes this +last report section shows the memory tags around the accessed address +(see the `Implementation details`_ section). Boot parameters ~~~~~~~~~~~~~~~ -Hardware tag-based KASAN mode (see the section about different mode below) is +Hardware tag-based KASAN mode (see the section about various modes below) is intended for use in production as a security mitigation. Therefore it supports boot parameters that allow to disable KASAN competely or otherwise control particular KASAN features.@@ -305,6 +304,13 @@ reserved to tag freed memory regions. Hardware tag-based KASAN currently only supports tagging of kmem_cache_alloc/kmalloc and page_alloc memory. +If the hardware doesn't support MTE (pre ARMv8.5), hardware tag-based KASAN +won't be enabled. In this case all boot parameters are ignored. + +Note, that enabling CONFIG_KASAN_HW_TAGS always results in in-kernel TBI being +enabled. Even when kasan.mode=off is provided, or when the hardware doesn't +support MTE (but supports TBI). + What memory accesses are sanitised by KASAN? --------------------------------------------Signature · 1 line
2.29.2.729.g45daf8777d-goog
PATCH 3/11 kasan: clean up comments in tests
In reply to Andrey KonovalovClarify and update comments and info messages in KASAN tests.
--- lib/test_kasan.c | 94 +++++++++++++++++++++++------------------ lib/test_kasan_module.c | 5 ++- 2 files changed, 55 insertions(+), 44 deletions(-)
lib/test_kasan.c +52 -42
@@ -28,10 +28,9 @@#define OOB_TAG_OFF (IS_ENABLED(CONFIG_KASAN_GENERIC) ? 0 : KASAN_GRANULE_SIZE)/* -* We assign some test results to these globals to make sure the tests -* are not eliminated as dead code. +* Some tests use these global variables to store return values from function +* calls that could otherwise be eliminated by the compiler as dead code. */ -void *kasan_ptr_result;int kasan_int_result;
@@ -39,14 +38,13 @@ static struct kunit_resource resource;static struct kunit_kasan_expectation fail_data;static bool multishot; +/* +* Temporarily enable multi-shot mode. Otherwise, KASAN would only report the +* first detected bug and panic the kernel if panic_on_warn is enabled. +*/ static int kasan_test_init (struct kunit *test) { -/* -* Temporarily enable multi-shot mode and set panic_on_warn=0. -* Otherwise, we'd only get a report for the first case. -*/ multishot =kasan_save_enable_multi_shot (); -return 0 ; }
@@ -56,12 +54,12 @@ static void kasan_test_exit(struct kunit *test) }/** -* KUNIT_EXPECT_KASAN_FAIL() - Causes a test failure when the expression does -* not cause a KASAN error. This uses a KUnit resource named "kasan_data." Do -* Do not use this name for a KUnit resource outside here. -* +* KUNIT_EXPECT_KASAN_FAIL() - check that the executed expression produces a +* KASAN report; causes a test failure otherwise. This relies on a KUnit +* resource named "kasan_data". Do not use this name for KUnit resources +* outside of KASAN tests. */ -#define KUNIT_EXPECT_KASAN_FAIL (test, condition) do { \ +#define KUNIT_EXPECT_KASAN_FAIL (test, expression) do { \ fail_data.report_expected = true; \ fail_data.report_found = false; \ kunit_add_named_resource(test, \
@@ -69,7 +67,7 @@ static void kasan_test_exit(struct kunit *test)NULL , \ &resource, \ "kasan_data", &fail_data); \ - condition; \ + expression; \KUNIT_EXPECT_EQ (test, \ fail_data.report_expected, \ fail_data.report_found); \
@@ -117,11 +115,12 @@ static void kmalloc_pagealloc_oob_right(struct kunit *test)size_t size =KMALLOC_MAX_CACHE_SIZE +10 ;if (!IS_ENABLED (CONFIG_SLUB )) { -kunit_info (test,"CONFIG_SLUB is not enabled." ); +kunit_info (test,"skipping, CONFIG_SLUB required" );return ; } -/* Allocate a chunk that does not fit into a SLUB cache to trigger +/* +* Allocate a chunk that does not fit into a SLUB cache to trigger * the page allocator fallback. */ ptr =kmalloc (size,GFP_KERNEL );
@@ -137,7 +136,7 @@ static void kmalloc_pagealloc_uaf(struct kunit *test)size_t size =KMALLOC_MAX_CACHE_SIZE +10 ;if (!IS_ENABLED (CONFIG_SLUB )) { -kunit_info (test,"CONFIG_SLUB is not enabled." ); +kunit_info (test,"skipping, CONFIG_SLUB required" );return ; }
@@ -154,7 +153,7 @@ static void kmalloc_pagealloc_invalid_free(struct kunit *test)size_t size =KMALLOC_MAX_CACHE_SIZE +10 ;if (!IS_ENABLED (CONFIG_SLUB )) { -kunit_info (test,"CONFIG_SLUB is not enabled." ); +kunit_info (test,"skipping, CONFIG_SLUB required" );return ; }
@@ -168,7 +167,9 @@ static void kmalloc_large_oob_right(struct kunit *test) {char *ptr;size_t size =KMALLOC_MAX_CACHE_SIZE -256 ; -/* Allocate a chunk that is large enough, but still fits into a slab + +/* +* Allocate a chunk that is large enough, but still fits into a slab * and does not trigger the page allocator fallback in SLUB. */ ptr =kmalloc (size,GFP_KERNEL );
@@ -218,7 +219,7 @@ static void kmalloc_oob_16(struct kunit *test)/* This test is specifically crafted for the generic mode. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required\n" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }
@@ -454,7 +455,7 @@ static void kasan_global_oob(struct kunit *test)/* Only generic mode instruments globals. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }
@@ -469,10 +470,13 @@ static void ksize_unpoisons_memory(struct kunit *test) ptr =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); real_size =ksize (ptr); -/* This access doesn't trigger an error. */ + +/* This access shouldn't trigger a KASAN report. */ ptr[size] ='x' ; -/* This one does. */ + +/* This one must. */ KUNIT_EXPECT_KASAN_FAIL (test, ptr[real_size] ='y' ); +kfree (ptr); }
@@ -483,7 +487,7 @@ static void kasan_stack_oob(struct kunit *test)char *p = &stack_array[ARRAY_SIZE (stack_array) + i];if (!IS_ENABLED (CONFIG_KASAN_STACK )) { -kunit_info (test,"CONFIG_KASAN_STACK is not enabled" ); +kunit_info (test,"skipping, CONFIG_KASAN_STACK required" );return ; }
@@ -498,12 +502,12 @@ static void kasan_alloca_oob_left(struct kunit *test)/* Only generic mode instruments dynamic allocas. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }if (!IS_ENABLED (CONFIG_KASAN_STACK )) { -kunit_info (test,"CONFIG_KASAN_STACK is not enabled" ); +kunit_info (test,"skipping, CONFIG_KASAN_STACK required" );return ; }
@@ -518,12 +522,12 @@ static void kasan_alloca_oob_right(struct kunit *test)/* Only generic mode instruments dynamic allocas. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }if (!IS_ENABLED (CONFIG_KASAN_STACK )) { -kunit_info (test,"CONFIG_KASAN_STACK is not enabled" ); +kunit_info (test,"skipping, CONFIG_KASAN_STACK required" );return ; }
@@ -568,7 +572,7 @@ static void kmem_cache_invalid_free(struct kunit *test)return ; } -/* Trigger invalid free, the object doesn't get freed */ +/* Trigger invalid free, the object doesn't get freed. */ KUNIT_EXPECT_KASAN_FAIL (test,kmem_cache_free (cache, p +1 ));/*
@@ -585,10 +589,12 @@ static void kasan_memchr(struct kunit *test)char *ptr;size_t size =24 ; -/* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 */ +/* +* str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT. +* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 for details. +*/ if (IS_ENABLED (CONFIG_AMD_MEM_ENCRYPT )) { -kunit_info (test, -"str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT" ); +kunit_info (test,"skipping, CONFIG_AMD_MEM_ENCRYPT enabled" );return ; }
@@ -610,10 +616,12 @@ static void kasan_memcmp(struct kunit *test)size_t size =24 ;int arr[9 ]; -/* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 */ +/* +* str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT. +* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 for details. +*/ if (IS_ENABLED (CONFIG_AMD_MEM_ENCRYPT )) { -kunit_info (test, -"str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT" ); +kunit_info (test,"skipping, CONFIG_AMD_MEM_ENCRYPT enabled" );return ; }
@@ -634,10 +642,12 @@ static void kasan_strings(struct kunit *test)char *ptr;size_t size =24 ; -/* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 */ +/* +* str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT. +* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 for details. +*/ if (IS_ENABLED (CONFIG_AMD_MEM_ENCRYPT )) { -kunit_info (test, -"str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT" ); +kunit_info (test,"skipping, CONFIG_AMD_MEM_ENCRYPT enabled" );return ; }
@@ -701,12 +711,12 @@ static void kasan_bitops_generic(struct kunit *test)/* This test is specifically crafted for the generic mode. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required\n" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }/* -* Allocate 1 more byte, which causes kzalloc to round up to 16-bytes; +* Allocate 1 more byte, which causes kzalloc to round up to 16 bytes; * this way we do not actually corrupt other memory. */ bits =kzalloc (sizeof (*bits) +1 ,GFP_KERNEL );
@@ -733,7 +743,7 @@ static void kasan_bitops_tags(struct kunit *test)/* This test is specifically crafted for the tag-based mode. */ if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_SW_TAGS required\n" ); +kunit_info (test,"skipping, CONFIG_KASAN_SW_TAGS required" );return ; }
@@ -765,7 +775,7 @@ static void vmalloc_oob(struct kunit *test)void *area;if (!IS_ENABLED (CONFIG_KASAN_VMALLOC )) { -kunit_info (test,"CONFIG_KASAN_VMALLOC is not enabled." ); +kunit_info (test,"skipping, CONFIG_KASAN_VMALLOC required" );return ; }
lib/test_kasan_module.c +3 -2
@@ -123,8 +123,9 @@ static noinline void __init kasan_workqueue_uaf(void)static int __init test_kasan_module_init (void ) {/* -* Temporarily enable multi-shot mode. Otherwise, we'd only get a -* report for the first case. +* Temporarily enable multi-shot mode. Otherwise, KASAN would only +* report the first detected bug and panic the kernel if panic_on_warn +* is enabled. */ bool multishot =kasan_save_enable_multi_shot ();
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Alexander Potapenko
reviewerRe: [PATCH 03/11] kasan: clean up comments in tests
Quoted 9 lines · Andrey Konovalov
Clarify and update comments and info messages in KASAN tests.
Signed-off-by Andrey Konovalov <[email protected]>void
*kasan_ptr_result;
intkasan_int_result;
Shouldn't these two variables be static, by the way?
lib/test_kasan.c static struct kunit_resource resource; · line 44
Quoted 9 lines
lib/test_kasan.c +4
-0@@ -39,14 +38,13 @@ static struct kunit_resource resource;static struct kunit_kasan_expectation fail_data;static bool multishot; +/* +* Temporarily enable multi-shot mode. Otherwise, KASAN would only report the +* first detected bug and panic the kernel if panic_on_warn is enabled. +*/
YMMV, but I think this comment was at its place already.
lib/test_kasan.c static struct kunit_resource resource; · line 47
Quoted 7 lines
lib/test_kasan.c +0
-4static int kasan_test_init (struct kunit *test) { -/* -* Temporarily enable multi-shot mode and set panic_on_warn=0. -* Otherwise, we'd only get a report for the first case. -*/ multishot =kasan_save_enable_multi_shot ();
Unrelated to this change, but have you considered storing test-specific data in test->priv instead of globals?
lib/test_kasan.c static void kmalloc_pagealloc_oob_right(struct kunit *test) · line 120
Quoted 5 lines
lib/test_kasan.c +1
-1if (!IS_ENABLED (CONFIG_SLUB )) { -kunit_info (test,"CONFIG_SLUB is not enabled." ); +kunit_info (test,"skipping, CONFIG_SLUB required" );return ; }
You may want to introduce a macro that takes a config name and prints the warning/returns if it's not enabled.
Alex
Re: [PATCH 03/11] kasan: clean up comments in tests
Quoted 11 lines · Alexander Potapenko
Quoted 8 lines · Andrey Konovalov
Clarify and update comments and info messages in KASAN tests.
Signed-off-by Andrey Konovalov <[email protected]>void
*kasan_ptr_result;
intkasan_int_result;Shouldn't these two variables be static, by the way?
No, then the compiler starts eliminating accesses.
Quoted 10 lines
lib/test_kasan.c static struct kunit_resource resource; · line 44
Quoted 8 lines
lib/test_kasan.c +4
-0@@ -39,14 +38,13 @@ static struct kunit_resource resource;static struct kunit_kasan_expectation fail_data;static bool multishot; +/* +* Temporarily enable multi-shot mode. Otherwise, KASAN would only report the +* first detected bug and panic the kernel if panic_on_warn is enabled. +*/ YMMV, but I think this comment was at its place already.
It gets updated by one of the subsequent patches.
Quoted 10 lines
lib/test_kasan.c static struct kunit_resource resource; · line 47
Quoted 7 lines
lib/test_kasan.c +0
-4static int kasan_test_init (struct kunit *test) { -/* -* Temporarily enable multi-shot mode and set panic_on_warn=0. -* Otherwise, we'd only get a report for the first case. -*/ multishot =kasan_save_enable_multi_shot ();Unrelated to this change, but have you considered storing test-specific data in
test->privinstead of globals?
I'd say that test->priv is for some per-test data that's used in the tests, and multishot is not a part of that.
Quoted 8 lines
lib/test_kasan.c static void kmalloc_pagealloc_oob_right(struct kunit *test) · line 120
Quoted 5 lines
lib/test_kasan.c +1
-1if (!IS_ENABLED (CONFIG_SLUB )) { -kunit_info (test,"CONFIG_SLUB is not enabled." ); +kunit_info (test,"skipping, CONFIG_SLUB required" );return ; }You may want to introduce a macro that takes a config name and prints the warning/returns if it's not enabled.
Good idea, will do in v2.
Thanks!
Re: [PATCH 03/11] kasan: clean up comments in tests
In reply to Andrey KonovalovQuoted 4 lines · Andrey Konovalov
Clarify and update comments and info messages in KASAN tests.
Quoted 294 lines
--- lib/test_kasan.c | 94 +++++++++++++++++++++++------------------ lib/test_kasan_module.c | 5 ++- 2 files changed, 55 insertions(+), 44 deletions(-)lib/test_kasan.c +52
-42@@ -28,10 +28,9 @@#define OOB_TAG_OFF (IS_ENABLED(CONFIG_KASAN_GENERIC) ? 0 : KASAN_GRANULE_SIZE)/* -* We assign some test results to these globals to make sure the tests -* are not eliminated as dead code. +* Some tests use these global variables to store return values from function +* calls that could otherwise be eliminated by the compiler as dead code. */ -void *kasan_ptr_result;int kasan_int_result;@@ -39,14 +38,13 @@ static struct kunit_resource resource;static struct kunit_kasan_expectation fail_data;static bool multishot; +/* +* Temporarily enable multi-shot mode. Otherwise, KASAN would only report the +* first detected bug and panic the kernel if panic_on_warn is enabled. +*/ static int kasan_test_init (struct kunit *test) { -/* -* Temporarily enable multi-shot mode and set panic_on_warn=0. -* Otherwise, we'd only get a report for the first case. -*/ multishot =kasan_save_enable_multi_shot (); -return 0 ; }@@ -56,12 +54,12 @@ static void kasan_test_exit(struct kunit *test) }/** -* KUNIT_EXPECT_KASAN_FAIL() - Causes a test failure when the expression does -* not cause a KASAN error. This uses a KUnit resource named "kasan_data." Do -* Do not use this name for a KUnit resource outside here. -* +* KUNIT_EXPECT_KASAN_FAIL() - check that the executed expression produces a +* KASAN report; causes a test failure otherwise. This relies on a KUnit +* resource named "kasan_data". Do not use this name for KUnit resources +* outside of KASAN tests. */ -#define KUNIT_EXPECT_KASAN_FAIL (test, condition) do { \ +#define KUNIT_EXPECT_KASAN_FAIL (test, expression) do { \ fail_data.report_expected = true; \ fail_data.report_found = false; \ kunit_add_named_resource(test, \@@ -69,7 +67,7 @@ static void kasan_test_exit(struct kunit *test)NULL , \ &resource, \ "kasan_data", &fail_data); \ - condition; \ + expression; \KUNIT_EXPECT_EQ (test, \ fail_data.report_expected, \ fail_data.report_found); \@@ -117,11 +115,12 @@ static void kmalloc_pagealloc_oob_right(struct kunit *test)size_t size =KMALLOC_MAX_CACHE_SIZE +10 ;if (!IS_ENABLED (CONFIG_SLUB )) { -kunit_info (test,"CONFIG_SLUB is not enabled." ); +kunit_info (test,"skipping, CONFIG_SLUB required" );return ; } -/* Allocate a chunk that does not fit into a SLUB cache to trigger +/* +* Allocate a chunk that does not fit into a SLUB cache to trigger * the page allocator fallback. */ ptr =kmalloc (size,GFP_KERNEL );@@ -137,7 +136,7 @@ static void kmalloc_pagealloc_uaf(struct kunit *test)size_t size =KMALLOC_MAX_CACHE_SIZE +10 ;if (!IS_ENABLED (CONFIG_SLUB )) { -kunit_info (test,"CONFIG_SLUB is not enabled." ); +kunit_info (test,"skipping, CONFIG_SLUB required" );return ; }@@ -154,7 +153,7 @@ static void kmalloc_pagealloc_invalid_free(struct kunit *test)size_t size =KMALLOC_MAX_CACHE_SIZE +10 ;if (!IS_ENABLED (CONFIG_SLUB )) { -kunit_info (test,"CONFIG_SLUB is not enabled." ); +kunit_info (test,"skipping, CONFIG_SLUB required" );return ; }@@ -168,7 +167,9 @@ static void kmalloc_large_oob_right(struct kunit *test) {char *ptr;size_t size =KMALLOC_MAX_CACHE_SIZE -256 ; -/* Allocate a chunk that is large enough, but still fits into a slab + +/* +* Allocate a chunk that is large enough, but still fits into a slab * and does not trigger the page allocator fallback in SLUB. */ ptr =kmalloc (size,GFP_KERNEL );@@ -218,7 +219,7 @@ static void kmalloc_oob_16(struct kunit *test)/* This test is specifically crafted for the generic mode. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required\n" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }@@ -454,7 +455,7 @@ static void kasan_global_oob(struct kunit *test)/* Only generic mode instruments globals. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }@@ -469,10 +470,13 @@ static void ksize_unpoisons_memory(struct kunit *test) ptr =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); real_size =ksize (ptr); -/* This access doesn't trigger an error. */ + +/* This access shouldn't trigger a KASAN report. */ ptr[size] ='x' ; -/* This one does. */ + +/* This one must. */ KUNIT_EXPECT_KASAN_FAIL (test, ptr[real_size] ='y' ); +kfree (ptr); }@@ -483,7 +487,7 @@ static void kasan_stack_oob(struct kunit *test)char *p = &stack_array[ARRAY_SIZE (stack_array) + i];if (!IS_ENABLED (CONFIG_KASAN_STACK )) { -kunit_info (test,"CONFIG_KASAN_STACK is not enabled" ); +kunit_info (test,"skipping, CONFIG_KASAN_STACK required" );return ; }@@ -498,12 +502,12 @@ static void kasan_alloca_oob_left(struct kunit *test)/* Only generic mode instruments dynamic allocas. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }if (!IS_ENABLED (CONFIG_KASAN_STACK )) { -kunit_info (test,"CONFIG_KASAN_STACK is not enabled" ); +kunit_info (test,"skipping, CONFIG_KASAN_STACK required" );return ; }@@ -518,12 +522,12 @@ static void kasan_alloca_oob_right(struct kunit *test)/* Only generic mode instruments dynamic allocas. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }if (!IS_ENABLED (CONFIG_KASAN_STACK )) { -kunit_info (test,"CONFIG_KASAN_STACK is not enabled" ); +kunit_info (test,"skipping, CONFIG_KASAN_STACK required" );return ; }@@ -568,7 +572,7 @@ static void kmem_cache_invalid_free(struct kunit *test)return ; } -/* Trigger invalid free, the object doesn't get freed */ +/* Trigger invalid free, the object doesn't get freed. */ KUNIT_EXPECT_KASAN_FAIL (test,kmem_cache_free (cache, p +1 ));/* @@ -585,10 +589,12 @@ static void kasan_memchr(struct kunit *test)char *ptr;size_t size =24 ; -/* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 */ +/* +* str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT. +* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 for details. +*/ if (IS_ENABLED (CONFIG_AMD_MEM_ENCRYPT )) { -kunit_info (test, -"str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT" ); +kunit_info (test,"skipping, CONFIG_AMD_MEM_ENCRYPT enabled" );return ; }@@ -610,10 +616,12 @@ static void kasan_memcmp(struct kunit *test)size_t size =24 ;int arr[9 ]; -/* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 */ +/* +* str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT. +* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 for details. +*/ if (IS_ENABLED (CONFIG_AMD_MEM_ENCRYPT )) { -kunit_info (test, -"str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT" ); +kunit_info (test,"skipping, CONFIG_AMD_MEM_ENCRYPT enabled" );return ; }@@ -634,10 +642,12 @@ static void kasan_strings(struct kunit *test)char *ptr;size_t size =24 ; -/* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 */ +/* +* str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT. +* See https://bugzilla.kernel.org/show_bug.cgi?id=206337 for details. +*/ if (IS_ENABLED (CONFIG_AMD_MEM_ENCRYPT )) { -kunit_info (test, -"str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT" ); +kunit_info (test,"skipping, CONFIG_AMD_MEM_ENCRYPT enabled" );return ; }@@ -701,12 +711,12 @@ static void kasan_bitops_generic(struct kunit *test)/* This test is specifically crafted for the generic mode. */ if (!IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_GENERIC required\n" ); +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC required" );return ; }/* -* Allocate 1 more byte, which causes kzalloc to round up to 16-bytes; +* Allocate 1 more byte, which causes kzalloc to round up to 16 bytes; * this way we do not actually corrupt other memory. */ bits =kzalloc (sizeof (*bits) +1 ,GFP_KERNEL );@@ -733,7 +743,7 @@ static void kasan_bitops_tags(struct kunit *test)/* This test is specifically crafted for the tag-based mode. */ if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"CONFIG_KASAN_SW_TAGS required\n" ); +kunit_info (test,"skipping, CONFIG_KASAN_SW_TAGS required" );return ; }@@ -765,7 +775,7 @@ static void vmalloc_oob(struct kunit *test)void *area;if (!IS_ENABLED (CONFIG_KASAN_VMALLOC )) { -kunit_info (test,"CONFIG_KASAN_VMALLOC is not enabled." ); +kunit_info (test,"skipping, CONFIG_KASAN_VMALLOC required" );return ; }lib/test_kasan_module.c +3
-2@@ -123,8 +123,9 @@ static noinline void __init kasan_workqueue_uaf(void)static int __init test_kasan_module_init (void ) {/* -* Temporarily enable multi-shot mode. Otherwise, we'd only get a -* report for the first case. +* Temporarily enable multi-shot mode. Otherwise, KASAN would only +* report the first detected bug and panic the kernel if panic_on_warn +* is enabled. */ bool multishot =kasan_save_enable_multi_shot ();Signature · 1 line
2.29.2.729.g45daf8777d-goog
PATCH 4/11 kasan: add match-all tag tests
In reply to Andrey KonovalovAdd 3 new tests for tag-based KASAN modes:
1. Check that match-all pointer tag is not assigned randomly.
2. Check that 0xff works as a match-all pointer tag.
3. Check that there are no match-all memory tags.
Note, that test #3 causes a significant number (255) of KASAN reports to be printed during execution for the SW_TAGS mode.
--- lib/test_kasan.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ mm/kasan/kasan.h | 6 ++++ 2 files changed, 99 insertions(+)
lib/test_kasan.c +93 -0
@@ -13,6 +13,7 @@#include <linux/mman.h> #include <linux/module.h> #include <linux/printk.h> +#include <linux/random.h> #include <linux/slab.h> #include <linux/string.h> #include <linux/uaccess.h>
@@ -790,6 +791,95 @@ static void vmalloc_oob(struct kunit *test)vfree (area); } +/* +* Check that match-all pointer tag is not assigned randomly for +* tag-based modes. +*/ +static void match_all_not_assigned (struct kunit *test) +{ +char *ptr; +struct page *pages; +int i, size, order; + +for (i =0 ; i <256 ; i++) { + size =get_random_int () %KMALLOC_MAX_SIZE ; + ptr =kmalloc (128 ,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +KUNIT_EXPECT_NE (test, (u8 )get_tag (ptr), (u8 )KASAN_TAG_KERNEL ); +kfree (ptr); + } + +for (i =0 ; i <256 ; i++) { + order =get_random_int () %4 ; + pages =alloc_pages (GFP_KERNEL , order); + ptr =page_address (pages); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +KUNIT_EXPECT_NE (test, (u8 )get_tag (ptr), (u8 )KASAN_TAG_KERNEL ); +free_pages ((unsigned long )ptr, order); + } +} + +/* Check that 0xff works as a match-all pointer tag for tag-based modes. */ +static void match_all_ptr_tag (struct kunit *test) +{ +char *ptr; +u8 tag; + +if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" ); +return ; + } + + ptr =kmalloc (128 ,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); + +/* Backup the assigned tag. */ + tag =get_tag (ptr); +KUNIT_EXPECT_NE (test, tag, (u8 )KASAN_TAG_KERNEL ); + +/* Reset the tag to 0xff.*/ + ptr =set_tag (ptr,KASAN_TAG_KERNEL ); + +/* This access shouldn't trigger a KASAN report. */ + *ptr =0 ; + +/* Recover the pointer tag and free. */ + ptr =set_tag (ptr, tag); +kfree (ptr); +} + +/* Check that there are no match-all memory tags for tag-based modes. */ +static void match_all_mem_tag (struct kunit *test) +{ +char *ptr; +int tag; + +if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" ); +return ; + } + + ptr =kmalloc (128 ,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +KUNIT_EXPECT_NE (test, (u8 )get_tag (ptr), (u8 )KASAN_TAG_KERNEL ); + +/* For each possible tag value not matching the pointer tag. */ +for (tag =KASAN_TAG_MIN ; tag <=KASAN_TAG_KERNEL ; tag++) { +if (tag ==get_tag (ptr)) +continue ; + +/* Mark the first memory granule with the chosen memory tag. */ +kasan_poison (ptr,KASAN_GRANULE_SIZE , (u8 )tag); + +/* This access must cause a KASAN report. */ +KUNIT_EXPECT_KASAN_FAIL (test, *ptr =0 ); + } + +/* Recover the memory tag and free. */ +kasan_poison (ptr,KASAN_GRANULE_SIZE ,get_tag (ptr)); +kfree (ptr); +} +static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kmalloc_oob_right),KUNIT_CASE (kmalloc_oob_left),
@@ -829,6 +919,9 @@ static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kasan_bitops_tags),KUNIT_CASE (kmalloc_double_kzfree),KUNIT_CASE (vmalloc_oob), +KUNIT_CASE (match_all_not_assigned), +KUNIT_CASE (match_all_ptr_tag), + KUNIT_CASE(match_all_mem_tag), {} };
mm/kasan/kasan.h +6 -0
@@ -36,6 +36,12 @@ extern bool kasan_flag_panic __ro_after_init;#define KASAN_TAG_INVALID 0xFE/* inaccessible memory tag */ #define KASAN_TAG_MAX 0xFD/* maximum value for random tags */ +#ifdef CONFIG_KASAN_HW_TAGS +#define KASAN_TAG_MIN 0xF0/* mimimum value for random tags */ +#else +#define KASAN_TAG_MIN 0x00/* mimimum value for random tags */ +#endif +#ifdef CONFIG_KASAN_GENERIC #define KASAN_FREE_PAGE 0xFF/* page was freed */ #define KASAN_PAGE_REDZONE 0xFE/* redzone for kmalloc_large allocations */
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Alexander Potapenko
reviewerRe: [PATCH 04/11] kasan: add match-all tag tests
Quoted 30 lines · Andrey Konovalov
Add 3 new tests for tag-based KASAN modes:
1. Check that match-all pointer tag is not assigned randomly.
2. Check that0xffworks as a match-all pointer tag.
3. Check that there are no match-all memory tags.Note, that test #3 causes a significant number (255) of KASAN reports to be printed during execution for the
SW_TAGSmode.Signed-off-by Andrey Konovalov <[email protected]>--- lib/test_kasan.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ mm/kasan/kasan.h | 6 ++++ 2 files changed, 99 insertions(+)lib/test_kasan.c +1
-0@@ -13,6 +13,7 @@#include <linux/mman.h> #include <linux/module.h> #include <linux/printk.h> +#include <linux/random.h> #include <linux/slab.h> #include <linux/string.h> #include <linux/uaccess.h>
lib/test_kasan.c static void vmalloc_oob(struct kunit *test) · line 799
Quoted 9 lines
lib/test_kasan.c +6
-0@@ -790,6 +791,95 @@ static void vmalloc_oob(struct kunit *test)vfree (area); } +/* +* Check that match-all pointer tag is not assigned randomly for +* tag-based modes. +*/ +static void match_all_not_assigned (struct kunit *test) +{
Do we want to run this test in non-tag-based modes? Probably not?
Re: [PATCH 04/11] kasan: add match-all tag tests
Quoted 35 lines · Alexander Potapenko
Quoted 33 lines · Andrey Konovalov
Add 3 new tests for tag-based KASAN modes:
1. Check that match-all pointer tag is not assigned randomly.
2. Check that0xffworks as a match-all pointer tag.
3. Check that there are no match-all memory tags.Note, that test #3 causes a significant number (255) of KASAN reports to be printed during execution for the
SW_TAGSmode.Signed-off-by Andrey Konovalov <[email protected]>--- lib/test_kasan.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ mm/kasan/kasan.h | 6 ++++ 2 files changed, 99 insertions(+)lib/test_kasan.c +1
-0@@ -13,6 +13,7 @@#include <linux/mman.h> #include <linux/module.h> #include <linux/printk.h> +#include <linux/random.h> #include <linux/slab.h> #include <linux/string.h> #include <linux/uaccess.h> @@ -790,6 +791,95 @@ static void vmalloc_oob(struct kunit *test)vfree (area); }
Quoted 8 lines
lib/test_kasan.c static void vmalloc_oob(struct kunit *test) · line 799
Quoted 6 lines
lib/test_kasan.c +6
-0+/* +* Check that match-all pointer tag is not assigned randomly for +* tag-based modes. +*/ +static void match_all_not_assigned (struct kunit *test) +{Do we want to run this test in non-tag-based modes? Probably not?
Will fix in v2, thanks!
Re: [PATCH 04/11] kasan: add match-all tag tests
In reply to Andrey KonovalovQuoted 34 lines · Andrey Konovalov
Add 3 new tests for tag-based KASAN modes:
1. Check that match-all pointer tag is not assigned randomly.
2. Check that0xffworks as a match-all pointer tag.
3. Check that there are no match-all memory tags.Note, that test #3 causes a significant number (255) of KASAN reports to be printed during execution for the
SW_TAGSmode.Signed-off-by Andrey Konovalov <[email protected]>--- lib/test_kasan.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ mm/kasan/kasan.h | 6 ++++ 2 files changed, 99 insertions(+)lib/test_kasan.c +3
-0@@ -13,6 +13,7 @@#include <linux/mman.h> #include <linux/module.h> #include <linux/printk.h> +#include <linux/random.h> #include <linux/slab.h> #include <linux/string.h> #include <linux/uaccess.h> @@ -790,6 +791,95 @@ static void vmalloc_oob(struct kunit *test)vfree (area); } +/* +* Check that match-all pointer tag is not assigned randomly for
lib/test_kasan.c static void vmalloc_oob(struct kunit *test) · line 805
Quoted 10 lines
lib/test_kasan.c +10
-0+* tag-based modes. +*/ +static void match_all_not_assigned (struct kunit *test) +{ +char *ptr; +struct page *pages; +int i, size, order; + +for (i =0 ; i <256 ; i++) { + size =get_random_int () %KMALLOC_MAX_SIZE ;
size appears to be unused?
Quoted 110 lines
lib/test_kasan.c +80
-0+ ptr =kmalloc (128 ,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +KUNIT_EXPECT_NE (test, (u8 )get_tag (ptr), (u8 )KASAN_TAG_KERNEL ); +kfree (ptr); + } + +for (i =0 ; i <256 ; i++) { + order =get_random_int () %4 ; + pages =alloc_pages (GFP_KERNEL , order); + ptr =page_address (pages); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +KUNIT_EXPECT_NE (test, (u8 )get_tag (ptr), (u8 )KASAN_TAG_KERNEL ); +free_pages ((unsigned long )ptr, order); + } +} + +/* Check that 0xff works as a match-all pointer tag for tag-based modes. */ +static void match_all_ptr_tag (struct kunit *test) +{ +char *ptr; +u8 tag; + +if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" ); +return ; + } + + ptr =kmalloc (128 ,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); + +/* Backup the assigned tag. */ + tag =get_tag (ptr); +KUNIT_EXPECT_NE (test, tag, (u8 )KASAN_TAG_KERNEL ); + +/* Reset the tag to 0xff.*/ + ptr =set_tag (ptr,KASAN_TAG_KERNEL ); + +/* This access shouldn't trigger a KASAN report. */ + *ptr =0 ; + +/* Recover the pointer tag and free. */ + ptr =set_tag (ptr, tag); +kfree (ptr); +} + +/* Check that there are no match-all memory tags for tag-based modes. */ +static void match_all_mem_tag (struct kunit *test) +{ +char *ptr; +int tag; + +if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" ); +return ; + } + + ptr =kmalloc (128 ,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +KUNIT_EXPECT_NE (test, (u8 )get_tag (ptr), (u8 )KASAN_TAG_KERNEL ); + +/* For each possible tag value not matching the pointer tag. */ +for (tag =KASAN_TAG_MIN ; tag <=KASAN_TAG_KERNEL ; tag++) { +if (tag ==get_tag (ptr)) +continue ; + +/* Mark the first memory granule with the chosen memory tag. */ +kasan_poison (ptr,KASAN_GRANULE_SIZE , (u8 )tag); + +/* This access must cause a KASAN report. */ +KUNIT_EXPECT_KASAN_FAIL (test, *ptr =0 ); + } + +/* Recover the memory tag and free. */ +kasan_poison (ptr,KASAN_GRANULE_SIZE ,get_tag (ptr)); +kfree (ptr); +} +static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kmalloc_oob_right),KUNIT_CASE (kmalloc_oob_left),@@ -829,6 +919,9 @@ static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kasan_bitops_tags),KUNIT_CASE (kmalloc_double_kzfree),KUNIT_CASE (vmalloc_oob), +KUNIT_CASE (match_all_not_assigned), +KUNIT_CASE (match_all_ptr_tag), + KUNIT_CASE(match_all_mem_tag), {} };mm/kasan/kasan.h +6
-0@@ -36,6 +36,12 @@ extern bool kasan_flag_panic __ro_after_init;#define KASAN_TAG_INVALID 0xFE/* inaccessible memory tag */ #define KASAN_TAG_MAX 0xFD/* maximum value for random tags */ +#ifdef CONFIG_KASAN_HW_TAGS +#define KASAN_TAG_MIN 0xF0/* mimimum value for random tags */ +#else +#define KASAN_TAG_MIN 0x00/* mimimum value for random tags */ +#endif +#ifdef CONFIG_KASAN_GENERIC #define KASAN_FREE_PAGE 0xFF/* page was freed */ #define KASAN_PAGE_REDZONE 0xFE/* redzone for kmalloc_large allocations */ Signature · 1 line
2.29.2.729.g45daf8777d-goog
Re: [PATCH 04/11] kasan: add match-all tag tests
Quoted 34 lines · Marco Elver
Quoted 32 lines · Andrey Konovalov
Add 3 new tests for tag-based KASAN modes:
1. Check that match-all pointer tag is not assigned randomly.
2. Check that0xffworks as a match-all pointer tag.
3. Check that there are no match-all memory tags.Note, that test #3 causes a significant number (255) of KASAN reports to be printed during execution for the
SW_TAGSmode.Signed-off-by Andrey Konovalov <[email protected]>--- lib/test_kasan.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ mm/kasan/kasan.h | 6 ++++ 2 files changed, 99 insertions(+)lib/test_kasan.c +1
-0@@ -13,6 +13,7 @@#include <linux/mman.h> #include <linux/module.h> #include <linux/printk.h> +#include <linux/random.h> #include <linux/slab.h> #include <linux/string.h> #include <linux/uaccess.h> @@ -790,6 +791,95 @@ static void vmalloc_oob(struct kunit *test)vfree (area); }
Quoted 14 lines
lib/test_kasan.c static void vmalloc_oob(struct kunit *test) · line 805
Quoted 12 lines
lib/test_kasan.c +12
-0+/* +* Check that match-all pointer tag is not assigned randomly for +* tag-based modes. +*/ +static void match_all_not_assigned (struct kunit *test) +{ +char *ptr; +struct page *pages; +int i, size, order; + +for (i =0 ; i <256 ; i++) { + size =get_random_int () %KMALLOC_MAX_SIZE ;size appears to be unused?
Indeed, will fix in v2, thanks!
Quoted 111 lines
Quoted 111 lines
lib/test_kasan.c +80
-0+ ptr =kmalloc (128 ,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +KUNIT_EXPECT_NE (test, (u8 )get_tag (ptr), (u8 )KASAN_TAG_KERNEL ); +kfree (ptr); + } + +for (i =0 ; i <256 ; i++) { + order =get_random_int () %4 ; + pages =alloc_pages (GFP_KERNEL , order); + ptr =page_address (pages); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +KUNIT_EXPECT_NE (test, (u8 )get_tag (ptr), (u8 )KASAN_TAG_KERNEL ); +free_pages ((unsigned long )ptr, order); + } +} + +/* Check that 0xff works as a match-all pointer tag for tag-based modes. */ +static void match_all_ptr_tag (struct kunit *test) +{ +char *ptr; +u8 tag; + +if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" ); +return ; + } + + ptr =kmalloc (128 ,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); + +/* Backup the assigned tag. */ + tag =get_tag (ptr); +KUNIT_EXPECT_NE (test, tag, (u8 )KASAN_TAG_KERNEL ); + +/* Reset the tag to 0xff.*/ + ptr =set_tag (ptr,KASAN_TAG_KERNEL ); + +/* This access shouldn't trigger a KASAN report. */ + *ptr =0 ; + +/* Recover the pointer tag and free. */ + ptr =set_tag (ptr, tag); +kfree (ptr); +} + +/* Check that there are no match-all memory tags for tag-based modes. */ +static void match_all_mem_tag (struct kunit *test) +{ +char *ptr; +int tag; + +if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" ); +return ; + } + + ptr =kmalloc (128 ,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +KUNIT_EXPECT_NE (test, (u8 )get_tag (ptr), (u8 )KASAN_TAG_KERNEL ); + +/* For each possible tag value not matching the pointer tag. */ +for (tag =KASAN_TAG_MIN ; tag <=KASAN_TAG_KERNEL ; tag++) { +if (tag ==get_tag (ptr)) +continue ; + +/* Mark the first memory granule with the chosen memory tag. */ +kasan_poison (ptr,KASAN_GRANULE_SIZE , (u8 )tag); + +/* This access must cause a KASAN report. */ +KUNIT_EXPECT_KASAN_FAIL (test, *ptr =0 ); + } + +/* Recover the memory tag and free. */ +kasan_poison (ptr,KASAN_GRANULE_SIZE ,get_tag (ptr)); +kfree (ptr); +} +static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kmalloc_oob_right),KUNIT_CASE (kmalloc_oob_left),@@ -829,6 +919,9 @@ static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kasan_bitops_tags),KUNIT_CASE (kmalloc_double_kzfree),KUNIT_CASE (vmalloc_oob), +KUNIT_CASE (match_all_not_assigned), +KUNIT_CASE (match_all_ptr_tag), + KUNIT_CASE(match_all_mem_tag), {} };mm/kasan/kasan.h +6
-1@@ -36,6 +36,12 @@ extern bool kasan_flag_panic __ro_after_init;#define KASAN_TAG_INVALID 0xFE/* inaccessible memory tag */ #define KASAN_TAG_MAX 0xFD/* maximum value for random tags */ +#ifdef CONFIG_KASAN_HW_TAGS +#define KASAN_TAG_MIN 0xF0/* mimimum value for random tags */ +#else +#define KASAN_TAG_MIN 0x00/* mimimum value for random tags */ +#endif +#ifdef CONFIG_KASAN_GENERIC #define KASAN_FREE_PAGE 0xFF/* page was freed */ #define KASAN_PAGE_REDZONE 0xFE/* redzone for kmalloc_large allocations */ --2.29.2.729.g45daf8777d-goog
PATCH 5/11 kasan, arm64: allow using KUnit tests with HW_TAGS mode
In reply to Andrey KonovalovOn a high level, this patch allows running KUnit KASAN tests with the hardware tag-based KASAN mode.
Internally, this change reenables tag checking at the end of each KASAN test that triggers a tag fault and leads to tag checking being disabled.
With this patch KASAN tests are still failing for the hardware tag-based mode; fixes come in the next few patches.
--- arch/arm64/include/asm/memory.h | 1 + arch/arm64/include/asm/mte-kasan.h | 12 +++++++++ arch/arm64/kernel/mte.c | 12 +++++++++ arch/arm64/mm/fault.c | 16 +++++++----- lib/Kconfig.kasan | 4 +-- lib/test_kasan.c | 42 +++++++++++++++++++++--------- mm/kasan/kasan.h | 9 +++++++ 7 files changed, 75 insertions(+), 21 deletions(-)
arch/arm64/include/asm/memory.h +1 -0
@@ -232,6 +232,7 @@ static inline const void *__tag_set(const void *addr, u8 tag)#ifdef CONFIG_KASAN_HW_TAGS #define arch_enable_tagging () mte_enable_kernel() +#define arch_set_tagging_report_once (state) mte_set_report_once(state)#define arch_init_tags (max_tag) mte_init_tags(max_tag)#define arch_get_random_tag () mte_get_random_tag()#define arch_get_mem_tag (addr) mte_get_mem_tag(addr)
arch/arm64/include/asm/mte-kasan.h +12 -0
@@ -32,6 +32,9 @@ void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag);void mte_enable_kernel (void );void mte_init_tags (u64 max_tag); +void mte_set_report_once (bool state); +bool mte_report_once (void ); +#else /* CONFIG_ARM64_MTE */ static inline u8 mte_get_ptr_tag (void *ptr)
@@ -60,6 +63,15 @@ static inline void mte_init_tags(u64 max_tag) { } +static inline void mte_set_report_once (bool state) +{ +} + +static inline bool mte_report_once (void ) +{ +return false; +} +#endif /* CONFIG_ARM64_MTE */ #endif /* __ASSEMBLY__ */
arch/arm64/kernel/mte.c +12 -0
@@ -25,6 +25,8 @@u64 gcr_kernel_excl__ro_after_init ; +static bool report_fault_once = true; +static void mte_sync_page_tags (struct page *page,pte_t *ptep,bool check_swap) {pte_t old_pte =READ_ONCE (*ptep);
@@ -158,6 +160,16 @@ void mte_enable_kernel(void)isb (); } +void mte_set_report_once (bool state) +{ +WRITE_ONCE (report_fault_once, state); +} + +bool mte_report_once (void ) +{ +return READ_ONCE (report_fault_once); +} +static void update_sctlr_el1_tcf0 (u64 tcf0) {/* ISB required for the kernel uaccess routines */
arch/arm64/mm/fault.c +10 -6
@@ -302,12 +302,20 @@ static void die_kernel_fault(const char *msg, unsigned long addr,static void report_tag_fault (unsigned long addr,unsigned int esr,struct pt_regs *regs) { -bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +static bool reported; +bool is_write; + +if (READ_ONCE (reported)) +return ; + +if (mte_report_once ()) +WRITE_ONCE (reported, true);/* * SAS bits aren't set for all faults reported in EL1, so we can't * find out access size. */ + is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ;kasan_report (addr,0 , is_write, regs->pc); }#else
@@ -319,12 +327,8 @@ static inline void report_tag_fault(unsigned long addr, unsigned int esr,static void do_tag_recovery (unsigned long addr,unsigned int esr,struct pt_regs *regs) { -static bool reported; -if (!READ_ONCE (reported)) { -report_tag_fault (addr, esr, regs); -WRITE_ONCE (reported, true); - } +report_tag_fault (addr, esr, regs);/* * Disable MTE Tag Checking on the local CPU for the current EL.
lib/Kconfig.kasan +2 -2
@@ -190,11 +190,11 @@ config KASAN_KUNIT_TEST kernel debugging features like KASAN. For more information on KUnit and unit tests in general, please refer - to the KUnit documentation in Documentation/dev-tools/kunit + to the KUnit documentation in Documentation/dev-tools/kunit. config TEST_KASAN_MODULE tristate "KUnit-incompatible tests of KASAN bug detection capabilities" - depends on m && KASAN + depends on m && KASAN && !KASAN_HW_TAGS help This is a part of the KASAN test suite that is incompatible with KUnit. Currently includes tests that do bad copy_from/to_user
lib/test_kasan.c +29 -13
64 lines not on this page · raw message
mm/kasan/kasan.h +9 -0
@@ -280,6 +280,9 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)#ifndef arch_init_tags#define arch_init_tags (max_tag)#endif +#ifndef arch_set_tagging_report_once +#define arch_set_tagging_report_once (state) +#endif #ifndef arch_get_random_tag#define arch_get_random_tag () (0xFF)#endif
@@ -292,10 +295,16 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)#define hw_enable_tagging () arch_enable_tagging()#define hw_init_tags (max_tag) arch_init_tags(max_tag) +#define hw_set_tagging_report_once (state) arch_set_tagging_report_once(state)#define hw_get_random_tag () arch_get_random_tag()#define hw_get_mem_tag (addr) arch_get_mem_tag(addr)#define hw_set_mem_tag_range (addr, size, tag) arch_set_mem_tag_range((addr), (size), (tag)) +#else /* CONFIG_KASAN_HW_TAGS */ + +#define hw_enable_tagging () +#define hw_set_tagging_report_once (state) +#endif /* CONFIG_KASAN_HW_TAGS */ #ifdef CONFIG_KASAN_SW_TAGS
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Re: [PATCH 05/11] kasan, arm64: allow using KUnit tests with HW_TAGS mode
Quoted 7 lines · Andrey Konovalov
arch/arm64/mm/fault.c +0
-0@@ -302,12 +302,20 @@ static void die_kernel_fault(const char *msg, unsigned long addr,static void report_tag_fault (unsigned long addr,unsigned int esr,struct pt_regs *regs)
arch/arm64/mm/fault.c static void die_kernel_fault(const char *msg, unsigned long addr, · line 312
Quoted 10 lines
arch/arm64/mm/fault.c +8
-1{ -bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +static bool reported; +bool is_write; + +if (READ_ONCE (reported)) +return ; + +if (mte_report_once ()) +WRITE_ONCE (reported, true);
I guess the assumption here is that you don't get any report before the tests start and temporarily set report_once to false. It's probably fine, if we get a tag check failure we'd notice in the logs anyway.
arch/arm64/mm/fault.c static void die_kernel_fault(const char *msg, unsigned long addr, · line 318
Quoted 5 lines
arch/arm64/mm/fault.c +1
-0/* * SAS bits aren't set for all faults reported in EL1, so we can't * find out access size. */ + is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ;
I now noticed, you could write this in a shorter way:
is_write = !!(esr & ESR_ELx_WNR);
arch/arm64/mm/fault.c static void die_kernel_fault(const char *msg, unsigned long addr, · line 320
Quoted 2 lines
arch/arm64/mm/fault.c +0
-0kasan_report (addr,0 , is_write, regs->pc); }
The patch looks fine to me.
Re: [PATCH 05/11] kasan, arm64: allow using KUnit tests with HW_TAGS mode
Quoted 15 lines · Catalin Marinas
Quoted 13 lines · Andrey Konovalov
arch/arm64/mm/fault.c +4
-1@@ -302,12 +302,20 @@ static void die_kernel_fault(const char *msg, unsigned long addr,static void report_tag_fault (unsigned long addr,unsigned int esr,struct pt_regs *regs) { -bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +static bool reported; +bool is_write; + +if (READ_ONCE (reported))
Quoted 8 lines
arch/arm64/mm/fault.c static void die_kernel_fault(const char *msg, unsigned long addr, · line 312
Quoted 4 lines
arch/arm64/mm/fault.c +4
-0+return ; + +if (mte_report_once ()) +WRITE_ONCE (reported, true);I guess the assumption here is that you don't get any report before the tests start and temporarily set
report_onceto false. It's probably fine, if we get a tag check failure we'd notice in the logs anyway.
Good point. I'll add a note in a comment in v4.
Quoted 6 lines
arch/arm64/mm/fault.c static void die_kernel_fault(const char *msg, unsigned long addr, · line 318
Quoted 5 lines
arch/arm64/mm/fault.c +1
-0/* * SAS bits aren't set for all faults reported in EL1, so we can't * find out access size. */ + is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ;
Quoted 6 lines
I now noticed, you could write this in a shorter way:
is_write = !!(esr & ESR_ELx_WNR);Quoted 2 lines
arch/arm64/mm/fault.c +0
-0kasan_report (addr,0 , is_write, regs->pc); }
Will do in v4.
Quoted 3 lines
The patch looks fine to me.
Reviewed-by Catalin Marinas <[email protected]>
Thanks!
Re: [PATCH 05/11] kasan, arm64: allow using KUnit tests with HW_TAGS mode
In reply to Andrey KonovalovHi Andrey,
Quoted 9 lines · Andrey Konovalov
On a high level, this patch allows running KUnit KASAN tests with the hardware tag-based KASAN mode.
Internally, this change reenables tag checking at the end of each KASAN test that triggers a tag fault and leads to tag checking being disabled.
With this patch KASAN tests are still failing for the hardware tag-based mode; fixes come in the next few patches.
A part what Catalin noticed already:
Quoted 244 lines
Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/include/asm/memory.h | 1 + arch/arm64/include/asm/mte-kasan.h | 12 +++++++++ arch/arm64/kernel/mte.c | 12 +++++++++ arch/arm64/mm/fault.c | 16 +++++++----- lib/Kconfig.kasan | 4 +-- lib/test_kasan.c | 42 +++++++++++++++++++++--------- mm/kasan/kasan.h | 9 +++++++ 7 files changed, 75 insertions(+), 21 deletions(-)arch/arm64/include/asm/memory.h +1
-0@@ -232,6 +232,7 @@ static inline const void *__tag_set(const void *addr, u8 tag)#ifdef CONFIG_KASAN_HW_TAGS #define arch_enable_tagging () mte_enable_kernel() +#define arch_set_tagging_report_once (state) mte_set_report_once(state)#define arch_init_tags (max_tag) mte_init_tags(max_tag)#define arch_get_random_tag () mte_get_random_tag()#define arch_get_mem_tag (addr) mte_get_mem_tag(addr)arch/arm64/include/asm/mte-kasan.h +12
-0@@ -32,6 +32,9 @@ void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag);void mte_enable_kernel (void );void mte_init_tags (u64 max_tag); +void mte_set_report_once (bool state); +bool mte_report_once (void ); +#else /* CONFIG_ARM64_MTE */ static inline u8 mte_get_ptr_tag (void *ptr)@@ -60,6 +63,15 @@ static inline void mte_init_tags(u64 max_tag) { } +static inline void mte_set_report_once (bool state) +{ +} + +static inline bool mte_report_once (void ) +{ +return false; +} +#endif /* CONFIG_ARM64_MTE */ #endif /* __ASSEMBLY__ */ arch/arm64/kernel/mte.c +12
-0@@ -25,6 +25,8 @@u64 gcr_kernel_excl__ro_after_init ; +static bool report_fault_once = true; +static void mte_sync_page_tags (struct page *page,pte_t *ptep,bool check_swap) {pte_t old_pte =READ_ONCE (*ptep);@@ -158,6 +160,16 @@ void mte_enable_kernel(void)isb (); } +void mte_set_report_once (bool state) +{ +WRITE_ONCE (report_fault_once, state); +} + +bool mte_report_once (void ) +{ +return READ_ONCE (report_fault_once); +} +static void update_sctlr_el1_tcf0 (u64 tcf0) {/* ISB required for the kernel uaccess routines */ arch/arm64/mm/fault.c +10
-6@@ -302,12 +302,20 @@ static void die_kernel_fault(const char *msg, unsigned long addr,static void report_tag_fault (unsigned long addr,unsigned int esr,struct pt_regs *regs) { -bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +static bool reported; +bool is_write; + +if (READ_ONCE (reported)) +return ; + +if (mte_report_once ()) +WRITE_ONCE (reported, true);/* * SAS bits aren't set for all faults reported in EL1, so we can't * find out access size. */ + is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ;kasan_report (addr,0 , is_write, regs->pc); }#else @@ -319,12 +327,8 @@ static inline void report_tag_fault(unsigned long addr, unsigned int esr,static void do_tag_recovery (unsigned long addr,unsigned int esr,struct pt_regs *regs) { -static bool reported; -if (!READ_ONCE (reported)) { -report_tag_fault (addr, esr, regs); -WRITE_ONCE (reported, true); - } +report_tag_fault (addr, esr, regs);/* * Disable MTE Tag Checking on the local CPU for the current EL. lib/Kconfig.kasan +2
-2@@ -190,11 +190,11 @@ config KASAN_KUNIT_TEST kernel debugging features like KASAN. For more information on KUnit and unit tests in general, please refer - to the KUnit documentation in Documentation/dev-tools/kunit + to the KUnit documentation in Documentation/dev-tools/kunit. config TEST_KASAN_MODULE tristate "KUnit-incompatible tests of KASAN bug detection capabilities" - depends on m && KASAN + depends on m && KASAN && !KASAN_HW_TAGS help This is a part of the KASAN test suite that is incompatible with KUnit. Currently includes tests that do bad copy_from/to_userlib/test_kasan.c +29
-13@@ -41,16 +41,20 @@ static bool multishot;/* * Temporarily enable multi-shot mode. Otherwise, KASAN would only report the -* first detected bug and panic the kernel if panic_on_warn is enabled. +* first detected bug and panic the kernel if panic_on_warn is enabled. For +* hardware tag-based KASAN also allow tag checking to be reenabled for each +* test, see the comment for KUNIT_EXPECT_KASAN_FAIL(). */ static int kasan_test_init (struct kunit *test) { multishot =kasan_save_enable_multi_shot (); +hw_set_tagging_report_once (false);return 0 ; }static void kasan_test_exit (struct kunit *test) { +hw_set_tagging_report_once (true);kasan_restore_multi_shot (multishot); }@@ -59,19 +63,31 @@ static void kasan_test_exit(struct kunit *test)* KASAN report; causes a test failure otherwise. This relies on a KUnit * resource named "kasan_data". Do not use this name for KUnit resources * outside of KASAN tests. +* +* For hardware tag-based KASAN, when a tag fault happens, tag checking is +* normally auto-disabled. When this happens, this test handler reenables +* tag checking. As tag checking can be only disabled or enabled per CPU, this +* handler disables migration (preemption). */ -#define KUNIT_EXPECT_KASAN_FAIL (test, expression) do { \ - fail_data.report_expected = true; \ - fail_data.report_found = false; \ - kunit_add_named_resource(test, \ - NULL, \ - NULL, \ - &resource, \ - "kasan_data", &fail_data); \ - expression; \ - KUNIT_EXPECT_EQ(test, \ - fail_data.report_expected, \ - fail_data.report_found); \ +#define KUNIT_EXPECT_KASAN_FAIL (test, expression) do { \ + if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) \ + migrate_disable(); \ + fail_data.report_expected = true; \ + fail_data.report_found = false; \ + kunit_add_named_resource(test, \ + NULL, \ + NULL, \ + &resource, \ + "kasan_data", &fail_data); \ + expression; \ + KUNIT_EXPECT_EQ(test, \ + fail_data.report_expected, \ + fail_data.report_found); \ + if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \ + if (fail_data.report_found) \ + hw_enable_tagging(); \ + migrate_enable(); \ + } \ } while (0)static void kmalloc_oob_right (struct kunit *test)mm/kasan/kasan.h +9
-0@@ -280,6 +280,9 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)#ifndef arch_init_tags#define arch_init_tags (max_tag)#endif +#ifndef arch_set_tagging_report_once +#define arch_set_tagging_report_once (state) +#endif #ifndef arch_get_random_tag#define arch_get_random_tag () (0xFF)#endif @@ -292,10 +295,16 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)#define hw_enable_tagging () arch_enable_tagging()#define hw_init_tags (max_tag) arch_init_tags(max_tag) +#define hw_set_tagging_report_once (state) arch_set_tagging_report_once(state)#define hw_get_random_tag () arch_get_random_tag()#define hw_get_mem_tag (addr) arch_get_mem_tag(addr)#define hw_set_mem_tag_range (addr, size, tag) arch_set_mem_tag_range((addr), (size), (tag)) +#else /* CONFIG_KASAN_HW_TAGS */ + +#define hw_enable_tagging () +#define hw_set_tagging_report_once (state) +#endif /* CONFIG_KASAN_HW_TAGS */ #ifdef CONFIG_KASAN_SW_TAGS
Signature · 2 lines
Regards, Vincenzo
PATCH 6/11 kasan: rename CONFIG_TEST_KASAN_MODULE
In reply to Andrey KonovalovRename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST.
This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST.
--- Documentation/dev-tools/kasan.rst | 6 +++--- lib/Kconfig.kasan | 2 +- lib/Makefile | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
Documentation/dev-tools/kasan.rst +3 -3
@@ -374,8 +374,8 @@ unmapped. This will require changes in arch-specific code. This allows ``VMAP_STACK`` support on x86, and can simplify support of architectures that do not have a fixed module region. -CONFIG_KASAN_KUNIT_TEST & CONFIG_TEST_KASAN_MODULE --------------------------------------------------- +CONFIG_KASAN_KUNIT_TEST and CONFIG_KASAN_MODULE_TEST +---------------------------------------------------- KASAN tests consist on two parts:
@@ -384,7 +384,7 @@ KASAN tests consist on two parts: automatically in a few different ways, see the instructions below. 2. Tests that are currently incompatible with KUnit. Enabled with -``CONFIG_TEST_KASAN_MODULE`` and can only be run as a module. These tests can +``CONFIG_KASAN_MODULE_TEST`` and can only be run as a module. These tests can only be verified manually, by loading the kernel module and inspecting the kernel log for KASAN reports.
lib/Kconfig.kasan +1 -1
@@ -192,7 +192,7 @@ config KASAN_KUNIT_TEST For more information on KUnit and unit tests in general, please refer to the KUnit documentation in Documentation/dev-tools/kunit. -config TEST_KASAN_MODULE +config KASAN_MODULE_TEST tristate "KUnit-incompatible tests of KASAN bug detection capabilities" depends on m && KASAN && !KASAN_HW_TAGS help
lib/Makefile +1 -1
@@ -68,7 +68,7 @@ obj-$(CONFIG_TEST_IDA) += test_ida.o obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o CFLAGS_test_kasan.o += -fno-builtin CFLAGS_test_kasan.o += $(call cc-disable-warning, vla) -obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o +obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o CFLAGS_test_kasan_module.o += -fno-builtin obj-$(CONFIG_TEST_UBSAN) += test_ubsan.o CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Re: [PATCH 06/11] kasan: rename CONFIG_TEST_KASAN_MODULE
Quoted 7 lines · Andrey Konovalov
Rename
CONFIG_TEST_KASAN_MODULEtoCONFIG_KASAN_MODULE_TEST.This naming is more consistent with the existing
CONFIG_KASAN_KUNIT_TEST.
Quoted 1 line
KASAN tests consist on two parts:
While at it: "consist of".
Re: [PATCH 06/11] kasan: rename CONFIG_TEST_KASAN_MODULE
Quoted 7 lines · Alexander Potapenko
Quoted 5 lines · Andrey Konovalov
Rename
CONFIG_TEST_KASAN_MODULEtoCONFIG_KASAN_MODULE_TEST.This naming is more consistent with the existing
CONFIG_KASAN_KUNIT_TEST.
Quoted 8 lines
Quoted 2 lines
Reviewed-by Alexander Potapenko <[email protected]>Quoted 1 line
KASAN tests consist on two parts:
While at it: "consist of".
Will do in v2, thanks!
Re: [PATCH 06/11] kasan: rename CONFIG_TEST_KASAN_MODULE
In reply to Andrey KonovalovQuoted 6 lines · Andrey Konovalov
Rename
CONFIG_TEST_KASAN_MODULEtoCONFIG_KASAN_MODULE_TEST.This naming is more consistent with the existing
CONFIG_KASAN_KUNIT_TEST.
For this patch, as-is. But we could potentially do better in future -- see below.
Quoted 48 lines
--- Documentation/dev-tools/kasan.rst | 6 +++--- lib/Kconfig.kasan | 2 +- lib/Makefile | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)Documentation/dev-tools/kasan.rst +3
-3@@ -374,8 +374,8 @@ unmapped. This will require changes in arch-specific code. This allows ``VMAP_STACK`` support on x86, and can simplify support of architectures that do not have a fixed module region. -CONFIG_KASAN_KUNIT_TEST & CONFIG_TEST_KASAN_MODULE --------------------------------------------------- +CONFIG_KASAN_KUNIT_TEST and CONFIG_KASAN_MODULE_TEST +---------------------------------------------------- KASAN tests consist on two parts:@@ -384,7 +384,7 @@ KASAN tests consist on two parts: automatically in a few different ways, see the instructions below. 2. Tests that are currently incompatible with KUnit. Enabled with -``CONFIG_TEST_KASAN_MODULE`` and can only be run as a module. These tests can +``CONFIG_KASAN_MODULE_TEST`` and can only be run as a module. These tests can only be verified manually, by loading the kernel module and inspecting the kernel log for KASAN reports.lib/Kconfig.kasan +1
-1@@ -192,7 +192,7 @@ config KASAN_KUNIT_TEST For more information on KUnit and unit tests in general, please refer to the KUnit documentation in Documentation/dev-tools/kunit. -config TEST_KASAN_MODULE +config KASAN_MODULE_TEST tristate "KUnit-incompatible tests of KASAN bug detection capabilities" depends on m && KASAN && !KASAN_HW_TAGS helplib/Makefile +0
-0
lib/Makefile obj-$(CONFIG_TEST_IDA) += test_ida.o · line 72
Quoted 6 lines
lib/Makefile +1
-1@@ -68,7 +68,7 @@ obj-$(CONFIG_TEST_IDA) += test_ida.o obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o CFLAGS_test_kasan.o += -fno-builtin CFLAGS_test_kasan.o += $(call cc-disable-warning, vla) -obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o +obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o CFLAGS_test_kasan_module.o += -fno-builtin
[1] https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-file-and-module-names
Do we eventually want to rename the tests to follow the style recommendation more closely?
Option 1: Rename the KUnit test to kasan_test.c? And then also rename test_kasan_module.c -> kasan_module_test.c? Then the file names would be mostly consistent with the config names.
Option 2: The style guide [1] also mentions where there are non-KUnit tests around to use _kunit for KUnit test, and _test (or similar) for the non-KUnit test. So here we'd end up with kasan_kunit.c and kasan_test.c. That would get rid of the confusing "module" part. The config variable could either remain CONFIG_KASAN_MODULE_TEST, or simply become CONFIG_KASAN_TEST, since we already have CONFIG_KASAN_KUNIT_TEST to distinguish.
But I won't bikeshed further. If you do a v2, I leave it to your judgement to decide what is most appropriate.
Thanks,
-- Marco
Re: [PATCH 06/11] kasan: rename CONFIG_TEST_KASAN_MODULE
Quoted 86 lines · Marco Elver
Quoted 6 lines · Andrey Konovalov
Rename
CONFIG_TEST_KASAN_MODULEtoCONFIG_KASAN_MODULE_TEST.This naming is more consistent with the existing
CONFIG_KASAN_KUNIT_TEST.Reviewed-by Marco Elver <[email protected]>For this patch, as-is. But we could potentially do better in future -- see below.
Quoted 48 lines
--- Documentation/dev-tools/kasan.rst | 6 +++--- lib/Kconfig.kasan | 2 +- lib/Makefile | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)Documentation/dev-tools/kasan.rst +3
-3@@ -374,8 +374,8 @@ unmapped. This will require changes in arch-specific code. This allows ``VMAP_STACK`` support on x86, and can simplify support of architectures that do not have a fixed module region. -CONFIG_KASAN_KUNIT_TEST & CONFIG_TEST_KASAN_MODULE --------------------------------------------------- +CONFIG_KASAN_KUNIT_TEST and CONFIG_KASAN_MODULE_TEST +---------------------------------------------------- KASAN tests consist on two parts:@@ -384,7 +384,7 @@ KASAN tests consist on two parts: automatically in a few different ways, see the instructions below. 2. Tests that are currently incompatible with KUnit. Enabled with -``CONFIG_TEST_KASAN_MODULE`` and can only be run as a module. These tests can +``CONFIG_KASAN_MODULE_TEST`` and can only be run as a module. These tests can only be verified manually, by loading the kernel module and inspecting the kernel log for KASAN reports.lib/Kconfig.kasan +1
-1@@ -192,7 +192,7 @@ config KASAN_KUNIT_TEST For more information on KUnit and unit tests in general, please refer to the KUnit documentation in Documentation/dev-tools/kunit. -config TEST_KASAN_MODULE +config KASAN_MODULE_TEST tristate "KUnit-incompatible tests of KASAN bug detection capabilities" depends on m && KASAN && !KASAN_HW_TAGS helplib/Makefile +0
-0lib/Makefile obj-$(CONFIG_TEST_IDA) += test_ida.o · line 72
Quoted 6 lines
lib/Makefile +1
-1@@ -68,7 +68,7 @@ obj-$(CONFIG_TEST_IDA) += test_ida.o obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o CFLAGS_test_kasan.o += -fno-builtin CFLAGS_test_kasan.o += $(call cc-disable-warning, vla) -obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o +obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o CFLAGS_test_kasan_module.o += -fno-builtin[1] https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-file-and-module-names
Do we eventually want to rename the tests to follow the style recommendation more closely?
Option 1: Rename the KUnit test to
kasan_test.c? And then also renametest_kasan_module.c->kasan_module_test.c? Then the file names would be mostly consistent with the config names.Option 2: The style guide [1] also mentions where there are non-KUnit tests around to use
_kunitfor KUnit test, and_test(or similar) for the non-KUnit test. So here we'd end up withkasan_kunit.candkasan_test.c. That would get rid of the confusing "module" part. The config variable could either remainCONFIG_KASAN_MODULE_TEST, or simply becomeCONFIG_KASAN_TEST, since we already haveCONFIG_KASAN_KUNIT_TESTto distinguish.
Quoted 2 lines
But I won't bikeshed further. If you do a v2, I leave it to your judgement to decide what is most appropriate.
Most tests in lib/ start with test_, so not using that pattern for KASAN tests could be confusing. Maybe we can move them to mm/kasan. Anyway, I won't look into this right now.
Thanks!
PATCH 7/11 kasan: add compiler barriers to KUNIT_EXPECT_KASAN_FAIL
In reply to Andrey KonovalovIt might not be obvious to the compiler that the expression must be executed between writing and reading to fail_data. In this case, the compiler might reorder or optimize away some of the accesses, and the tests will fail.
Add compiler barriers around the expression in KUNIT_EXPECT_KASAN_FAIL.
--- lib/test_kasan.c | 2 ++ 1 file changed, 2 insertions(+)
lib/test_kasan.c +2 -0
@@ -79,7 +79,9 @@ static void kasan_test_exit(struct kunit *test)NULL , \ &resource, \ "kasan_data", &fail_data); \ +barrier (); \ expression; \ +barrier (); \KUNIT_EXPECT_EQ (test, \ fail_data.report_expected, \ fail_data.report_found); \
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Alexander Potapenko
reviewerRe: [PATCH 07/11] kasan: add compiler barriers to KUNIT_EXPECT_KASAN_FAIL
Quoted 5 lines · Andrey Konovalov
It might not be obvious to the compiler that the expression must be executed between writing and reading to
fail_data. In this case, the compiler might reorder or optimize away some of the accesses, and the tests will fail.
Have you seen this happen in practice?
Are these accesses to fail_data that are optimized (in which case we could make it volatile), or some part of the expression? Note that compiler barriers won't probably help against removing memory accesses, they only prevent reordering.
lib/test_kasan.c static void kasan_test_exit(struct kunit *test) · line 84
Quoted 3 lines
lib/test_kasan.c +2
-0+barrier (); \ expression; \ +barrier (); \
The need for barriers is not obvious to the reader, so a comment in the code clarifying that would be nice.
Re: [PATCH 07/11] kasan: add compiler barriers to KUNIT_EXPECT_KASAN_FAIL
Quoted 9 lines · Alexander Potapenko
Quoted 5 lines · Andrey Konovalov
It might not be obvious to the compiler that the expression must be executed between writing and reading to
fail_data. In this case, the compiler might reorder or optimize away some of the accesses, and the tests will fail.Have you seen this happen in practice?
Yes.
Quoted 2 lines
Are these accesses to
fail_datathat are optimized (in which case we could make it volatile)?
Yes. AFAIU compiler doesn't expect expression to change fail_data fields, no those accesses and checks are optimized away.
Quoted 9 lines
Note that compiler barriers won't probably help against removing memory accesses, they only prevent reordering.
lib/test_kasan.c static void kasan_test_exit(struct kunit *test) · line 84
Quoted 3 lines
lib/test_kasan.c +2
-0+barrier (); \ expression; \ +barrier (); \The need for barriers is not obvious to the reader, so a comment in the code clarifying that would be nice.
Will add a comment in v2, thanks!
Re: [PATCH 07/11] kasan: add compiler barriers to KUNIT_EXPECT_KASAN_FAIL
Quoted 12 lines · Andrey Konovalov
Quoted 9 lines · Alexander Potapenko
Quoted 5 lines · Andrey Konovalov
It might not be obvious to the compiler that the expression must be executed between writing and reading to
fail_data. In this case, the compiler might reorder or optimize away some of the accesses, and the tests will fail.Have you seen this happen in practice?
Quoted 7 lines
Yes.
Quoted 2 lines
Are these accesses to
fail_datathat are optimized (in which case we could make it volatile)?Yes. AFAIU compiler doesn't expect expression to change
fail_datafields, no those accesses and checks are optimized away.
Ah, actually no, it reorders the expression and puts it after fail_data fields checks. That's why I put the barriers.
Quoted 2 lines
Quoted 2 lines
Note that compiler barriers won't probably help against removing memory accesses, they only prevent reordering.
But using WRITE/READ_ONCE() might also be a good idea, as technically the compiler can optimize away the accesses.
Re: [PATCH 07/11] kasan: add compiler barriers to KUNIT_EXPECT_KASAN_FAIL
In reply to Andrey KonovalovQuoted 9 lines · Andrey Konovalov
It might not be obvious to the compiler that the expression must be executed between writing and reading to
fail_data. In this case, the compiler might reorder or optimize away some of the accesses, and the tests will fail.Add compiler barriers around the expression in
KUNIT_EXPECT_KASAN_FAIL.
Quoted 21 lines
--- lib/test_kasan.c | 2 ++ 1 file changed, 2 insertions(+)lib/test_kasan.c +2
-0@@ -79,7 +79,9 @@ static void kasan_test_exit(struct kunit *test)NULL , \ &resource, \ "kasan_data", &fail_data); \ +barrier (); \ expression; \ +barrier (); \KUNIT_EXPECT_EQ (test, \ fail_data.report_expected, \ fail_data.report_found); \Signature · 1 line
2.29.2.729.g45daf8777d-goog
PATCH 8/11 kasan: adopt kmalloc_uaf2 test to HW_TAGS mode
In reply to Andrey KonovalovIn the kmalloc_uaf2() test, the pointers to the two allocated memory blocks might be the same, and the test will fail. With the software tag-based mode, the probability of the that happening is 1/254, so it's hard to observe the failure. For the hardware tag-based mode though, the probablity is 1/14, which is quite noticable.
Allow up to 4 attempts at generating different tags for the tag-based modes.
--- lib/test_kasan.c | 9 +++++++++ 1 file changed, 9 insertions(+)
lib/test_kasan.c +9 -0
@@ -375,7 +375,9 @@ static void kmalloc_uaf2(struct kunit *test) {char *ptr1, *ptr2;size_t size =43 ; +int counter =0 ; +again: ptr1 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr1);
@@ -384,6 +386,13 @@ static void kmalloc_uaf2(struct kunit *test) ptr2 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr2); +/* +* For tag-based KASAN ptr1 and ptr2 tags might happen to be the same. +* Allow up to 4 attempts at generating different tags. +*/ +if (!IS_ENABLED (CONFIG_KASAN_GENERIC ) && ptr1 == ptr2 && counter++ <4 ) + goto again; +KUNIT_EXPECT_KASAN_FAIL (test, ptr1[40 ] ='x' );KUNIT_EXPECT_PTR_NE (test, ptr1, ptr2);
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Alexander Potapenko
reviewerRe: [PATCH 08/11] kasan: adopt kmalloc_uaf2 test to HW_TAGS mode
Nit: s/adopt/adapt in the title.
Quoted 5 lines
lib/test_kasan.c +1
-0+again: ptr1 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr1);
lib/test_kasan.c static void kmalloc_uaf2(struct kunit *test) · line 395
Quoted 10 lines
lib/test_kasan.c +7
-0@@ -384,6 +386,13 @@ static void kmalloc_uaf2(struct kunit *test) ptr2 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr2); +/* +* For tag-based KASAN ptr1 and ptr2 tags might happen to be the same. +* Allow up to 4 attempts at generating different tags. +*/ +if (!IS_ENABLED (CONFIG_KASAN_GENERIC ) && ptr1 == ptr2 && counter++ <4 ) + goto again; +
Looks like we are leaking memory allocated for ptr2 here?
Re: [PATCH 08/11] kasan: adopt kmalloc_uaf2 test to HW_TAGS mode
Quoted 12 lines · Alexander Potapenko
Nit: s/adopt/adapt in the title.
Quoted 8 lines
lib/test_kasan.c +1
-0+again: ptr1 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr1);@@ -384,6 +386,13 @@ static void kmalloc_uaf2(struct kunit *test) ptr2 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr2);
Quoted 9 lines
lib/test_kasan.c static void kmalloc_uaf2(struct kunit *test) · line 395
Quoted 7 lines
lib/test_kasan.c +7
-0+/* +* For tag-based KASAN ptr1 and ptr2 tags might happen to be the same. +* Allow up to 4 attempts at generating different tags. +*/ +if (!IS_ENABLED (CONFIG_KASAN_GENERIC ) && ptr1 == ptr2 && counter++ <4 ) + goto again; +Looks like we are leaking memory allocated for ptr2 here?
Will fix in v2, thanks!
Re: [PATCH 08/11] kasan: adopt kmalloc_uaf2 test to HW_TAGS mode
In reply to Andrey KonovalovQuoted 30 lines · Andrey Konovalov
In the
kmalloc_uaf2()test, the pointers to the two allocated memory blocks might be the same, and the test will fail. With the software tag-based mode, the probability of the that happening is 1/254, so it's hard to observe the failure. For the hardware tag-based mode though, the probablity is 1/14, which is quite noticable.Allow up to 4 attempts at generating different tags for the tag-based modes.
Signed-off-by Andrey Konovalov <[email protected]>--- lib/test_kasan.c | 9 +++++++++ 1 file changed, 9 insertions(+)lib/test_kasan.c +2
-0@@ -375,7 +375,9 @@ static void kmalloc_uaf2(struct kunit *test) {char *ptr1, *ptr2;size_t size =43 ; +int counter =0 ; +again: ptr1 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr1);
lib/test_kasan.c static void kmalloc_uaf2(struct kunit *test) · line 395
Quoted 10 lines
lib/test_kasan.c +7
-0@@ -384,6 +386,13 @@ static void kmalloc_uaf2(struct kunit *test) ptr2 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr2); +/* +* For tag-based KASAN ptr1 and ptr2 tags might happen to be the same. +* Allow up to 4 attempts at generating different tags. +*/ +if (!IS_ENABLED (CONFIG_KASAN_GENERIC ) && ptr1 == ptr2 && counter++ <4 ) + goto again; +
Why do we even need a limit? Why not retry until ptr1 != ptr2?
Quoted 6 lines
lib/test_kasan.c +0
-0KUNIT_EXPECT_KASAN_FAIL (test, ptr1[40 ] ='x' );KUNIT_EXPECT_PTR_NE (test, ptr1, ptr2);Signature · 1 line
2.29.2.729.g45daf8777d-goog
Re: [PATCH 08/11] kasan: adopt kmalloc_uaf2 test to HW_TAGS mode
Quoted 35 lines · Marco Elver
Quoted 33 lines · Andrey Konovalov
In the
kmalloc_uaf2()test, the pointers to the two allocated memory blocks might be the same, and the test will fail. With the software tag-based mode, the probability of the that happening is 1/254, so it's hard to observe the failure. For the hardware tag-based mode though, the probablity is 1/14, which is quite noticable.Allow up to 4 attempts at generating different tags for the tag-based modes.
Signed-off-by Andrey Konovalov <[email protected]>--- lib/test_kasan.c | 9 +++++++++ 1 file changed, 9 insertions(+)lib/test_kasan.c +2
-0@@ -375,7 +375,9 @@ static void kmalloc_uaf2(struct kunit *test) {char *ptr1, *ptr2;size_t size =43 ; +int counter =0 ; +again: ptr1 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr1);@@ -384,6 +386,13 @@ static void kmalloc_uaf2(struct kunit *test) ptr2 =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr2);
Quoted 9 lines
lib/test_kasan.c static void kmalloc_uaf2(struct kunit *test) · line 395
Quoted 7 lines
lib/test_kasan.c +7
-0+/* +* For tag-based KASAN ptr1 and ptr2 tags might happen to be the same. +* Allow up to 4 attempts at generating different tags. +*/ +if (!IS_ENABLED (CONFIG_KASAN_GENERIC ) && ptr1 == ptr2 && counter++ <4 ) + goto again; +Why do we even need a limit? Why not retry until ptr1 != ptr2?
Then the test will hang if it's failing. Let's do up to 16 attempts, it should be more than enough in practice. Thanks!
PATCH 9/11 kasan: fix memory corruption in kasan_bitops_tags test
In reply to Andrey KonovalovSince the hardware tag-based KASAN mode might not have a redzone that comes after an allocated object (when kasan.mode=prod is enabled), the kasan_bitops_tags() test ends up corrupting the next object in memory.
Change the test so it always accesses the redzone that lies within the allocated object's boundaries.
--- lib/test_kasan.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
lib/test_kasan.c +6 -6
@@ -771,17 +771,17 @@ static void kasan_bitops_tags(struct kunit *test)/* This test is specifically crafted for the tag-based mode. */ if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"skipping, CONFIG_KASAN_SW_TAGS required" ); +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" );return ; } -/* Allocation size will be rounded to up granule size, which is 16. */ - bits =kzalloc (sizeof (*bits),GFP_KERNEL ); +/* kmalloc-64 cache will be used and the last 16 bytes will be the redzone. */ + bits =kzalloc (48 ,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, bits); -/* Do the accesses past the 16 allocated bytes. */ -kasan_bitops_modify (test,BITS_PER_LONG , &bits[1 ]); -kasan_bitops_test_and_modify (test,BITS_PER_LONG +BITS_PER_BYTE , &bits[1 ]); +/* Do the accesses past the 48 allocated bytes, but within the redone. */ +kasan_bitops_modify (test,BITS_PER_LONG , (void *)bits +48 ); +kasan_bitops_test_and_modify (test,BITS_PER_LONG +BITS_PER_BYTE , (void *)bits +48 );kfree (bits); }
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Alexander Potapenko
reviewerRe: [PATCH 09/11] kasan: fix memory corruption in kasan_bitops_tags test
Quoted 21 lines · Andrey Konovalov
Since the hardware tag-based KASAN mode might not have a redzone that comes after an allocated object (when kasan.mode=prod is enabled), the
kasan_bitops_tags()test ends up corrupting the next object in memory.Change the test so it always accesses the redzone that lies within the allocated object's boundaries.
Signed-off-by Andrey Konovalov <[email protected]>--- lib/test_kasan.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)lib/test_kasan.c +0
-0@@ -771,17 +771,17 @@ static void kasan_bitops_tags(struct kunit *test)/* This test is specifically crafted for the tag-based mode. */
lib/test_kasan.c static void kasan_bitops_tags(struct kunit *test) · line 779
Quoted 10 lines
lib/test_kasan.c +3
-3if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"skipping, CONFIG_KASAN_SW_TAGS required" ); +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" );return ; } -/* Allocation size will be rounded to up granule size, which is 16. */ - bits =kzalloc (sizeof (*bits),GFP_KERNEL ); +/* kmalloc-64 cache will be used and the last 16 bytes will be the redzone. */ + bits =kzalloc (48 ,GFP_KERNEL );
I think it might make sense to call ksize() here to ensure we have these spare bytes.
Re: [PATCH 09/11] kasan: fix memory corruption in kasan_bitops_tags test
Quoted 29 lines · Alexander Potapenko
Quoted 27 lines · Andrey Konovalov
Since the hardware tag-based KASAN mode might not have a redzone that comes after an allocated object (when kasan.mode=prod is enabled), the
kasan_bitops_tags()test ends up corrupting the next object in memory.Change the test so it always accesses the redzone that lies within the allocated object's boundaries.
Signed-off-by Andrey Konovalov <[email protected]>--- lib/test_kasan.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)lib/test_kasan.c +1
-1@@ -771,17 +771,17 @@ static void kasan_bitops_tags(struct kunit *test)/* This test is specifically crafted for the tag-based mode. */ if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"skipping, CONFIG_KASAN_SW_TAGS required" ); +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" );return ; }
Quoted 7 lines
lib/test_kasan.c static void kasan_bitops_tags(struct kunit *test) · line 779
Quoted 4 lines
lib/test_kasan.c +2
-2-/* Allocation size will be rounded to up granule size, which is 16. */ - bits =kzalloc (sizeof (*bits),GFP_KERNEL ); +/* kmalloc-64 cache will be used and the last 16 bytes will be the redzone. */ + bits =kzalloc (48 ,GFP_KERNEL );I think it might make sense to call
ksize()here to ensure we have these spare bytes.
Calling ksize() will unpoison the whole object.
I think it's OK to make assumptions about KASAN internals in tests. I would actually say that we need more tests that check such internal properties.
Thanks!
Re: [PATCH 09/11] kasan: fix memory corruption in kasan_bitops_tags test
Quoted 9 lines · Andrey Konovalov' via kasan-dev
Quoted 7 lines · Alexander Potapenko
Quoted 5 lines · Andrey Konovalov
Since the hardware tag-based KASAN mode might not have a redzone that comes after an allocated object (when kasan.mode=prod is enabled), the
kasan_bitops_tags()test ends up corrupting the next object in memory.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Change the test so it always accesses the redzone that lies within the allocated object's boundaries.
Quoted 25 lines
Quoted 17 lines
Quoted 17 lines
--- lib/test_kasan.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)lib/test_kasan.c +1
-1@@ -771,17 +771,17 @@ static void kasan_bitops_tags(struct kunit *test)/* This test is specifically crafted for the tag-based mode. */ if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"skipping, CONFIG_KASAN_SW_TAGS required" ); +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" );return ; }Quoted 7 lines
lib/test_kasan.c static void kasan_bitops_tags(struct kunit *test) · line 779
Quoted 4 lines
lib/test_kasan.c +2
-2-/* Allocation size will be rounded to up granule size, which is 16. */ - bits =kzalloc (sizeof (*bits),GFP_KERNEL ); +/* kmalloc-64 cache will be used and the last 16 bytes will be the redzone. */ + bits =kzalloc (48 ,GFP_KERNEL );I think it might make sense to call
ksize()here to ensure we have these spare bytes.
Quoted 1 line
Calling
ksize()will unpoison the whole object.
Ah, that's right.
Quoted 3 lines
I think it's OK to make assumptions about KASAN internals in tests. I would actually say that we need more tests that check such internal properties.
Agreed.
Re: [PATCH 09/11] kasan: fix memory corruption in kasan_bitops_tags test
In reply to Andrey KonovalovQuoted 9 lines · Andrey Konovalov
Since the hardware tag-based KASAN mode might not have a redzone that comes after an allocated object (when kasan.mode=prod is enabled), the
kasan_bitops_tags()test ends up corrupting the next object in memory.Change the test so it always accesses the redzone that lies within the allocated object's boundaries.
Quoted 35 lines
--- lib/test_kasan.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)lib/test_kasan.c +6
-6@@ -771,17 +771,17 @@ static void kasan_bitops_tags(struct kunit *test)/* This test is specifically crafted for the tag-based mode. */ if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { -kunit_info (test,"skipping, CONFIG_KASAN_SW_TAGS required" ); +kunit_info (test,"skipping, CONFIG_KASAN_SW/HW_TAGS required" );return ; } -/* Allocation size will be rounded to up granule size, which is 16. */ - bits =kzalloc (sizeof (*bits),GFP_KERNEL ); +/* kmalloc-64 cache will be used and the last 16 bytes will be the redzone. */ + bits =kzalloc (48 ,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, bits); -/* Do the accesses past the 16 allocated bytes. */ -kasan_bitops_modify (test,BITS_PER_LONG , &bits[1 ]); -kasan_bitops_test_and_modify (test,BITS_PER_LONG +BITS_PER_BYTE , &bits[1 ]); +/* Do the accesses past the 48 allocated bytes, but within the redone. */ +kasan_bitops_modify (test,BITS_PER_LONG , (void *)bits +48 ); +kasan_bitops_test_and_modify (test,BITS_PER_LONG +BITS_PER_BYTE , (void *)bits +48 );kfree (bits); }Signature · 1 line
2.29.2.729.g45daf8777d-goog
PATCH 10/11 kasan: fix bug detection via ksize for HW_TAGS mode
In reply to Andrey KonovalovThe currently existing kasan_check_read/write() annotations are intended to be used for kernel modules that have KASAN compiler instrumentation disabled. Thus, they are only relevant for the software KASAN modes that rely on compiler instrumentation.
However there's another use case for these annotations: ksize() checks that the object passed to it is indeed accessible before unpoisoning the whole object. This is currently done via __kasan_check_read(), which is compiled away for the hardware tag-based mode that doesn't rely on compiler instrumentation. This leads to KASAN missing detecting some memory corruptions.
Provide another annotation called kasan_check_byte() that is available for all KASAN modes. As the implementation rename and reuse kasan_check_invalid_free(). Use this new annotation in ksize().
Also add a new ksize_uaf() test that checks that a use-after-free is detected via ksize() itself, and via plain accesses that happen later.
--- include/linux/kasan-checks.h | 6 ++++++ include/linux/kasan.h | 13 +++++++++++++ lib/test_kasan.c | 20 ++++++++++++++++++++ mm/kasan/common.c | 11 ++++++++++- mm/kasan/generic.c | 4 ++-- mm/kasan/kasan.h | 10 +++++----- mm/kasan/sw_tags.c | 6 +++--- mm/slab_common.c | 15 +++++++++------ 8 files changed, 68 insertions(+), 17 deletions(-)
include/linux/kasan-checks.h +6 -0
@@ -4,6 +4,12 @@#include <linux/types.h> +/* +* The annotations present in this file are only relevant for the software +* KASAN modes that rely on compiler instrumentation, and will be optimized +* away for the hardware tag-based KASAN mode. Use kasan_check_byte() instead. +*/ +/* * __kasan_check_*: Always available when KASAN is enabled. This may be used * even in compilation units that selectively disable KASAN, but must use KASAN
include/linux/kasan.h +13 -0
@@ -243,6 +243,18 @@ static __always_inline void kasan_kfree_large(void *ptr, unsigned long ip)__kasan_kfree_large (ptr, ip); } +/* +* Unlike kasan_check_read/write(), kasan_check_byte() is performed even for +* the hardware tag-based mode that doesn't rely on compiler instrumentation. +*/ +bool __kasan_check_byte (const void *addr,unsigned long ip); +static __always_inline bool kasan_check_byte(const void *addr, unsigned long ip) +{ +if (kasan_enabled ()) +return __kasan_check_byte (addr, ip); +return true; +} +bool kasan_save_enable_multi_shot (void );void kasan_restore_multi_shot (bool enabled);
@@ -299,6 +311,7 @@ static inline void *kasan_krealloc(const void *object, size_t new_size, return (void *)object; }static inline void kasan_kfree_large (void *ptr,unsigned long ip) {} +static inline bool kasan_check_byte (const void *address,unsigned long ip) {}#endif /* CONFIG_KASAN */
lib/test_kasan.c +20 -0
@@ -490,6 +490,7 @@ static void kasan_global_oob(struct kunit *test)KUNIT_EXPECT_KASAN_FAIL (test, *(volatile char *)p); } +/* Check that ksize() makes the whole object accessible. */ static void ksize_unpoisons_memory (struct kunit *test) {char *ptr;
@@ -508,6 +509,24 @@ static void ksize_unpoisons_memory(struct kunit *test)kfree (ptr); } +/* +* Check that a use-after-free is detected by ksize() and via normal accesses +* after it. +*/ +static void ksize_uaf (struct kunit *test) +{ +char *ptr; +int size =128 -KASAN_GRANULE_SIZE ; + + ptr =kmalloc (size,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +kfree (ptr); + +KUNIT_EXPECT_KASAN_FAIL (test,ksize (ptr)); +KUNIT_EXPECT_KASAN_FAIL (test, kasan_int_result = *ptr); +KUNIT_EXPECT_KASAN_FAIL (test, kasan_int_result = *(ptr + size)); +} +static void kasan_stack_oob (struct kunit *test) {char stack_array[10 ];
@@ -937,6 +956,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kasan_alloca_oob_left),KUNIT_CASE (kasan_alloca_oob_right),KUNIT_CASE (ksize_unpoisons_memory), +KUNIT_CASE (ksize_uaf),KUNIT_CASE (kmem_cache_double_free),KUNIT_CASE (kmem_cache_invalid_free), KUNIT_CASE(kasan_memchr),
mm/kasan/common.c +10 -1
@@ -345,7 +345,7 @@ static bool ____kasan_slab_free(struct kmem_cache *cache, void *object,if (unlikely (cache->flags &SLAB_TYPESAFE_BY_RCU ))return false; -if (kasan_check_invalid_free (tagged_object)) { +if (!kasan_check (tagged_object)) {kasan_report_invalid_free (tagged_object, ip);return true; }
@@ -490,3 +490,12 @@ void __kasan_kfree_large(void *ptr, unsigned long ip)kasan_report_invalid_free (ptr, ip);/* The object will be poisoned by kasan_free_pages(). */ } + +bool __kasan_check_byte (const void *address,unsigned long ip) +{ +if (!kasan_check (address)) { +kasan_report_invalid_free ((void *)address, ip); +return false; + } +return true; +}
mm/kasan/generic.c +2 -2
@@ -185,11 +185,11 @@ bool kasan_check_range(unsigned long addr, size_t size, bool write,return check_region_inline (addr, size, write, ret_ip); } -bool kasan_check_invalid_free (void *addr) +bool kasan_check (const void *addr) {s8 shadow_byte =READ_ONCE (*(s8 *)kasan_mem_to_shadow (addr)); -return shadow_byte <0 || shadow_byte >=KASAN_GRANULE_SIZE ; +return shadow_byte >=0 && shadow_byte <KASAN_GRANULE_SIZE ; }void kasan_cache_shrink (struct kmem_cache *cache)
mm/kasan/kasan.h +5 -5
@@ -329,20 +329,20 @@ static inline void kasan_unpoison(const void *address, size_t size) round_up(size, KASAN_GRANULE_SIZE), get_tag(address)); } -static inline bool kasan_check_invalid_free (void *addr) +static inline bool kasan_check (const void *addr) {u8 ptr_tag =get_tag (addr); -u8 mem_tag =hw_get_mem_tag (addr); +u8 mem_tag =hw_get_mem_tag ((void *)addr); -return (mem_tag ==KASAN_TAG_INVALID ) || - (ptr_tag !=KASAN_TAG_KERNEL && ptr_tag != mem_tag); +return (mem_tag !=KASAN_TAG_INVALID ) && + (ptr_tag ==KASAN_TAG_KERNEL || ptr_tag == mem_tag); }#else /* CONFIG_KASAN_HW_TAGS */ void kasan_poison (const void *address,size_t size,u8 value);void kasan_unpoison (const void *address,size_t size); -bool kasan_check_invalid_free (void *addr); +bool kasan_check (const void *addr);#endif /* CONFIG_KASAN_HW_TAGS */
mm/kasan/sw_tags.c +3 -3
@@ -118,13 +118,13 @@ bool kasan_check_range(unsigned long addr, size_t size, bool write,return true; } -bool kasan_check_invalid_free (void *addr) +bool kasan_check (const void *addr) {u8 tag =get_tag (addr);u8 shadow_byte =READ_ONCE (*(u8 *)kasan_mem_to_shadow (kasan_reset_tag (addr))); -return (shadow_byte ==KASAN_TAG_INVALID ) || - (tag !=KASAN_TAG_KERNEL && tag != shadow_byte); +return (shadow_byte !=KASAN_TAG_INVALID ) && + (tag ==KASAN_TAG_KERNEL || tag == shadow_byte); }#define DEFINE_HWASAN_LOAD_STORE (size) \
mm/slab_common.c +9 -6
@@ -1157,11 +1157,13 @@ size_t ksize(const void *objp)size_t size;/* -* 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). +* We need to first check that the pointer to the object is valid, and +* only then unpoison the memory. The report printed from ksize() is +* more useful, then when it's printed later when the behaviour could +* be undefined due to a potential use-after-free or double-free. +* +* We use kasan_check_byte(), which is supported for hardware tag-based +* KASAN mode, unlike kasan_check_read/write(). * * If the pointed to memory is invalid we return 0, to avoid users of * ksize() writing to and potentially corrupting the memory region.
@@ -1169,7 +1171,8 @@ size_t ksize(const void *objp)* We want to perform the check before __ksize(), to avoid potentially * crashing in __ksize() due to accessing invalid metadata. */ -if (unlikely (ZERO_OR_NULL_PTR (objp)) || !__kasan_check_read (objp,1 )) +if (unlikely (ZERO_OR_NULL_PTR (objp)) || + !kasan_check_byte (objp, _RET_IP_))return 0 ; size =__ksize (objp);
Signature · 1 line
2.29.2.729.g45daf8777d-goog
kernel test robot
build botRe: [PATCH 10/11] kasan: fix bug detection via ksize for HW_TAGS mode
I love your patch! Yet something to improve:
Hi Andrey,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.11-rc2]
[cannot apply to arm64/for-next/core hnaz-linux-mm/master next-20210104] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Andrey-Konovalov/kasan-HW_TAGS-tests-support-and-fixes/20210106-022940 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62 config: x86_64-randconfig-a006-20210105 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/15d82adbf82e57e44789e091da9e141ba4247dba
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrey-Konovalov/kasan-HW_TAGS-tests-support-and-fixes/20210106-022940 git checkout 15d82adbf82e57e44789e091da9e141ba4247dba
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
In file included from arch/x86/kernel/asm-offsets.c:9:
In file included from include/linux/crypto.h:20:
In file included from include/linux/slab.h:136:
>> include/linux/kasan.h:314:77: error: non-void function does not return a value [-Werror,-Wreturn-type]
static inline bool kasan_check_byte(const void *address, unsigned long ip) {}
^ 1 error generated.
--
In file included from arch/x86/kernel/asm-offsets.c:9:
In file included from include/linux/crypto.h:20:
In file included from include/linux/slab.h:136:
>> include/linux/kasan.h:314:77: error: non-void function does not return a value [-Werror,-Wreturn-type]
static inline bool kasan_check_byte(const void *address, unsigned long ip) {}
^ 1 error generated.
make[2]: *** [scripts/Makefile.build:117: arch/x86/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1206: prepare0] Error 2
make[1]: Target 'modules_prepare' not remade because of errors.
make: *** [Makefile:185: __sub-make] Error 2
make: Target 'modules_prepare' not remade because of errors.
--
In file included from arch/x86/kernel/asm-offsets.c:9:
In file included from include/linux/crypto.h:20:
In file included from include/linux/slab.h:136:
>> include/linux/kasan.h:314:77: error: non-void function does not return a value [-Werror,-Wreturn-type]
static inline bool kasan_check_byte(const void *address, unsigned long ip) {}
^ 1 error generated.
make[2]: *** [scripts/Makefile.build:117: arch/x86/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1206: prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:185: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.
vim +314 include/linux/kasan.h
262
263 static inline bool kasan_enabled(void)
264 {
265 return false;
266 }
267 static inline slab_flags_t kasan_never_merge(void)
268 {
269 return 0;
270 }
271 static inline void kasan_unpoison_range(const void *address, size_t size) {}
272 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
273 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
274 static inline void kasan_cache_create(struct kmem_cache *cache,
275 unsigned int *size,
276 slab_flags_t *flags) {}
277 static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
278 static inline void kasan_poison_slab(struct page *page) {}
279 static inline void kasan_unpoison_object_data(struct kmem_cache *cache,
280 void *object) {}
281 static inline void kasan_poison_object_data(struct kmem_cache *cache,
282 void *object) {}
283 static inline void *kasan_init_slab_obj(struct kmem_cache *cache,
284 const void *object)
285 {
286 return (void *)object;
287 }
288 static inline bool kasan_slab_free(struct kmem_cache *s, void *object,
289 unsigned long ip)
290 {
291 return false;
292 }
293 static inline void kasan_slab_free_mempool(void *ptr, unsigned long ip) {}
294 static inline void *kasan_slab_alloc(struct kmem_cache *s, void *object,
295 gfp_t flags)
296 {
297 return object;
298 }
299 static inline void *kasan_kmalloc(struct kmem_cache *s, const void *object,
300 size_t size, gfp_t flags)
301 {
302 return (void *)object;
303 }
304 static inline void *kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags)
305 {
306 return (void *)ptr;
307 }
308 static inline void *kasan_krealloc(const void *object, size_t new_size,
309 gfp_t flags)
310 {
311 return (void *)object;
312 }
313 static inline void kasan_kfree_large(void *ptr, unsigned long ip) {}
> 314 static inline bool kasan_check_byte(const void *address, unsigned long ip) {}315
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
kernel test robot
build botRe: [PATCH 10/11] kasan: fix bug detection via ksize for HW_TAGS mode
In reply to Andrey KonovalovI love your patch! Perhaps something to improve:
Hi Andrey,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.11-rc2]
[cannot apply to arm64/for-next/core hnaz-linux-mm/master next-20210104] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Andrey-Konovalov/kasan-HW_TAGS-tests-support-and-fixes/20210106-022940 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62 config: x86_64-randconfig-a003-20210105 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/15d82adbf82e57e44789e091da9e141ba4247dba
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrey-Konovalov/kasan-HW_TAGS-tests-support-and-fixes/20210106-022940 git checkout 15d82adbf82e57e44789e091da9e141ba4247dba
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
In file included from arch/x86/boot/compressed/cmdline.c:2:
In file included from arch/x86/boot/compressed/misc.h:30:
In file included from include/linux/acpi.h:14:
In file included from include/linux/resource_ext.h:11:
In file included from include/linux/slab.h:136:
>> include/linux/kasan.h:314:77: warning: non-void function does not return a value [-Wreturn-type]
static inline bool kasan_check_byte(const void *address, unsigned long ip) {}
^1 warning generated.
vim +314 include/linux/kasan.h
262
263 static inline bool kasan_enabled(void)
264 {
265 return false;
266 }
267 static inline slab_flags_t kasan_never_merge(void)
268 {
269 return 0;
270 }
271 static inline void kasan_unpoison_range(const void *address, size_t size) {}
272 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
273 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
274 static inline void kasan_cache_create(struct kmem_cache *cache,
275 unsigned int *size,
276 slab_flags_t *flags) {}
277 static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
278 static inline void kasan_poison_slab(struct page *page) {}
279 static inline void kasan_unpoison_object_data(struct kmem_cache *cache,
280 void *object) {}
281 static inline void kasan_poison_object_data(struct kmem_cache *cache,
282 void *object) {}
283 static inline void *kasan_init_slab_obj(struct kmem_cache *cache,
284 const void *object)
285 {
286 return (void *)object;
287 }
288 static inline bool kasan_slab_free(struct kmem_cache *s, void *object,
289 unsigned long ip)
290 {
291 return false;
292 }
293 static inline void kasan_slab_free_mempool(void *ptr, unsigned long ip) {}
294 static inline void *kasan_slab_alloc(struct kmem_cache *s, void *object,
295 gfp_t flags)
296 {
297 return object;
298 }
299 static inline void *kasan_kmalloc(struct kmem_cache *s, const void *object,
300 size_t size, gfp_t flags)
301 {
302 return (void *)object;
303 }
304 static inline void *kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags)
305 {
306 return (void *)ptr;
307 }
308 static inline void *kasan_krealloc(const void *object, size_t new_size,
309 gfp_t flags)
310 {
311 return (void *)object;
312 }
313 static inline void kasan_kfree_large(void *ptr, unsigned long ip) {}
> 314 static inline bool kasan_check_byte(const void *address, unsigned long ip) {}315
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
Andrew Morton
maintainerRe: [PATCH 10/11] kasan: fix bug detection via ksize for HW_TAGS mode
Quoted 11 lines · On Wed, 6 Jan 2021 08:09:00 +0800 kernel test robot <[email protected]> wrote:
Hi Andrey,
In file included fromarch/x86/boot/compressed/cmdline.c:2:
In file included fromarch/x86/boot/compressed/misc.h:30:
In file included frominclude/linux/acpi.h:14:
In file included frominclude/linux/resource_ext.h:11:
In file included frominclude/linux/slab.h:136:Quoted 1 line
Quoted 1 line
include/linux/kasan.h:314:77: warning: non-void function does not return a value [-Wreturn-type]static inline bool
kasan_check_byte(const void *address, unsigned long ip){}^1 warning generated.
This?
include/linux/kasan.h renamed from include/linux/kasan.h~kasan-fix-bug-detection-via-ksize-for-hw_tags-mode-fix +4 -1
@@ -311,7 +311,10 @@ static inline void *kasan_krealloc(const return (void *)object; }static inline void kasan_kfree_large (void *ptr,unsigned long ip) {} -static inline bool kasan_check_byte (const void *address,unsigned long ip) {} +static inline bool kasan_check_byte (const void *address,unsigned long ip) +{ +return true; +}#endif /* CONFIG_KASAN */
btw, "kasan_check_byte" isn't a good function name. Check for what? Does it return true for a check which passed, or for a check which failed? Something like "kasan_byte_valid" would be better - the name explains the return value.
Re: [PATCH 10/11] kasan: fix bug detection via ksize for HW_TAGS mode
Quoted 20 lines · Andrew Morton
Quoted 11 lines · On Wed, 6 Jan 2021 08:09:00 +0800 kernel test robot <[email protected]> wrote:
Hi Andrey,
In file included fromarch/x86/boot/compressed/cmdline.c:2:
In file included fromarch/x86/boot/compressed/misc.h:30:
In file included frominclude/linux/acpi.h:14:
In file included frominclude/linux/resource_ext.h:11:
In file included frominclude/linux/slab.h:136:Quoted 1 line
Quoted 1 line
include/linux/kasan.h:314:77: warning: non-void function does not return a value [-Wreturn-type]static inline bool
kasan_check_byte(const void *address, unsigned long ip){}^1 warning generated.
This?
include/linux/kasan.h static inline void *kasan_krealloc(const · line 319
Quoted 10 lines
include/linux/kasan.h renamed from include/linux/kasan.h~kasan-fix-bug-detection-via-ksize-for-hw_tags-mode-fix +4
-1@@ -311,7 +311,10 @@ static inline void *kasan_krealloc(const return (void *)object; }static inline void kasan_kfree_large (void *ptr,unsigned long ip) {} -static inline bool kasan_check_byte (const void *address,unsigned long ip) {} +static inline bool kasan_check_byte (const void *address,unsigned long ip) +{ +return true; +}#endif /* CONFIG_KASAN */
Yes.
Quoted 4 lines
btw, "
kasan_check_byte" isn't a good function name. Check for what? Does it return true for a check which passed, or for a check which failed? Something like "kasan_byte_valid" would be better - the name explains the return value.
Sounds good, will fix in v2 along with the warning.
Thank you!
Re: [PATCH 10/11] kasan: fix bug detection via ksize for HW_TAGS mode
In reply to Andrey KonovalovQuoted 54 lines · Andrey Konovalov
The currently existing kasan_check_read/write() annotations are intended to be used for kernel modules that have KASAN compiler instrumentation disabled. Thus, they are only relevant for the software KASAN modes that rely on compiler instrumentation.
However there's another use case for these annotations:
ksize()checks that the object passed to it is indeed accessible before unpoisoning the whole object. This is currently done via__kasan_check_read(), which is compiled away for the hardware tag-based mode that doesn't rely on compiler instrumentation. This leads to KASAN missing detecting some memory corruptions.Provide another annotation called
kasan_check_byte()that is available for all KASAN modes. As the implementation rename and reusekasan_check_invalid_free(). Use this new annotation inksize().Also add a new
ksize_uaf()test that checks that a use-after-free is detected viaksize()itself, and via plain accesses that happen later.Signed-off-by Andrey Konovalov <[email protected]>--- include/linux/kasan-checks.h | 6 ++++++ include/linux/kasan.h | 13 +++++++++++++ lib/test_kasan.c | 20 ++++++++++++++++++++ mm/kasan/common.c | 11 ++++++++++- mm/kasan/generic.c | 4 ++-- mm/kasan/kasan.h | 10 +++++----- mm/kasan/sw_tags.c | 6 +++--- mm/slab_common.c | 15 +++++++++------ 8 files changed, 68 insertions(+), 17 deletions(-)include/linux/kasan-checks.h +6
-0@@ -4,6 +4,12 @@#include <linux/types.h> +/* +* The annotations present in this file are only relevant for the software +* KASAN modes that rely on compiler instrumentation, and will be optimized +* away for the hardware tag-based KASAN mode. Use kasan_check_byte() instead. +*/ +/* * __kasan_check_*: Always available when KASAN is enabled. This may be used * even in compilation units that selectively disable KASAN, but must use KASAN include/linux/kasan.h +0
-0
include/linux/kasan.h static __always_inline void kasan_kfree_large(void *ptr, unsigned long ip) · line 249
Quoted 7 lines
include/linux/kasan.h +4
-0@@ -243,6 +243,18 @@ static __always_inline void kasan_kfree_large(void *ptr, unsigned long ip)__kasan_kfree_large (ptr, ip); } +/* +* Unlike kasan_check_read/write(), kasan_check_byte() is performed even for +* the hardware tag-based mode that doesn't rely on compiler instrumentation. +*/
We have too many check-functions, and the name needs to be more precise. Intuitively, I would have thought this should have access-type, i.e. read or write, effectively mirroring a normal access.
Would kasan_check_byte_read() be better (and just not have a 'write' variant because we do not need it)? This would restore ksize() closest to what it was before (assuming reporting behaviour is fixed, too).
Quoted 78 lines
include/linux/kasan.h +9
-0+bool __kasan_check_byte (const void *addr,unsigned long ip); +static __always_inline bool kasan_check_byte(const void *addr, unsigned long ip) +{ +if (kasan_enabled ()) +return __kasan_check_byte (addr, ip); +return true; +} +bool kasan_save_enable_multi_shot (void );void kasan_restore_multi_shot (bool enabled);@@ -299,6 +311,7 @@ static inline void *kasan_krealloc(const void *object, size_t new_size, return (void *)object; }static inline void kasan_kfree_large (void *ptr,unsigned long ip) {} +static inline bool kasan_check_byte (const void *address,unsigned long ip) {}#endif /* CONFIG_KASAN */ lib/test_kasan.c +20
-0@@ -490,6 +490,7 @@ static void kasan_global_oob(struct kunit *test)KUNIT_EXPECT_KASAN_FAIL (test, *(volatile char *)p); } +/* Check that ksize() makes the whole object accessible. */ static void ksize_unpoisons_memory (struct kunit *test) {char *ptr;@@ -508,6 +509,24 @@ static void ksize_unpoisons_memory(struct kunit *test)kfree (ptr); } +/* +* Check that a use-after-free is detected by ksize() and via normal accesses +* after it. +*/ +static void ksize_uaf (struct kunit *test) +{ +char *ptr; +int size =128 -KASAN_GRANULE_SIZE ; + + ptr =kmalloc (size,GFP_KERNEL ); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +kfree (ptr); + +KUNIT_EXPECT_KASAN_FAIL (test,ksize (ptr)); +KUNIT_EXPECT_KASAN_FAIL (test, kasan_int_result = *ptr); +KUNIT_EXPECT_KASAN_FAIL (test, kasan_int_result = *(ptr + size)); +} +static void kasan_stack_oob (struct kunit *test) {char stack_array[10 ];@@ -937,6 +956,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kasan_alloca_oob_left),KUNIT_CASE (kasan_alloca_oob_right),KUNIT_CASE (ksize_unpoisons_memory), +KUNIT_CASE (ksize_uaf),KUNIT_CASE (kmem_cache_double_free),KUNIT_CASE (kmem_cache_invalid_free), KUNIT_CASE(kasan_memchr),mm/kasan/common.c +1
-1@@ -345,7 +345,7 @@ static bool ____kasan_slab_free(struct kmem_cache *cache, void *object,if (unlikely (cache->flags &SLAB_TYPESAFE_BY_RCU ))return false; -if (kasan_check_invalid_free (tagged_object)) { +if (!kasan_check (tagged_object)) {kasan_report_invalid_free (tagged_object, ip);return true; }
mm/kasan/common.c void __kasan_kfree_large(void *ptr, unsigned long ip) · line 497
Quoted 8 lines
mm/kasan/common.c +5
-0@@ -490,3 +490,12 @@ void __kasan_kfree_large(void *ptr, unsigned long ip)kasan_report_invalid_free (ptr, ip);/* The object will be poisoned by kasan_free_pages(). */ } + +bool __kasan_check_byte (const void *address,unsigned long ip) +{ +if (!kasan_check (address)) { +kasan_report_invalid_free ((void *)address, ip);
This is strange: why does it report an invalid free? Should this be a use-after-free? I think this could just call kasan_report(....) for 1 byte, and we'd get the right report.
Quoted 39 lines
mm/kasan/common.c +4
-0+return false; + } +return true; +}mm/kasan/generic.c +2
-2@@ -185,11 +185,11 @@ bool kasan_check_range(unsigned long addr, size_t size, bool write,return check_region_inline (addr, size, write, ret_ip); } -bool kasan_check_invalid_free (void *addr) +bool kasan_check (const void *addr) {s8 shadow_byte =READ_ONCE (*(s8 *)kasan_mem_to_shadow (addr)); -return shadow_byte <0 || shadow_byte >=KASAN_GRANULE_SIZE ; +return shadow_byte >=0 && shadow_byte <KASAN_GRANULE_SIZE ; }void kasan_cache_shrink (struct kmem_cache *cache)mm/kasan/kasan.h +2
-4@@ -329,20 +329,20 @@ static inline void kasan_unpoison(const void *address, size_t size) round_up(size, KASAN_GRANULE_SIZE), get_tag(address)); } -static inline bool kasan_check_invalid_free (void *addr) +static inline bool kasan_check (const void *addr) {u8 ptr_tag =get_tag (addr); -u8 mem_tag =hw_get_mem_tag (addr); +u8 mem_tag =hw_get_mem_tag ((void *)addr); -return (mem_tag ==KASAN_TAG_INVALID ) || - (ptr_tag !=KASAN_TAG_KERNEL && ptr_tag != mem_tag);
mm/kasan/kasan.h static inline void kasan_unpoison(const void *address, size_t size) · line 345
Quoted 10 lines
mm/kasan/kasan.h +3
-1+return (mem_tag !=KASAN_TAG_INVALID ) && + (ptr_tag == KASAN_TAG_KERNEL || ptr_tag == mem_tag); }#else /* CONFIG_KASAN_HW_TAGS */ void kasan_poison (const void *address,size_t size,u8 value);void kasan_unpoison (const void *address,size_t size); -bool kasan_check_invalid_free (void *addr); +bool kasan_check (const void *addr);
Definitely prefer shorted names, but we're in the unfortunate situation of having numerous kasan_check-functions, so we probably need to be more precise.
kasan_check() makes me think this also does reporting, but it does not (it seems to only check the metadata for validity).
The internal function could therefore be kasan_check_allocated() (it's now the inverse of kasan_check_invalid_free()).
Quoted 60 lines
mm/kasan/kasan.h +0
-0#endif /* CONFIG_KASAN_HW_TAGS */ mm/kasan/sw_tags.c +3
-3@@ -118,13 +118,13 @@ bool kasan_check_range(unsigned long addr, size_t size, bool write,return true; } -bool kasan_check_invalid_free (void *addr) +bool kasan_check (const void *addr) {u8 tag =get_tag (addr);u8 shadow_byte =READ_ONCE (*(u8 *)kasan_mem_to_shadow (kasan_reset_tag (addr))); -return (shadow_byte ==KASAN_TAG_INVALID ) || - (tag !=KASAN_TAG_KERNEL && tag != shadow_byte); +return (shadow_byte !=KASAN_TAG_INVALID ) && + (tag ==KASAN_TAG_KERNEL || tag == shadow_byte); }#define DEFINE_HWASAN_LOAD_STORE (size) \mm/slab_common.c +9
-6@@ -1157,11 +1157,13 @@ size_t ksize(const void *objp)size_t size;/* -* 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). +* We need to first check that the pointer to the object is valid, and +* only then unpoison the memory. The report printed from ksize() is +* more useful, then when it's printed later when the behaviour could +* be undefined due to a potential use-after-free or double-free. +* +* We use kasan_check_byte(), which is supported for hardware tag-based +* KASAN mode, unlike kasan_check_read/write(). * * If the pointed to memory is invalid we return 0, to avoid users of * ksize() writing to and potentially corrupting the memory region. @@ -1169,7 +1171,8 @@ size_t ksize(const void *objp)* We want to perform the check before __ksize(), to avoid potentially * crashing in __ksize() due to accessing invalid metadata. */ -if (unlikely (ZERO_OR_NULL_PTR (objp)) || !__kasan_check_read (objp,1 )) +if (unlikely (ZERO_OR_NULL_PTR (objp)) || + !kasan_check_byte (objp, _RET_IP_))return 0 ; size =__ksize (objp);Signature · 1 line
2.29.2.729.g45daf8777d-goog
Re: [PATCH 10/11] kasan: fix bug detection via ksize for HW_TAGS mode
Quoted 23 lines · Marco Elver
Quoted 5 lines
+/*
+ * Unlike kasan_check_read/write(),kasan_check_byte()is performed even for
+ * the hardware tag-based mode that doesn't rely on compiler instrumentation.
+ */We have too many check-functions, and the name needs to be more precise. Intuitively, I would have thought this should have access-type, i.e. read or write, effectively mirroring a normal access.
Would
kasan_check_byte_read()be better (and just not have a 'write' variant because we do not need it)? This would restoreksize()closest to what it was before (assuming reporting behaviour is fixed, too).mm/kasan/kasan.h static inline void kasan_unpoison(const void *address, size_t size) · line 345
Quoted 4 lines
void
kasan_poison(const void *address, size_t size, u8 value); voidkasan_unpoison(const void *address, size_t size);mm/kasan/kasan.h +1
-1-bool kasan_check_invalid_free (void *addr); +bool kasan_check (const void *addr);Definitely prefer shorted names, but we're in the unfortunate situation of having numerous kasan_check-functions, so we probably need to be more precise.
Quoted 5 lines
kasan_check()makes me think this also does reporting, but it does not (it seems to only check the metadata for validity).The internal function could therefore be
kasan_check_allocated()(it's now the inverse ofkasan_check_invalid_free()).
Re: kasan_check_byte():
I think the _read suffix is only making the name longer. ksize() isn't checking that the memory is readable (or writable), it's checking that it's addressable. At least that's the intention of the annotation, so it makes sense to name it correspondingly despite the implementation.
Having all kasan_check_*() functions both checking and reporting makes sense, so let's keep the kasan_check_ prefix.
What isn't obvious from the name is that this function is present for every kasan mode. Maybe kasan_check_byte_always()? Although it also seems too long.
But I'm OK with keeping kasan_check_byte().
Re kasan_check():
Here we can use Andrew's suggestion about the name being related to what the function returns. And also drop the kasan_check_ prefix as this function only does the checking.
Let's use kasan_byte_accessible() instead of kasan_check().
Quoted 8 lines
mm/kasan/common.c void __kasan_kfree_large(void *ptr, unsigned long ip) · line 497
Quoted 4 lines
mm/kasan/common.c +4
-0+bool __kasan_check_byte (const void *address,unsigned long ip) +{ +if (!kasan_check (address)) { +kasan_report_invalid_free ((void *)address, ip);This is strange: why does it report an invalid free? Should this be a use-after-free? I think this could just call
kasan_report(....)for 1 byte, and we'd get the right report.
Will fix in v2.
Thanks!
Re: [PATCH 10/11] kasan: fix bug detection via ksize for HW_TAGS mode
Quoted 38 lines · Andrey Konovalov
Quoted 23 lines · Marco Elver
Quoted 5 lines
+/*
+ * Unlike kasan_check_read/write(),kasan_check_byte()is performed even for
+ * the hardware tag-based mode that doesn't rely on compiler instrumentation.
+ */We have too many check-functions, and the name needs to be more precise. Intuitively, I would have thought this should have access-type, i.e. read or write, effectively mirroring a normal access.
Would
kasan_check_byte_read()be better (and just not have a 'write' variant because we do not need it)? This would restoreksize()closest to what it was before (assuming reporting behaviour is fixed, too).mm/kasan/kasan.h static inline void kasan_unpoison(const void *address, size_t size) · line 345
Quoted 4 lines
void
kasan_poison(const void *address, size_t size, u8 value); voidkasan_unpoison(const void *address, size_t size);mm/kasan/kasan.h +1
-1-bool kasan_check_invalid_free (void *addr); +bool kasan_check (const void *addr);Definitely prefer shorted names, but we're in the unfortunate situation of having numerous kasan_check-functions, so we probably need to be more precise.
Quoted 5 lines
kasan_check()makes me think this also does reporting, but it does not (it seems to only check the metadata for validity).The internal function could therefore be
kasan_check_allocated()(it's now the inverse ofkasan_check_invalid_free()).Re:
kasan_check_byte():I think the
_readsuffix is only making the name longer.ksize()isn't checking that the memory is readable (or writable), it's checking that it's addressable. At least that's the intention of the annotation, so it makes sense to name it correspondingly despite the implementation.
Quoted 8 lines
Having all kasan_check_*() functions both checking and reporting makes sense, so let's keep the
kasan_check_prefix.What isn't obvious from the name is that this function is present for every kasan mode. Maybe
kasan_check_byte_always()? Although it also seems too long.But I'm OK with keeping
kasan_check_byte().
This is fine.
Quoted 7 lines
Re
kasan_check():Here we can use Andrew's suggestion about the name being related to what the function returns. And also drop the
kasan_check_prefix as this function only does the checking.Let's use
kasan_byte_accessible()instead ofkasan_check().
Sounds reasonable to me.
Thanks,
-- Marco
PATCH 11/11 kasan: add proper page allocator tests
In reply to Andrey KonovalovThe currently existing page allocator tests rely on kmalloc fallback with large sizes that is only present for SLUB. Add proper tests that use alloc/free_pages().
--- lib/test_kasan.c | 54 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 5 deletions(-)
lib/test_kasan.c +49 -5
@@ -128,6 +128,12 @@ static void kmalloc_node_oob_right(struct kunit *test)kfree (ptr); } +/* +* These kmalloc_pagealloc_* tests try allocating a memory chunk that doesn't +* fit into a slab cache and therefore is allocated via the page allocator +* fallback. Since this kind of fallback is only implemented for SLUB, these +* tests are limited to that allocator. +*/ static void kmalloc_pagealloc_oob_right (struct kunit *test) {char *ptr;
@@ -138,14 +144,11 @@ static void kmalloc_pagealloc_oob_right(struct kunit *test)return ; } -/* -* Allocate a chunk that does not fit into a SLUB cache to trigger -* the page allocator fallback. -*/ ptr =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr);KUNIT_EXPECT_KASAN_FAIL (test, ptr[size +OOB_TAG_OFF ] =0 ); +kfree (ptr); }
@@ -161,8 +164,8 @@ static void kmalloc_pagealloc_uaf(struct kunit *test) ptr =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); -kfree (ptr); +KUNIT_EXPECT_KASAN_FAIL (test, ptr[0 ] =0 ); }
@@ -182,6 +185,45 @@ static void kmalloc_pagealloc_invalid_free(struct kunit *test)KUNIT_EXPECT_KASAN_FAIL (test,kfree (ptr +1 )); } +static void pagealloc_oob_right (struct kunit *test) +{ +char *ptr; +struct page *pages; +size_t order =4 ; +size_t size = (1UL << (PAGE_SHIFT + order)); + +/* +* With generic KASAN page allocations have no redzones, thus +* out-of-bounds detection is not guaranteed. +* See https://bugzilla.kernel.org/show_bug.cgi?id=210503. +*/ +if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC enabled" ); +return ; + } + + pages =alloc_pages (GFP_KERNEL , order); + ptr =page_address (pages); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); + +KUNIT_EXPECT_KASAN_FAIL (test, ptr[size] =0 ); +free_pages ((unsigned long )ptr, order); +} + +static void pagealloc_uaf (struct kunit *test) +{ +char *ptr; +struct page *pages; +size_t order =4 ; + + pages =alloc_pages (GFP_KERNEL , order); + ptr =page_address (pages); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +free_pages ((unsigned long )ptr, order); + +KUNIT_EXPECT_KASAN_FAIL (test, ptr[0 ] =0 ); +} +static void kmalloc_large_oob_right (struct kunit *test) {char *ptr;
@@ -933,6 +975,8 @@ static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kmalloc_pagealloc_oob_right),KUNIT_CASE (kmalloc_pagealloc_uaf),KUNIT_CASE (kmalloc_pagealloc_invalid_free), +KUNIT_CASE (pagealloc_oob_right), +KUNIT_CASE (pagealloc_uaf),KUNIT_CASE (kmalloc_large_oob_right),KUNIT_CASE (kmalloc_oob_krealloc_more), KUNIT_CASE(kmalloc_oob_krealloc_less),
Signature · 1 line
2.29.2.729.g45daf8777d-goog
Re: [PATCH 11/11] kasan: add proper page allocator tests
Quoted 7 lines · Andrey Konovalov
The currently existing page allocator tests rely on kmalloc fallback with large sizes that is only present for SLUB. Add proper tests that use alloc/free_pages().
Re: [PATCH 11/11] kasan: add proper page allocator tests
In reply to Andrey KonovalovQuoted 6 lines · Andrey Konovalov
The currently existing page allocator tests rely on kmalloc fallback with large sizes that is only present for SLUB. Add proper tests that use alloc/free_pages().
Quoted 105 lines
--- lib/test_kasan.c | 54 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 5 deletions(-)lib/test_kasan.c +49
-5@@ -128,6 +128,12 @@ static void kmalloc_node_oob_right(struct kunit *test)kfree (ptr); } +/* +* These kmalloc_pagealloc_* tests try allocating a memory chunk that doesn't +* fit into a slab cache and therefore is allocated via the page allocator +* fallback. Since this kind of fallback is only implemented for SLUB, these +* tests are limited to that allocator. +*/ static void kmalloc_pagealloc_oob_right (struct kunit *test) {char *ptr;@@ -138,14 +144,11 @@ static void kmalloc_pagealloc_oob_right(struct kunit *test)return ; } -/* -* Allocate a chunk that does not fit into a SLUB cache to trigger -* the page allocator fallback. -*/ ptr =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr);KUNIT_EXPECT_KASAN_FAIL (test, ptr[size +OOB_TAG_OFF ] =0 ); +kfree (ptr); }@@ -161,8 +164,8 @@ static void kmalloc_pagealloc_uaf(struct kunit *test) ptr =kmalloc (size,GFP_KERNEL );KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); -kfree (ptr); +KUNIT_EXPECT_KASAN_FAIL (test, ptr[0 ] =0 ); }@@ -182,6 +185,45 @@ static void kmalloc_pagealloc_invalid_free(struct kunit *test)KUNIT_EXPECT_KASAN_FAIL (test,kfree (ptr +1 )); } +static void pagealloc_oob_right (struct kunit *test) +{ +char *ptr; +struct page *pages; +size_t order =4 ; +size_t size = (1UL << (PAGE_SHIFT + order)); + +/* +* With generic KASAN page allocations have no redzones, thus +* out-of-bounds detection is not guaranteed. +* See https://bugzilla.kernel.org/show_bug.cgi?id=210503. +*/ +if (IS_ENABLED (CONFIG_KASAN_GENERIC )) { +kunit_info (test,"skipping, CONFIG_KASAN_GENERIC enabled" ); +return ; + } + + pages =alloc_pages (GFP_KERNEL , order); + ptr =page_address (pages); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); + +KUNIT_EXPECT_KASAN_FAIL (test, ptr[size] =0 ); +free_pages ((unsigned long )ptr, order); +} + +static void pagealloc_uaf (struct kunit *test) +{ +char *ptr; +struct page *pages; +size_t order =4 ; + + pages =alloc_pages (GFP_KERNEL , order); + ptr =page_address (pages); +KUNIT_ASSERT_NOT_ERR_OR_NULL (test, ptr); +free_pages ((unsigned long )ptr, order); + +KUNIT_EXPECT_KASAN_FAIL (test, ptr[0 ] =0 ); +} +static void kmalloc_large_oob_right (struct kunit *test) {char *ptr;@@ -933,6 +975,8 @@ static struct kunit_case kasan_kunit_test_cases[] = {KUNIT_CASE (kmalloc_pagealloc_oob_right),KUNIT_CASE (kmalloc_pagealloc_uaf),KUNIT_CASE (kmalloc_pagealloc_invalid_free), +KUNIT_CASE (pagealloc_oob_right), +KUNIT_CASE (pagealloc_uaf),KUNIT_CASE (kmalloc_large_oob_right),KUNIT_CASE (kmalloc_oob_krealloc_more), KUNIT_CASE(kmalloc_oob_krealloc_less),Signature · 1 line
2.29.2.729.g45daf8777d-goog