kasan: hardware tag-based mode for production use on arm64

Move get_free_info() call into quarantine_put() to simplify the call site.

No functional changes.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Iab0f04e7ebf8d83247024b7190c67c3c34c7940f

---
 mm/kasan/common.c     | 2 +-
 mm/kasan/kasan.h      | 5 ++---
 mm/kasan/quarantine.c | 3 ++-
 3 files changed, 5 insertions(+), 5 deletions(-)

mm/kasan/common.c +1 -1

index 2bb0ef6da6bd..5712c66c11c1 100644
@@ -308,7 +308,7 @@ static bool __kasan_slab_free(struct kmem_cache *cache, void *object,
 
        kasan_set_free_info(cache, object, tag);
 
-       quarantine_put(get_free_info(cache, object), cache);
+       quarantine_put(cache, object);
 
        return IS_ENABLED(CONFIG_KASAN_GENERIC);
 }

mm/kasan/kasan.h +2 -3

index 6850308c798a..5c0116c70579 100644
@@ -214,12 +214,11 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
 
 #if defined(CONFIG_KASAN_GENERIC) && \
        (defined(CONFIG_SLAB) || defined(CONFIG_SLUB))
-void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache);
+void quarantine_put(struct kmem_cache *cache, void *object);
 void quarantine_reduce(void);
 void quarantine_remove_cache(struct kmem_cache *cache);
 #else
-static inline void quarantine_put(struct kasan_free_meta *info,
-                               struct kmem_cache *cache) { }
+static inline void quarantine_put(struct kmem_cache *cache, void *object) { }
 static inline void quarantine_reduce(void) { }
 static inline void quarantine_remove_cache(struct kmem_cache *cache) { }
 #endif

mm/kasan/quarantine.c +2 -2

index 580ff5610fc1..a0792f0d6d0f 100644
@@ -161,11 +161,12 @@ static void qlist_free_all(struct qlist_head *q, struct kmem_cache *cache)
        qlist_init(q);
 }
 
-void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache)
+void quarantine_put(struct kmem_cache *cache, void *object)
 {
        unsigned long flags;
        struct qlist_head *q;
        struct qlist_head temp = QLIST_INIT;
+       struct kasan_free_meta *info = get_free_info(cache, object);
 
        /*
         * Note: irq must be disabled until after we move the batch to the
--

2.29.0.rc1.297.gfa9743e501-goog

Rename get_alloc_info() and get_free_info() to kasan_get_alloc_meta() and kasan_get_free_meta() to better reflect what those do and avoid confusion with kasan_set_free_info().

No functional changes.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Ib6e4ba61c8b12112b403d3479a9799ac8fff8de1

---
 mm/kasan/common.c         | 16 ++++++++--------
 mm/kasan/generic.c        | 12 ++++++------
 mm/kasan/hw_tags.c        |  4 ++--
 mm/kasan/kasan.h          |  8 ++++----
 mm/kasan/quarantine.c     |  4 ++--
 mm/kasan/report.c         | 12 ++++++------
 mm/kasan/report_sw_tags.c |  2 +-
 mm/kasan/sw_tags.c        |  4 ++--
 8 files changed, 31 insertions(+), 31 deletions(-)

mm/kasan/common.c +8 -8

index 5712c66c11c1..8fd04415d8f4 100644
@@ -175,14 +175,14 @@ size_t kasan_metadata_size(struct kmem_cache *cache)
                sizeof(struct kasan_free_meta) : 0);
 }
 
-struct kasan_alloc_meta *get_alloc_info(struct kmem_cache *cache,
-                                       const void *object)
+struct kasan_alloc_meta *kasan_get_alloc_meta(struct kmem_cache *cache,
+                                             const void *object)
 {
        return (void *)reset_tag(object) + cache->kasan_info.alloc_meta_offset;
 }
 
-struct kasan_free_meta *get_free_info(struct kmem_cache *cache,
-                                     const void *object)
+struct kasan_free_meta *kasan_get_free_meta(struct kmem_cache *cache,
+                                           const void *object)
 {
        BUILD_BUG_ON(sizeof(struct kasan_free_meta) > 32);
        return (void *)reset_tag(object) + cache->kasan_info.free_meta_offset;

@@ -259,13 +259,13 @@ static u8 assign_tag(struct kmem_cache *cache, const void *object,
 void * __must_check kasan_init_slab_obj(struct kmem_cache *cache,
                                                const void *object)
 {
-       struct kasan_alloc_meta *alloc_info;
+       struct kasan_alloc_meta *alloc_meta;
 
        if (!(cache->flags & SLAB_KASAN))
                return (void *)object;
 
-       alloc_info = get_alloc_info(cache, object);
-       __memset(alloc_info, 0, sizeof(*alloc_info));
+       alloc_meta = kasan_get_alloc_meta(cache, object);
+       __memset(alloc_meta, 0, sizeof(*alloc_meta));
 
        if (IS_ENABLED(CONFIG_KASAN_SW_TAGS) || IS_ENABLED(CONFIG_KASAN_HW_TAGS))
                object = set_tag(object, assign_tag(cache, object, true, false));

@@ -345,7 +345,7 @@ static void *__kasan_kmalloc(struct kmem_cache *cache, const void *object,
                KASAN_KMALLOC_REDZONE);
 
        if (cache->flags & SLAB_KASAN)
-               kasan_set_track(&get_alloc_info(cache, object)->alloc_track, flags);
+               kasan_set_track(&kasan_get_alloc_meta(cache, object)->alloc_track, flags);
 
        return set_tag(object, tag);
 }

mm/kasan/generic.c +6 -6

index e1af3b6c53b8..de6b3f03a023 100644
@@ -331,7 +331,7 @@ void kasan_record_aux_stack(void *addr)
 {
        struct page *page = kasan_addr_to_page(addr);
        struct kmem_cache *cache;
-       struct kasan_alloc_meta *alloc_info;
+       struct kasan_alloc_meta *alloc_meta;
        void *object;
 
        if (!(page && PageSlab(page)))

@@ -339,13 +339,13 @@ void kasan_record_aux_stack(void *addr)
 
        cache = page->slab_cache;
        object = nearest_obj(cache, page, addr);
-       alloc_info = get_alloc_info(cache, object);
+       alloc_meta = kasan_get_alloc_meta(cache, object);
 
        /*
         * record the last two call_rcu() call stacks.
         */
-       alloc_info->aux_stack[1] = alloc_info->aux_stack[0];
-       alloc_info->aux_stack[0] = kasan_save_stack(GFP_NOWAIT);
+       alloc_meta->aux_stack[1] = alloc_meta->aux_stack[0];
+       alloc_meta->aux_stack[0] = kasan_save_stack(GFP_NOWAIT);
 }
 
 void kasan_set_free_info(struct kmem_cache *cache,

@@ -353,7 +353,7 @@ void kasan_set_free_info(struct kmem_cache *cache,
 {
        struct kasan_free_meta *free_meta;
 
-       free_meta = get_free_info(cache, object);
+       free_meta = kasan_get_free_meta(cache, object);
        kasan_set_track(&free_meta->free_track, GFP_NOWAIT);
 
        /*

@@ -367,5 +367,5 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
 {
        if (*(u8 *)kasan_mem_to_shadow(object) != KASAN_KMALLOC_FREETRACK)
                return NULL;
-       return &get_free_info(cache, object)->free_track;
+       return &kasan_get_free_meta(cache, object)->free_track;
 }

mm/kasan/hw_tags.c +2 -2

index 7f0568df2a93..2a38885014e3 100644
@@ -56,7 +56,7 @@ void kasan_set_free_info(struct kmem_cache *cache,
 {
        struct kasan_alloc_meta *alloc_meta;
 
-       alloc_meta = get_alloc_info(cache, object);
+       alloc_meta = kasan_get_alloc_meta(cache, object);
        kasan_set_track(&alloc_meta->free_track[0], GFP_NOWAIT);
 }
 

@@ -65,6 +65,6 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
 {
        struct kasan_alloc_meta *alloc_meta;
 
-       alloc_meta = get_alloc_info(cache, object);
+       alloc_meta = kasan_get_alloc_meta(cache, object);
        return &alloc_meta->free_track[0];
 }

mm/kasan/kasan.h +4 -4

index 5c0116c70579..456b264e5124 100644
@@ -148,10 +148,10 @@ struct kasan_free_meta {
 #endif
 };
 
-struct kasan_alloc_meta *get_alloc_info(struct kmem_cache *cache,
-                                       const void *object);
-struct kasan_free_meta *get_free_info(struct kmem_cache *cache,
-                                       const void *object);
+struct kasan_alloc_meta *kasan_get_alloc_meta(struct kmem_cache *cache,
+                                               const void *object);
+struct kasan_free_meta *kasan_get_free_meta(struct kmem_cache *cache,
+                                               const void *object);
 
 void kasan_poison_memory(const void *address, size_t size, u8 value);
 

mm/kasan/quarantine.c +2 -2

index a0792f0d6d0f..0da3d37e1589 100644
@@ -166,7 +166,7 @@ void quarantine_put(struct kmem_cache *cache, void *object)
        unsigned long flags;
        struct qlist_head *q;
        struct qlist_head temp = QLIST_INIT;
-       struct kasan_free_meta *info = get_free_info(cache, object);
+       struct kasan_free_meta *meta = kasan_get_free_meta(cache, object);
 
        /*
         * Note: irq must be disabled until after we move the batch to the

@@ -179,7 +179,7 @@ void quarantine_put(struct kmem_cache *cache, void *object)
        local_irq_save(flags);
 
        q = this_cpu_ptr(&cpu_quarantine);
-       qlist_put(q, &info->quarantine_link, cache->size);
+       qlist_put(q, &meta->quarantine_link, cache->size);
        if (unlikely(q->bytes > QUARANTINE_PERCPU_SIZE)) {
                qlist_move_all(q, &temp);
 

mm/kasan/report.c +6 -6

index f8817d5685a7..dee5350b459c 100644
@@ -162,12 +162,12 @@ static void describe_object_addr(struct kmem_cache *cache, void *object,
 static void describe_object(struct kmem_cache *cache, void *object,
                                const void *addr, u8 tag)
 {
-       struct kasan_alloc_meta *alloc_info = get_alloc_info(cache, object);
+       struct kasan_alloc_meta *alloc_meta = kasan_get_alloc_meta(cache, object);
 
        if (cache->flags & SLAB_KASAN) {
                struct kasan_track *free_track;
 
-               print_track(&alloc_info->alloc_track, "Allocated");
+               print_track(&alloc_meta->alloc_track, "Allocated");
                pr_err("\n");
                free_track = kasan_get_free_track(cache, object, tag);
                if (free_track) {

@@ -176,14 +176,14 @@ static void describe_object(struct kmem_cache *cache, void *object,
                }
 
 #ifdef CONFIG_KASAN_GENERIC
-               if (alloc_info->aux_stack[0]) {
+               if (alloc_meta->aux_stack[0]) {
                        pr_err("Last call_rcu():\n");
-                       print_stack(alloc_info->aux_stack[0]);
+                       print_stack(alloc_meta->aux_stack[0]);
                        pr_err("\n");
                }
-               if (alloc_info->aux_stack[1]) {
+               if (alloc_meta->aux_stack[1]) {
                        pr_err("Second to last call_rcu():\n");
-                       print_stack(alloc_info->aux_stack[1]);
+                       print_stack(alloc_meta->aux_stack[1]);
                        pr_err("\n");
                }
 #endif

mm/kasan/report_sw_tags.c +1 -1

index aebc44a29e83..317100fd95b9 100644
@@ -46,7 +46,7 @@ const char *get_bug_type(struct kasan_access_info *info)
        if (page && PageSlab(page)) {
                cache = page->slab_cache;
                object = nearest_obj(cache, page, (void *)addr);
-               alloc_meta = get_alloc_info(cache, object);
+               alloc_meta = kasan_get_alloc_meta(cache, object);
 
                for (i = 0; i < KASAN_NR_FREE_STACKS; i++)
                        if (alloc_meta->free_pointer_tag[i] == tag)

mm/kasan/sw_tags.c +2 -3

index ccc35a311179..c10863a45775 100644
@@ -172,7 +172,7 @@ void kasan_set_free_info(struct kmem_cache *cache,
        struct kasan_alloc_meta *alloc_meta;
        u8 idx = 0;
 
-       alloc_meta = get_alloc_info(cache, object);
+       alloc_meta = kasan_get_alloc_meta(cache, object);
 
 #ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
        idx = alloc_meta->free_track_idx;

@@ -189,7 +189,7 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
        struct kasan_alloc_meta *alloc_meta;
        int i = 0;
 
-       alloc_meta = get_alloc_info(cache, object);
+       alloc_meta = kasan_get_alloc_meta(cache, object);
 
 #ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
        for (i = 0; i < KASAN_NR_FREE_STACKS; i++) {
--

2.29.0.rc1.297.gfa9743e501-goog

Add set_alloc_info() helper and move kasan_set_track() into it. This will simplify the code for one of the upcoming changes.

No functional changes.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/I0316193cbb4ecc9b87b7c2eee0dd79f8ec908c1a

---
 mm/kasan/common.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

mm/kasan/common.c +6 -2

index 8fd04415d8f4..a880e5a547ed 100644
@@ -318,6 +318,11 @@ bool kasan_slab_free(struct kmem_cache *cache, void *object, unsigned long ip)
        return __kasan_slab_free(cache, object, ip, true);
 }
 
+static void set_alloc_info(struct kmem_cache *cache, void *object, gfp_t flags)
+{
+       kasan_set_track(&kasan_get_alloc_meta(cache, object)->alloc_track, flags);
+}
+
 static void *__kasan_kmalloc(struct kmem_cache *cache, const void *object,
                                size_t size, gfp_t flags, bool keep_tag)
 {

@@ -345,7 +350,7 @@ static void *__kasan_kmalloc(struct kmem_cache *cache, const void *object,
                KASAN_KMALLOC_REDZONE);
 
        if (cache->flags & SLAB_KASAN)
-               kasan_set_track(&kasan_get_alloc_meta(cache, object)->alloc_track, flags);
+               set_alloc_info(cache, (void *)object, flags);
 
        return set_tag(object, tag);
 }
--

2.29.0.rc1.297.gfa9743e501-goog

There's a config option CONFIG_KASAN_STACK that has to be enabled for KASAN to use stack instrumentation and perform validity checks for stack variables.

There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is enabled.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3

---
 arch/arm64/kernel/sleep.S        |  2 +-
 arch/x86/kernel/acpi/wakeup_64.S |  2 +-
 include/linux/kasan.h            | 10 ++++++----
 mm/kasan/common.c                |  2 ++
 4 files changed, 10 insertions(+), 6 deletions(-)

arch/arm64/kernel/sleep.S +1 -1

index ba40d57757d6..bdadfa56b40e 100644
@@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume)
         */
        bl      cpu_do_resume
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        mov     x0, sp
        bl      kasan_unpoison_task_stack_below
 #endif

arch/x86/kernel/acpi/wakeup_64.S +1 -1

index c8daa92f38dc..5d3a0b8fd379 100644
@@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel)
        movq    pt_regs_r14(%rax), %r14
        movq    pt_regs_r15(%rax), %r15
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        /*
         * The suspend path may have poisoned some areas deeper in the stack,
         * which we now need to unpoison.

include/linux/kasan.h +0 -4

index 3f3f541e5d5f..7be9fb9146ac 100644
@@ -68,8 +68,6 @@ static inline void kasan_disable_current(void) {}
 
 void kasan_unpoison_memory(const void *address, size_t size);
 
-void kasan_unpoison_task_stack(struct task_struct *task);
-
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
 

@@ -114,8 +112,6 @@ void kasan_restore_multi_shot(bool enabled);
 
 static inline void kasan_unpoison_memory(const void *address, size_t size) {}
 
-static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
-
 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
 

include/linux/kasan.h +5 -0

+void kasan_unpoison_task_stack(struct task_struct *task);
+#else
+static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
+#endif
+
 #ifdef CONFIG_KASAN_GENERIC
 
 void kasan_cache_shrink(struct kmem_cache *cache);

mm/kasan/common.c +0 -0

index a880e5a547ed..a3e67d49b893 100644

mm/kasan/common.c +1 -1

 static void __kasan_unpoison_stack(struct task_struct *task, const void *sp)
 {
        void *base = task_stack_page(task);

@@ -84,6 +85,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)
 
        kasan_unpoison_memory(base, watermark - base);
 }
+#endif /* CONFIG_KASAN_STACK */
 
 void kasan_alloc_pages(struct page *page, unsigned int order)
 {
--

2.29.0.rc1.297.gfa9743e501-goog

Quoted 26 lines · Andrey Konovalov

There's a config option CONFIG_KASAN_STACK that has to be enabled for KASAN to use stack instrumentation and perform validity checks for stack variables.

There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is enabled.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3

---
 arch/arm64/kernel/sleep.S        |  2 +-
 arch/x86/kernel/acpi/wakeup_64.S |  2 +-
 include/linux/kasan.h            | 10 ++++++----
 mm/kasan/common.c                |  2 ++
 4 files changed, 10 insertions(+), 6 deletions(-)

arch/arm64/kernel/sleep.S +0 -0

index ba40d57757d6..bdadfa56b40e 100644
@@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume)
         */
        bl      cpu_do_resume
 

arch/arm64/kernel/sleep.S SYM_FUNC_START(_cpu_resume) · line 136

Quoted 2 lines

arch/arm64/kernel/sleep.S +1 -1

-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK

Quoted 11 lines

arch/arm64/kernel/sleep.S +0 -0

        mov     x0, sp
        bl      kasan_unpoison_task_stack_below
 #endif

arch/x86/kernel/acpi/wakeup_64.S +0 -0

index c8daa92f38dc..5d3a0b8fd379 100644
@@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel)
        movq    pt_regs_r14(%rax), %r14
        movq    pt_regs_r15(%rax), %r15
 

arch/x86/kernel/acpi/wakeup_64.S SYM_FUNC_START(do_suspend_lowlevel) · line 115

Quoted 2 lines

arch/x86/kernel/acpi/wakeup_64.S +1 -1

-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK

Quoted 26 lines

arch/x86/kernel/acpi/wakeup_64.S +0 -0

        /*
         * The suspend path may have poisoned some areas deeper in the stack,
         * which we now need to unpoison.

include/linux/kasan.h +0 -4

index 3f3f541e5d5f..7be9fb9146ac 100644
@@ -68,8 +68,6 @@ static inline void kasan_disable_current(void) {}
 
 void kasan_unpoison_memory(const void *address, size_t size);
 
-void kasan_unpoison_task_stack(struct task_struct *task);
-
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
 

@@ -114,8 +112,6 @@ void kasan_restore_multi_shot(bool enabled);
 
 static inline void kasan_unpoison_memory(const void *address, size_t size) {}
 
-static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
-
 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
 

include/linux/kasan.h static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; } · line 166

Quoted 4 lines

include/linux/kasan.h +1 -0

@@ -167,6 +163,12 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
 
 #endif /* CONFIG_KASAN */
 
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK

&& defined(CONFIG_KASAN_STACK) for consistency

Quoted 13 lines

include/linux/kasan.h +5 -0

+void kasan_unpoison_task_stack(struct task_struct *task);
+#else
+static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
+#endif
+
 #ifdef CONFIG_KASAN_GENERIC
 
 void kasan_cache_shrink(struct kmem_cache *cache);

mm/kasan/common.c +0 -0

index a880e5a547ed..a3e67d49b893 100644

mm/kasan/common.c void kasan_disable_current(void) · line 61

Quoted 4 lines

mm/kasan/common.c +1 -0

@@ -58,6 +58,7 @@ void kasan_disable_current(void)
 }
 #endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
 
+#if CONFIG_KASAN_STACK

#ifdef CONFIG_ is the form used toughout the kernel code

Quoted 14 lines

mm/kasan/common.c +1 -1

 static void __kasan_unpoison_stack(struct task_struct *task, const void *sp)
 {
        void *base = task_stack_page(task);

@@ -84,6 +85,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)
 
        kasan_unpoison_memory(base, watermark - base);
 }
+#endif /* CONFIG_KASAN_STACK */
 
 void kasan_alloc_pages(struct page *page, unsigned int order)
 {
--

2.29.0.rc1.297.gfa9743e501-goog

Quoted 66 lines · Andrey Konovalov

There's a config option CONFIG_KASAN_STACK that has to be enabled for KASAN to use stack instrumentation and perform validity checks for stack variables.

There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is enabled.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3

---
 arch/arm64/kernel/sleep.S        |  2 +-
 arch/x86/kernel/acpi/wakeup_64.S |  2 +-
 include/linux/kasan.h            | 10 ++++++----
 mm/kasan/common.c                |  2 ++
 4 files changed, 10 insertions(+), 6 deletions(-)

arch/arm64/kernel/sleep.S +1 -1

index ba40d57757d6..bdadfa56b40e 100644
@@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume)
         */
        bl      cpu_do_resume
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        mov     x0, sp
        bl      kasan_unpoison_task_stack_below
 #endif

arch/x86/kernel/acpi/wakeup_64.S +1 -1

index c8daa92f38dc..5d3a0b8fd379 100644
@@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel)
        movq    pt_regs_r14(%rax), %r14
        movq    pt_regs_r15(%rax), %r15
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        /*
         * The suspend path may have poisoned some areas deeper in the stack,
         * which we now need to unpoison.

include/linux/kasan.h +0 -4

index 3f3f541e5d5f..7be9fb9146ac 100644
@@ -68,8 +68,6 @@ static inline void kasan_disable_current(void) {}
 
 void kasan_unpoison_memory(const void *address, size_t size);
 
-void kasan_unpoison_task_stack(struct task_struct *task);
-
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
 

@@ -114,8 +112,6 @@ void kasan_restore_multi_shot(bool enabled);
 
 static inline void kasan_unpoison_memory(const void *address, size_t size) {}
 
-static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
-
 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
 

include/linux/kasan.h static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; } · line 166

Quoted 5 lines

include/linux/kasan.h +1 -0

@@ -167,6 +163,12 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
 
 #endif /* CONFIG_KASAN */
 
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK

&& defined(CONFIG_KASAN_STACK) for consistency

Quoted 14 lines

include/linux/kasan.h +5 -0

+void kasan_unpoison_task_stack(struct task_struct *task);
+#else
+static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
+#endif
+
 #ifdef CONFIG_KASAN_GENERIC
 
 void kasan_cache_shrink(struct kmem_cache *cache);

mm/kasan/common.c +0 -0

index a880e5a547ed..a3e67d49b893 100644

mm/kasan/common.c void kasan_disable_current(void) · line 61

Quoted 4 lines

mm/kasan/common.c +1 -0

@@ -58,6 +58,7 @@ void kasan_disable_current(void)
 }
 #endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
 
+#if CONFIG_KASAN_STACK

#ifdef CONFIG_ is the form used toughout the kernel code

Quoted 14 lines

mm/kasan/common.c +1 -1

 static void __kasan_unpoison_stack(struct task_struct *task, const void *sp)
 {
        void *base = task_stack_page(task);

@@ -84,6 +85,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)
 
        kasan_unpoison_memory(base, watermark - base);
 }
+#endif /* CONFIG_KASAN_STACK */
 
 void kasan_alloc_pages(struct page *page, unsigned int order)
 {
--

2.29.0.rc1.297.gfa9743e501-goog

Quoted 72 lines · Dmitry Vyukov

Quoted 70 lines · Andrey Konovalov

There's a config option CONFIG_KASAN_STACK that has to be enabled for KASAN to use stack instrumentation and perform validity checks for stack variables.

There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is enabled.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3

---
 arch/arm64/kernel/sleep.S        |  2 +-
 arch/x86/kernel/acpi/wakeup_64.S |  2 +-
 include/linux/kasan.h            | 10 ++++++----
 mm/kasan/common.c                |  2 ++
 4 files changed, 10 insertions(+), 6 deletions(-)

arch/arm64/kernel/sleep.S +1 -1

index ba40d57757d6..bdadfa56b40e 100644
@@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume)
         */
        bl      cpu_do_resume
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        mov     x0, sp
        bl      kasan_unpoison_task_stack_below
 #endif

arch/x86/kernel/acpi/wakeup_64.S +1 -1

index c8daa92f38dc..5d3a0b8fd379 100644
@@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel)
        movq    pt_regs_r14(%rax), %r14
        movq    pt_regs_r15(%rax), %r15
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        /*
         * The suspend path may have poisoned some areas deeper in the stack,
         * which we now need to unpoison.

include/linux/kasan.h +0 -4

index 3f3f541e5d5f..7be9fb9146ac 100644
@@ -68,8 +68,6 @@ static inline void kasan_disable_current(void) {}
 
 void kasan_unpoison_memory(const void *address, size_t size);
 
-void kasan_unpoison_task_stack(struct task_struct *task);
-
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
 

@@ -114,8 +112,6 @@ void kasan_restore_multi_shot(bool enabled);
 
 static inline void kasan_unpoison_memory(const void *address, size_t size) {}
 
-static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
-
 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
 

@@ -167,6 +163,12 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
 
 #endif /* CONFIG_KASAN */
 

Quoted 3 lines

include/linux/kasan.h static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; } · line 166

Quoted 1 line

include/linux/kasan.h +1 -0

+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK

&& defined(CONFIG_KASAN_STACK) for consistency

CONFIG_KASAN_STACK is different from other KASAN configs. It's always defined, and its value is what controls whether stack instrumentation is enabled.

Quoted 74 lines · Andrey Konovalov' via kasan-dev

Quoted 72 lines · Dmitry Vyukov

Quoted 70 lines · Andrey Konovalov

There's a config option CONFIG_KASAN_STACK that has to be enabled for KASAN to use stack instrumentation and perform validity checks for stack variables.

There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is enabled.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3

---
 arch/arm64/kernel/sleep.S        |  2 +-
 arch/x86/kernel/acpi/wakeup_64.S |  2 +-
 include/linux/kasan.h            | 10 ++++++----
 mm/kasan/common.c                |  2 ++
 4 files changed, 10 insertions(+), 6 deletions(-)

arch/arm64/kernel/sleep.S +1 -1

index ba40d57757d6..bdadfa56b40e 100644
@@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume)
         */
        bl      cpu_do_resume
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        mov     x0, sp
        bl      kasan_unpoison_task_stack_below
 #endif

arch/x86/kernel/acpi/wakeup_64.S +1 -1

index c8daa92f38dc..5d3a0b8fd379 100644
@@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel)
        movq    pt_regs_r14(%rax), %r14
        movq    pt_regs_r15(%rax), %r15
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        /*
         * The suspend path may have poisoned some areas deeper in the stack,
         * which we now need to unpoison.

include/linux/kasan.h +0 -4

index 3f3f541e5d5f..7be9fb9146ac 100644
@@ -68,8 +68,6 @@ static inline void kasan_disable_current(void) {}
 
 void kasan_unpoison_memory(const void *address, size_t size);
 
-void kasan_unpoison_task_stack(struct task_struct *task);
-
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
 

@@ -114,8 +112,6 @@ void kasan_restore_multi_shot(bool enabled);
 
 static inline void kasan_unpoison_memory(const void *address, size_t size) {}
 
-static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
-
 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
 

@@ -167,6 +163,12 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
 
 #endif /* CONFIG_KASAN */
 

Quoted 7 lines

Quoted 3 lines

include/linux/kasan.h static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; } · line 166

Quoted 1 line

include/linux/kasan.h +1 -0

+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK

&& defined(CONFIG_KASAN_STACK) for consistency

CONFIG_KASAN_STACK is different from other KASAN configs. It's always defined, and its value is what controls whether stack instrumentation is enabled.

Not sure why we did this instead of the following, but okay.

config KASAN_STACK

Quoted 86 lines · Dmitry Vyukov

Quoted 73 lines · Andrey Konovalov' via kasan-dev

Quoted 72 lines · Dmitry Vyukov

Quoted 70 lines · Andrey Konovalov

There's a config option CONFIG_KASAN_STACK that has to be enabled for KASAN to use stack instrumentation and perform validity checks for stack variables.

There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is enabled.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3

---
 arch/arm64/kernel/sleep.S        |  2 +-
 arch/x86/kernel/acpi/wakeup_64.S |  2 +-
 include/linux/kasan.h            | 10 ++++++----
 mm/kasan/common.c                |  2 ++
 4 files changed, 10 insertions(+), 6 deletions(-)

arch/arm64/kernel/sleep.S +1 -1

index ba40d57757d6..bdadfa56b40e 100644
@@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume)
         */
        bl      cpu_do_resume
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        mov     x0, sp
        bl      kasan_unpoison_task_stack_below
 #endif

arch/x86/kernel/acpi/wakeup_64.S +1 -1

index c8daa92f38dc..5d3a0b8fd379 100644
@@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel)
        movq    pt_regs_r14(%rax), %r14
        movq    pt_regs_r15(%rax), %r15
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        /*
         * The suspend path may have poisoned some areas deeper in the stack,
         * which we now need to unpoison.

include/linux/kasan.h +0 -4

index 3f3f541e5d5f..7be9fb9146ac 100644
@@ -68,8 +68,6 @@ static inline void kasan_disable_current(void) {}
 
 void kasan_unpoison_memory(const void *address, size_t size);
 
-void kasan_unpoison_task_stack(struct task_struct *task);
-
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
 

@@ -114,8 +112,6 @@ void kasan_restore_multi_shot(bool enabled);
 
 static inline void kasan_unpoison_memory(const void *address, size_t size) {}
 
-static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
-
 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
 

@@ -167,6 +163,12 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
 
 #endif /* CONFIG_KASAN */
 

Quoted 7 lines

Quoted 3 lines

include/linux/kasan.h static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; } · line 166

Quoted 1 line

include/linux/kasan.h +1 -0

+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK

&& defined(CONFIG_KASAN_STACK) for consistency

CONFIG_KASAN_STACK is different from other KASAN configs. It's always defined, and its value is what controls whether stack instrumentation is enabled.

Not sure why we did this instead of the following, but okay.

config KASAN_STACK

Review

Quoted 6 lines

(quoted diff) +3 -3

-       int
-       default 1 if KASAN_STACK_ENABLE || CC_IS_GCC
-       default 0
+       bool
+       default y if KASAN_STACK_ENABLE || CC_IS_GCC
+       default n

I wondered the same, but then looking at scripts/Makefile.kasan I think it's because we directly pass it to the compiler:

    ...
    $(call cc-param,asan-stack=$(CONFIG_KASAN_STACK)) \
    ...

Quoted 94 lines · Marco Elver

Quoted 86 lines · Dmitry Vyukov

Quoted 73 lines · Andrey Konovalov' via kasan-dev

Quoted 72 lines · Dmitry Vyukov

Quoted 70 lines · Andrey Konovalov

There's a config option CONFIG_KASAN_STACK that has to be enabled for KASAN to use stack instrumentation and perform validity checks for stack variables.

There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is enabled.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3

---
 arch/arm64/kernel/sleep.S        |  2 +-
 arch/x86/kernel/acpi/wakeup_64.S |  2 +-
 include/linux/kasan.h            | 10 ++++++----
 mm/kasan/common.c                |  2 ++
 4 files changed, 10 insertions(+), 6 deletions(-)

arch/arm64/kernel/sleep.S +1 -1

index ba40d57757d6..bdadfa56b40e 100644
@@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume)
         */
        bl      cpu_do_resume
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        mov     x0, sp
        bl      kasan_unpoison_task_stack_below
 #endif

arch/x86/kernel/acpi/wakeup_64.S +1 -1

index c8daa92f38dc..5d3a0b8fd379 100644
@@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel)
        movq    pt_regs_r14(%rax), %r14
        movq    pt_regs_r15(%rax), %r15
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        /*
         * The suspend path may have poisoned some areas deeper in the stack,
         * which we now need to unpoison.

include/linux/kasan.h +0 -4

index 3f3f541e5d5f..7be9fb9146ac 100644
@@ -68,8 +68,6 @@ static inline void kasan_disable_current(void) {}
 
 void kasan_unpoison_memory(const void *address, size_t size);
 
-void kasan_unpoison_task_stack(struct task_struct *task);
-
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
 

@@ -114,8 +112,6 @@ void kasan_restore_multi_shot(bool enabled);
 
 static inline void kasan_unpoison_memory(const void *address, size_t size) {}
 
-static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
-
 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
 

@@ -167,6 +163,12 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
 
 #endif /* CONFIG_KASAN */
 

Quoted 7 lines

Quoted 3 lines

include/linux/kasan.h static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; } · line 166

Quoted 1 line

include/linux/kasan.h +1 -0

+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK

&& defined(CONFIG_KASAN_STACK) for consistency

CONFIG_KASAN_STACK is different from other KASAN configs. It's always defined, and its value is what controls whether stack instrumentation is enabled.

Not sure why we did this instead of the following, but okay.

config KASAN_STACK

Review

Quoted 6 lines

(quoted diff) +3 -3

-       int
-       default 1 if KASAN_STACK_ENABLE || CC_IS_GCC
-       default 0
+       bool
+       default y if KASAN_STACK_ENABLE || CC_IS_GCC
+       default n

Quoted 5 lines

I wondered the same, but then looking at scripts/Makefile.kasan I think it's because we directly pass it to the compiler:

    ...
    $(call cc-param,asan-stack=$(CONFIG_KASAN_STACK)) \
    ...

Try this instead:

      $(call cc-param,asan-stack=$(if $(CONFIG_KASAN_STACK),1,0)) \

Quoted 102 lines · Catalin Marinas

Quoted 99 lines · Marco Elver

Quoted 86 lines · Dmitry Vyukov

Quoted 73 lines · Andrey Konovalov' via kasan-dev

Quoted 72 lines · Dmitry Vyukov

Quoted 70 lines · Andrey Konovalov

There's a config option CONFIG_KASAN_STACK that has to be enabled for KASAN to use stack instrumentation and perform validity checks for stack variables.

There's no need to unpoison stack when CONFIG_KASAN_STACK is not enabled. Only call kasan_unpoison_task_stack[_below]() when CONFIG_KASAN_STACK is enabled.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/If8a891e9fe01ea543e00b576852685afec0887e3

---
 arch/arm64/kernel/sleep.S        |  2 +-
 arch/x86/kernel/acpi/wakeup_64.S |  2 +-
 include/linux/kasan.h            | 10 ++++++----
 mm/kasan/common.c                |  2 ++
 4 files changed, 10 insertions(+), 6 deletions(-)

arch/arm64/kernel/sleep.S +1 -1

index ba40d57757d6..bdadfa56b40e 100644
@@ -133,7 +133,7 @@ SYM_FUNC_START(_cpu_resume)
         */
        bl      cpu_do_resume
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        mov     x0, sp
        bl      kasan_unpoison_task_stack_below
 #endif

arch/x86/kernel/acpi/wakeup_64.S +1 -1

index c8daa92f38dc..5d3a0b8fd379 100644
@@ -112,7 +112,7 @@ SYM_FUNC_START(do_suspend_lowlevel)
        movq    pt_regs_r14(%rax), %r14
        movq    pt_regs_r15(%rax), %r15
 
-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK
        /*
         * The suspend path may have poisoned some areas deeper in the stack,
         * which we now need to unpoison.

include/linux/kasan.h +0 -4

index 3f3f541e5d5f..7be9fb9146ac 100644
@@ -68,8 +68,6 @@ static inline void kasan_disable_current(void) {}
 
 void kasan_unpoison_memory(const void *address, size_t size);
 
-void kasan_unpoison_task_stack(struct task_struct *task);
-
 void kasan_alloc_pages(struct page *page, unsigned int order);
 void kasan_free_pages(struct page *page, unsigned int order);
 

@@ -114,8 +112,6 @@ void kasan_restore_multi_shot(bool enabled);
 
 static inline void kasan_unpoison_memory(const void *address, size_t size) {}
 
-static inline void kasan_unpoison_task_stack(struct task_struct *task) {}
-
 static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
 static inline void kasan_free_pages(struct page *page, unsigned int order) {}
 

@@ -167,6 +163,12 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }
 
 #endif /* CONFIG_KASAN */
 

Quoted 7 lines

Quoted 3 lines

include/linux/kasan.h static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; } · line 166

Quoted 1 line

include/linux/kasan.h +1 -0

+#if defined(CONFIG_KASAN) && CONFIG_KASAN_STACK

&& defined(CONFIG_KASAN_STACK) for consistency

CONFIG_KASAN_STACK is different from other KASAN configs. It's always defined, and its value is what controls whether stack instrumentation is enabled.

Not sure why we did this instead of the following, but okay.

config KASAN_STACK

Review

Quoted 6 lines

(quoted diff) +3 -3

-       int
-       default 1 if KASAN_STACK_ENABLE || CC_IS_GCC
-       default 0
+       bool
+       default y if KASAN_STACK_ENABLE || CC_IS_GCC
+       default n

I wondered the same, but then looking at scripts/Makefile.kasan I think it's because we directly pass it to the compiler:

    ...
    $(call cc-param,asan-stack=$(CONFIG_KASAN_STACK)) \
    ...

Quoted 3 lines

Try this instead:

      $(call cc-param,asan-stack=$(if $(CONFIG_KASAN_STACK),1,0)) \

We could have just 1 config instead of 2 as well.
For gcc we could do no prompt and default value y, and for clang -- prompt and default value n. I think it should do what we need.

Quoted 18 lines

Quoted 18 lines

Quoted 17 lines

Review

Quoted 11 lines

Not sure why we did this instead of the following, but okay.

config KASAN_STACK

(quoted diff) +3 -3

-       int
-       default 1 if KASAN_STACK_ENABLE || CC_IS_GCC
-       default 0
+       bool
+       default y if KASAN_STACK_ENABLE || CC_IS_GCC
+       default n

I wondered the same, but then looking at scripts/Makefile.kasan I think it's because we directly pass it to the compiler:

    ...
    $(call cc-param,asan-stack=$(CONFIG_KASAN_STACK)) \
    ...

Quoted 9 lines

Quoted 3 lines

Try this instead:

      $(call cc-param,asan-stack=$(if $(CONFIG_KASAN_STACK),1,0)) \

We could have just 1 config instead of 2 as well.
For gcc we could do no prompt and default value y, and for clang -- prompt and default value n. I think it should do what we need.

I agree with Catalin's proposal since it should simplify things.

Nit: 'default n' is the default hence I do not think it should be required explicitly.

Fixing this sounds like a good idea, but perhaps not as a part of this series, to not overinflate it even further.

I've filed a bug for this: https://bugzilla.kernel.org/show_bug.cgi?id=210221

Even though hardware tag-based mode currently doesn't support checking vmalloc allocations, it doesn't use shadow memory and works with VMAP_STACK as is.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/I3552cbc12321dec82cd7372676e9372a2eb452ac

---
 arch/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/Kconfig +0 -0

index af14a567b493..3caf7bcdcf93 100644

arch/Kconfig +0 -1

        help
          Enable this if you want the use virtually-mapped kernel stacks
          with guard pages.  This causes kernel stack overflows to be
--

2.29.0.rc1.297.gfa9743e501-goog

Quoted 16 lines · Andrey Konovalov

Even though hardware tag-based mode currently doesn't support checking vmalloc allocations, it doesn't use shadow memory and works with VMAP_STACK as is.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/I3552cbc12321dec82cd7372676e9372a2eb452ac

---
 arch/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/Kconfig +0 -0

index af14a567b493..3caf7bcdcf93 100644

arch/Kconfig config VMAP_STACK · line 871

Quoted 5 lines

arch/Kconfig +1 -1

@@ -868,7 +868,7 @@ config VMAP_STACK
        default y
        bool "Use a virtually-mapped stack"
        depends on HAVE_ARCH_VMAP_STACK
-       depends on !KASAN || KASAN_VMALLOC
+       depends on !(KASAN_GENERIC || KASAN_SW_TAGS) || KASAN_VMALLOC

I find it a bit simpler to interpret:

    depends on !KASAN || KASAN_HW_TAGS || KASAN_VMALLOC

due to simpler structure. But maybe it's just me.

Similarly to kasan_init() mark kasan_init_tags() as __init.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/I8792e22f1ca5a703c5e979969147968a99312558

---
 include/linux/kasan.h | 2 +-
 mm/kasan/hw_tags.c    | 2 +-
 mm/kasan/sw_tags.c    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

include/linux/kasan.h +1 -1

index 7be9fb9146ac..93d9834b7122 100644
@@ -185,7 +185,7 @@ static inline void kasan_record_aux_stack(void *ptr) {}
 
 #if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
 
-void kasan_init_tags(void);
+void __init kasan_init_tags(void);
 
 void *kasan_reset_tag(const void *addr);
 

mm/kasan/hw_tags.c +1 -1

index 2a38885014e3..0128062320d5 100644
@@ -15,7 +15,7 @@
 
 #include "kasan.h"
 
-void kasan_init_tags(void)
+void __init kasan_init_tags(void)
 {
        init_tags(KASAN_TAG_MAX);
 }

mm/kasan/sw_tags.c +1 -2

index c10863a45775..bf1422282bb5 100644
@@ -35,7 +35,7 @@
 
 static DEFINE_PER_CPU(u32, prng_state);
 
-void kasan_init_tags(void)
+void __init kasan_init_tags(void)
 {
        int cpu;
 
--

2.29.0.rc1.297.gfa9743e501-goog

--
You received this message because you are subscribed to the Google Groups "kasan-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/155123c77b1a068089421022c4c5b1ccb75defd8.1603372719.git.andreyknvl%40google.com.

Quoted 3 lines · Andrey Konovalov' via kasan-dev

Similarly to kasan_init() mark kasan_init_tags() as __init.

Quoted 2 lines

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/I8792e22f1ca5a703c5e979969147968a99312558

Reviewed-by Dmitry Vyukov <[email protected]>

init_tags itself is not __init, but that's added in a different patch. I've commented on that patch.

Quoted 53 lines

---
 include/linux/kasan.h | 2 +-
 mm/kasan/hw_tags.c    | 2 +-
 mm/kasan/sw_tags.c    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

include/linux/kasan.h +1 -1

index 7be9fb9146ac..93d9834b7122 100644
@@ -185,7 +185,7 @@ static inline void kasan_record_aux_stack(void *ptr) {}
 
 #if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
 
-void kasan_init_tags(void);
+void __init kasan_init_tags(void);
 
 void *kasan_reset_tag(const void *addr);
 

mm/kasan/hw_tags.c +1 -1

index 2a38885014e3..0128062320d5 100644
@@ -15,7 +15,7 @@
 
 #include "kasan.h"
 
-void kasan_init_tags(void)
+void __init kasan_init_tags(void)
 {
        init_tags(KASAN_TAG_MAX);
 }

mm/kasan/sw_tags.c +1 -2

index c10863a45775..bf1422282bb5 100644
@@ -35,7 +35,7 @@
 
 static DEFINE_PER_CPU(u32, prng_state);
 
-void kasan_init_tags(void)
+void __init kasan_init_tags(void)
 {
        int cpu;
 
--

2.29.0.rc1.297.gfa9743e501-goog

--
You received this message because you are subscribed to the Google Groups "kasan-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/155123c77b1a068089421022c4c5b1ccb75defd8.1603372719.git.andreyknvl%40google.com.

Tag-based KASAN modes are fully initialized with kasan_init_tags(), while the generic mode only requireds kasan_init(). Move the initialization message for tag-based modes into kasan_init_tags().

Also fix pr_fmt() usage for KASAN code: generic mode doesn't need it,

tag-based modes should use "kasan:" instead of KBUILD_MODNAME.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Idfd1e50625ffdf42dfc3dbf7455b11bd200a0a49

---
 arch/arm64/mm/kasan_init.c | 3 +++
 mm/kasan/generic.c         | 2 --
 mm/kasan/hw_tags.c         | 4 ++++
 mm/kasan/sw_tags.c         | 4 +++-
 4 files changed, 10 insertions(+), 3 deletions(-)

arch/arm64/mm/kasan_init.c +0 -0

index b6b9d55bb72e..8f17fa834b62 100644

arch/arm64/mm/kasan_init.c +1 -0

        pr_info("KernelAddressSanitizer initialized\n");
+#endif
 }

mm/kasan/generic.c +0 -2

index de6b3f03a023..d259e4c3aefd 100644
@@ -9,8 +9,6 @@
  *        Andrey Konovalov <[email protected]>
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
 #include <linux/export.h>
 #include <linux/interrupt.h>
 #include <linux/init.h>

mm/kasan/hw_tags.c +4 -0

index 0128062320d5..b372421258c8 100644
@@ -6,6 +6,8 @@
  * Author: Andrey Konovalov <[email protected]>
  */
 
+#define pr_fmt(fmt) "kasan: " fmt
+
 #include <linux/kasan.h>
 #include <linux/kernel.h>
 #include <linux/memory.h>

@@ -18,6 +20,8 @@
 void __init kasan_init_tags(void)
 {
        init_tags(KASAN_TAG_MAX);
+
+       pr_info("KernelAddressSanitizer initialized\n");
 }
 
 void *kasan_reset_tag(const void *addr)

mm/kasan/sw_tags.c +3 -2

index bf1422282bb5..099af6dc8f7e 100644
@@ -6,7 +6,7 @@
  * Author: Andrey Konovalov <[email protected]>
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define pr_fmt(fmt) "kasan: " fmt
 
 #include <linux/export.h>
 #include <linux/interrupt.h>

@@ -41,6 +41,8 @@ void __init kasan_init_tags(void)
 
        for_each_possible_cpu(cpu)
                per_cpu(prng_state, cpu) = (u32)get_cycles();
+
+       pr_info("KernelAddressSanitizer initialized\n");
 }
 
 /*
--

2.29.0.rc1.297.gfa9743e501-goog

Quoted 6 lines · Andrey Konovalov

Tag-based KASAN modes are fully initialized with kasan_init_tags(), while the generic mode only requireds kasan_init(). Move the initialization message for tag-based modes into kasan_init_tags().

Also fix pr_fmt() usage for KASAN code: generic mode doesn't need it,

Why doesn't it need it? What's the difference with tag modes?

Quoted 2 lines

tag-based modes should use "kasan:" instead of KBUILD_MODNAME.

With generic KASAN I currently see:

[    0.571473][    T0] kasan: KernelAddressSanitizer initialized

So KBUILD_MODNAME somehow works. Is there some difference between files?

Quoted 10 lines

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Idfd1e50625ffdf42dfc3dbf7455b11bd200a0a49

---
 arch/arm64/mm/kasan_init.c | 3 +++
 mm/kasan/generic.c         | 2 --
 mm/kasan/hw_tags.c         | 4 ++++
 mm/kasan/sw_tags.c         | 4 +++-
 4 files changed, 10 insertions(+), 3 deletions(-)

arch/arm64/mm/kasan_init.c void __init kasan_init(void) · line 294

Quoted 10 lines

arch/arm64/mm/kasan_init.c +2 -0

index b6b9d55bb72e..8f17fa834b62 100644
@@ -290,5 +290,8 @@ void __init kasan_init(void)
 {
        kasan_init_shadow();
        kasan_init_depth();
+#if defined(CONFIG_KASAN_GENERIC)
+       /* CONFIG_KASAN_SW/HW_TAGS also requires kasan_init_tags(). */

A bit cleaner way may be to introduce kasan_init_early() and kasan_init_late(). Late() will do tag init and always print the message.

Quoted 10 lines · Dmitry Vyukov

Quoted 6 lines · Andrey Konovalov

Tag-based KASAN modes are fully initialized with kasan_init_tags(), while the generic mode only requireds kasan_init(). Move the initialization message for tag-based modes into kasan_init_tags().

Also fix pr_fmt() usage for KASAN code: generic mode doesn't need it,

Why doesn't it need it? What's the difference with tag modes?

I need to reword the patch descriptions: it's not the mode that doesn't need it, it's the generic.c file, as it doesn't use any pr_*() functions.

Quoted 8 lines

Quoted 2 lines

tag-based modes should use "kasan:" instead of KBUILD_MODNAME.

With generic KASAN I currently see:

[    0.571473][    T0] kasan: KernelAddressSanitizer initialized

So KBUILD_MODNAME somehow works. Is there some difference between files?

That code is printed from arch/xxx/mm/kasan_init*.c, which has its own pr_fmt defined.

Quoted 10 lines

Quoted 10 lines

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Idfd1e50625ffdf42dfc3dbf7455b11bd200a0a49

---
 arch/arm64/mm/kasan_init.c | 3 +++
 mm/kasan/generic.c         | 2 --
 mm/kasan/hw_tags.c         | 4 ++++
 mm/kasan/sw_tags.c         | 4 +++-
 4 files changed, 10 insertions(+), 3 deletions(-)

Quoted 14 lines

arch/arm64/mm/kasan_init.c void __init kasan_init(void) · line 294

Quoted 10 lines

arch/arm64/mm/kasan_init.c +2 -0

index b6b9d55bb72e..8f17fa834b62 100644
@@ -290,5 +290,8 @@ void __init kasan_init(void)
 {
        kasan_init_shadow();
        kasan_init_depth();
+#if defined(CONFIG_KASAN_GENERIC)
+       /* CONFIG_KASAN_SW/HW_TAGS also requires kasan_init_tags(). */

A bit cleaner way may be to introduce kasan_init_early() and kasan_init_late(). Late() will do tag init and always print the message.

It appears we'll also need kasan_init_even_later() for some MTE-related stuff. I'll try to figure out some sane naming scheme here and include it into the next version.

There's no need for __kasan_unpoison_stack() helper, as it's only currently used in a single place. Removing it also removes undeed arithmetic.

No functional changes.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Ie5ba549d445292fe629b4a96735e4034957bcc50

---
 mm/kasan/common.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

mm/kasan/common.c +3 -10

index a3e67d49b893..9008fc6b0810 100644
@@ -59,18 +59,12 @@ void kasan_disable_current(void)
 #endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
 
 #if CONFIG_KASAN_STACK
-static void __kasan_unpoison_stack(struct task_struct *task, const void *sp)
-{
-       void *base = task_stack_page(task);
-       size_t size = sp - base;
-
-       kasan_unpoison_memory(base, size);
-}
-
 /* Unpoison the entire stack for a task. */
 void kasan_unpoison_task_stack(struct task_struct *task)
 {
-       __kasan_unpoison_stack(task, task_stack_page(task) + THREAD_SIZE);
+       void *base = task_stack_page(task);
+
+       kasan_unpoison_memory(base, THREAD_SIZE);
 }
 
 /* Unpoison the stack for the current task beyond a watermark sp value. */
--

2.29.0.rc1.297.gfa9743e501-goog

Using kasan_reset_tag() currently results in a function call. As it's called quite often from the allocator code this leads to a noticeable slowdown. Move it to include/linux/kasan.h and turn it into a static inline function.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/I4d2061acfe91d480a75df00b07c22d8494ef14b5

---
 include/linux/kasan.h | 5 ++++-
 mm/kasan/hw_tags.c    | 5 -----
 mm/kasan/kasan.h      | 6 ++----
 mm/kasan/sw_tags.c    | 5 -----
 4 files changed, 6 insertions(+), 15 deletions(-)

include/linux/kasan.h +0 -0

index 93d9834b7122..6377d7d3a951 100644

include/linux/kasan.h +1 -0

+}
 
 bool kasan_report(unsigned long addr, size_t size,
                bool is_write, unsigned long ip);

mm/kasan/hw_tags.c +0 -5

index b372421258c8..c3a0e83b5e7a 100644
@@ -24,11 +24,6 @@ void __init kasan_init_tags(void)
        pr_info("KernelAddressSanitizer initialized\n");
 }
 
-void *kasan_reset_tag(const void *addr)
-{
-       return reset_tag(addr);
-}
-
 void kasan_poison_memory(const void *address, size_t size, u8 value)
 {
        set_mem_tag_range(reset_tag(address),

mm/kasan/kasan.h +0 -0

index 456b264e5124..0ccbb3c4c519 100644
@@ -246,15 +246,13 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
        return addr;
 }

mm/kasan/kasan.h +0 -1

 #define set_tag(addr, tag)     ((void *)arch_kasan_set_tag((addr), (tag)))
-#define reset_tag(addr)                ((void *)arch_kasan_reset_tag(addr))
 #define get_tag(addr)          arch_kasan_get_tag(addr)
 
 #ifndef arch_init_tags

mm/kasan/sw_tags.c +0 -6

index 099af6dc8f7e..4db41f274702 100644
@@ -67,11 +67,6 @@ u8 random_tag(void)
        return (u8)(state % (KASAN_TAG_MAX + 1));
 }
 
-void *kasan_reset_tag(const void *addr)
-{
-       return reset_tag(addr);
-}
-
 bool check_memory_region(unsigned long addr, size_t size, bool write,
                                unsigned long ret_ip)
 {
--

2.29.0.rc1.297.gfa9743e501-goog

Quoted 23 lines · Andrey Konovalov

Using kasan_reset_tag() currently results in a function call. As it's called quite often from the allocator code this leads to a noticeable slowdown. Move it to include/linux/kasan.h and turn it into a static inline function.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/I4d2061acfe91d480a75df00b07c22d8494ef14b5

---
 include/linux/kasan.h | 5 ++++-
 mm/kasan/hw_tags.c    | 5 -----
 mm/kasan/kasan.h      | 6 ++----
 mm/kasan/sw_tags.c    | 5 -----
 4 files changed, 6 insertions(+), 15 deletions(-)

include/linux/kasan.h +0 -0

index 93d9834b7122..6377d7d3a951 100644
@@ -187,7 +187,10 @@ static inline void kasan_record_aux_stack(void *ptr) {}
 
 void __init kasan_init_tags(void);
 

include/linux/kasan.h static inline void kasan_record_aux_stack(void *ptr) {} · line 192

Quoted 4 lines

include/linux/kasan.h +3 -1

-void *kasan_reset_tag(const void *addr);
+static inline void *kasan_reset_tag(const void *addr)
+{
+       return (void *)arch_kasan_reset_tag(addr);

It seems that all implementations already return (void *), so the cast is not needed.

Quoted 36 lines

include/linux/kasan.h +1 -0

+}
 
 bool kasan_report(unsigned long addr, size_t size,
                bool is_write, unsigned long ip);

mm/kasan/hw_tags.c +0 -5

index b372421258c8..c3a0e83b5e7a 100644
@@ -24,11 +24,6 @@ void __init kasan_init_tags(void)
        pr_info("KernelAddressSanitizer initialized\n");
 }
 
-void *kasan_reset_tag(const void *addr)
-{
-       return reset_tag(addr);
-}
-
 void kasan_poison_memory(const void *address, size_t size, u8 value)
 {
        set_mem_tag_range(reset_tag(address),

mm/kasan/kasan.h +0 -3

index 456b264e5124..0ccbb3c4c519 100644
@@ -246,15 +246,13 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
        return addr;
 }
 #endif
-#ifndef arch_kasan_reset_tag
-#define arch_kasan_reset_tag(addr)     ((void *)(addr))
-#endif
 #ifndef arch_kasan_get_tag
 #define arch_kasan_get_tag(addr)       0
 #endif
 

mm/kasan/kasan.h static inline const void *arch_kasan_set_tag(const void *addr, u8 tag) · line 254

Quoted 2 lines

mm/kasan/kasan.h +2 -0

+/* kasan_reset_tag() defined in include/linux/kasan.h. */
+#define reset_tag(addr)                ((void *)kasan_reset_tag(addr))

The cast is not needed.

I would also now remove reset_tag entirely by replacing it with kasan_reset_tag. Having 2 names for the same thing does not add clarity.

Using random_tag() currently results in a function call. Move its definition to mm/kasan/kasan.h and turn it into a static inline function for hardware tag-based mode to avoid uneeded function call.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Iac5b2faf9a912900e16cca6834d621f5d4abf427

---
 mm/kasan/hw_tags.c |  5 -----
 mm/kasan/kasan.h   | 37 ++++++++++++++++++++-----------------
 2 files changed, 20 insertions(+), 22 deletions(-)

mm/kasan/hw_tags.c +0 -5

index c3a0e83b5e7a..4c24bfcfeff9 100644
@@ -36,11 +36,6 @@ void kasan_unpoison_memory(const void *address, size_t size)
                          round_up(size, KASAN_GRANULE_SIZE), get_tag(address));
 }
 
-u8 random_tag(void)
-{
-       return get_random_tag();
-}
-
 bool check_invalid_free(void *addr)
 {
        u8 ptr_tag = get_tag(addr);

mm/kasan/kasan.h +6 -17

index 0ccbb3c4c519..94ba15c2f860 100644
@@ -188,6 +188,12 @@ 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);
+#else
+static inline void print_tags(u8 addr_tag, const void *addr) { }
+#endif
+
 bool check_invalid_free(void *addr);
 
 void *find_first_bad_addr(void *addr, size_t size);

@@ -223,23 +229,6 @@ static inline void quarantine_reduce(void) { }
 static inline void quarantine_remove_cache(struct kmem_cache *cache) { }
 #endif
 
-#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
-
-void print_tags(u8 addr_tag, const void *addr);
-
-u8 random_tag(void);
-
-#else
-
-static inline void print_tags(u8 addr_tag, const void *addr) { }
-
-static inline u8 random_tag(void)
-{
-       return 0;
-}
-
-#endif
-
 #ifndef arch_kasan_set_tag
 static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 {

mm/kasan/kasan.h +8 -1

+}
+#else
+static inline u8 random_tag(void)
+{
+       return 0;
+}
+#endif
+
 /*
  * Exported functions for interfaces called from assembly or from generated
  * code. Declarations here to avoid warning about missing declarations.
--

2.29.0.rc1.297.gfa9743e501-goog

Quoted 73 lines · Andrey Konovalov

Using random_tag() currently results in a function call. Move its definition to mm/kasan/kasan.h and turn it into a static inline function for hardware tag-based mode to avoid uneeded function call.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Iac5b2faf9a912900e16cca6834d621f5d4abf427

---
 mm/kasan/hw_tags.c |  5 -----
 mm/kasan/kasan.h   | 37 ++++++++++++++++++++-----------------
 2 files changed, 20 insertions(+), 22 deletions(-)

mm/kasan/hw_tags.c +0 -5

index c3a0e83b5e7a..4c24bfcfeff9 100644
@@ -36,11 +36,6 @@ void kasan_unpoison_memory(const void *address, size_t size)
                          round_up(size, KASAN_GRANULE_SIZE), get_tag(address));
 }
 
-u8 random_tag(void)
-{
-       return get_random_tag();
-}
-
 bool check_invalid_free(void *addr)
 {
        u8 ptr_tag = get_tag(addr);

mm/kasan/kasan.h +6 -17

index 0ccbb3c4c519..94ba15c2f860 100644
@@ -188,6 +188,12 @@ 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);
+#else
+static inline void print_tags(u8 addr_tag, const void *addr) { }
+#endif
+
 bool check_invalid_free(void *addr);
 
 void *find_first_bad_addr(void *addr, size_t size);

@@ -223,23 +229,6 @@ static inline void quarantine_reduce(void) { }
 static inline void quarantine_remove_cache(struct kmem_cache *cache) { }
 #endif
 
-#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
-
-void print_tags(u8 addr_tag, const void *addr);
-
-u8 random_tag(void);
-
-#else
-
-static inline void print_tags(u8 addr_tag, const void *addr) { }
-
-static inline u8 random_tag(void)
-{
-       return 0;
-}
-
-#endif
-
 #ifndef arch_kasan_set_tag
 static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 {

@@ -273,6 +262,20 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 #define get_mem_tag(addr)                      arch_get_mem_tag(addr)
 #define set_mem_tag_range(addr, size, tag)     arch_set_mem_tag_range((addr), (size), (tag))
 

mm/kasan/kasan.h static inline const void *arch_kasan_set_tag(const void *addr, u8 tag) · line 270

Quoted 6 lines

mm/kasan/kasan.h +6 -0

+#ifdef CONFIG_KASAN_SW_TAGS
+u8 random_tag(void);
+#elif defined(CONFIG_KASAN_HW_TAGS)
+static inline u8 random_tag(void)
+{
+       return get_random_tag();

What's the difference between random_tag() and get_random_tag()? Do we need both?

Quoted 75 lines · Dmitry Vyukov

Quoted 73 lines · Andrey Konovalov

Using random_tag() currently results in a function call. Move its definition to mm/kasan/kasan.h and turn it into a static inline function for hardware tag-based mode to avoid uneeded function call.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Iac5b2faf9a912900e16cca6834d621f5d4abf427

---
 mm/kasan/hw_tags.c |  5 -----
 mm/kasan/kasan.h   | 37 ++++++++++++++++++++-----------------
 2 files changed, 20 insertions(+), 22 deletions(-)

mm/kasan/hw_tags.c +0 -5

index c3a0e83b5e7a..4c24bfcfeff9 100644
@@ -36,11 +36,6 @@ void kasan_unpoison_memory(const void *address, size_t size)
                          round_up(size, KASAN_GRANULE_SIZE), get_tag(address));
 }
 
-u8 random_tag(void)
-{
-       return get_random_tag();
-}
-
 bool check_invalid_free(void *addr)
 {
        u8 ptr_tag = get_tag(addr);

mm/kasan/kasan.h +6 -17

index 0ccbb3c4c519..94ba15c2f860 100644
@@ -188,6 +188,12 @@ 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);
+#else
+static inline void print_tags(u8 addr_tag, const void *addr) { }
+#endif
+
 bool check_invalid_free(void *addr);
 
 void *find_first_bad_addr(void *addr, size_t size);

@@ -223,23 +229,6 @@ static inline void quarantine_reduce(void) { }
 static inline void quarantine_remove_cache(struct kmem_cache *cache) { }
 #endif
 
-#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
-
-void print_tags(u8 addr_tag, const void *addr);
-
-u8 random_tag(void);
-
-#else
-
-static inline void print_tags(u8 addr_tag, const void *addr) { }
-
-static inline u8 random_tag(void)
-{
-       return 0;
-}
-
-#endif
-
 #ifndef arch_kasan_set_tag
 static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 {

@@ -273,6 +262,20 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 #define get_mem_tag(addr)                      arch_get_mem_tag(addr)
 #define set_mem_tag_range(addr, size, tag)     arch_set_mem_tag_range((addr), (size), (tag))
 

Quoted 9 lines

mm/kasan/kasan.h static inline const void *arch_kasan_set_tag(const void *addr, u8 tag) · line 270

Quoted 6 lines

mm/kasan/kasan.h +6 -0

+#ifdef CONFIG_KASAN_SW_TAGS
+u8 random_tag(void);
+#elif defined(CONFIG_KASAN_HW_TAGS)
+static inline u8 random_tag(void)
+{
+       return get_random_tag();

What's the difference between random_tag() and get_random_tag()? Do we need both?

Not really. Will simplify this in the next version and give cleaner names.

Using kasan_poison_memory() or check_invalid_free() currently results in function calls. Move their definitions to mm/kasan/kasan.h and turn them into static inline functions for hardware tag-based mode to avoid uneeded function calls.

Signed-off-by Andrey Konovalov <[email protected]>

Link https://linux-review.googlesource.com/id/Ia9d8191024a12d1374675b3d27197f10193f50bb

---
 mm/kasan/hw_tags.c | 15 ---------------
 mm/kasan/kasan.h   | 28 ++++++++++++++++++++++++----
 2 files changed, 24 insertions(+), 19 deletions(-)

mm/kasan/hw_tags.c +0 -15

index 4c24bfcfeff9..f03161f3da19 100644
@@ -24,27 +24,12 @@ void __init kasan_init_tags(void)
        pr_info("KernelAddressSanitizer initialized\n");
 }
 
-void kasan_poison_memory(const void *address, size_t size, u8 value)
-{
-       set_mem_tag_range(reset_tag(address),
-                         round_up(size, KASAN_GRANULE_SIZE), value);
-}
-
 void kasan_unpoison_memory(const void *address, size_t size)
 {
        set_mem_tag_range(reset_tag(address),
                          round_up(size, KASAN_GRANULE_SIZE), get_tag(address));
 }
 
-bool check_invalid_free(void *addr)
-{
-       u8 ptr_tag = get_tag(addr);
-       u8 mem_tag = get_mem_tag(addr);
-
-       return (mem_tag == KASAN_TAG_INVALID) ||
-               (ptr_tag != KASAN_TAG_KERNEL && ptr_tag != mem_tag);
-}
-
 void kasan_set_free_info(struct kmem_cache *cache,
                                void *object, u8 tag)
 {

mm/kasan/kasan.h +24 -5

index 94ba15c2f860..8d84ae6f58f1 100644
@@ -153,8 +153,6 @@ struct kasan_alloc_meta *kasan_get_alloc_meta(struct kmem_cache *cache,
 struct kasan_free_meta *kasan_get_free_meta(struct kmem_cache *cache,
                                                const void *object);
 
-void kasan_poison_memory(const void *address, size_t size, u8 value);
-
 #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
 
 static inline const void *kasan_shadow_to_mem(const void *shadow_addr)

@@ -194,8 +192,6 @@ void print_tags(u8 addr_tag, const void *addr);
 static inline void print_tags(u8 addr_tag, const void *addr) { }
 #endif
 
-bool check_invalid_free(void *addr);
-
 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);

@@ -276,6 +272,30 @@ static inline u8 random_tag(void)
 }
 #endif
 
+#ifdef CONFIG_KASAN_HW_TAGS
+
+static inline void kasan_poison_memory(const void *address, size_t size, u8 value)
+{
+       set_mem_tag_range(reset_tag(address),
+                         round_up(size, KASAN_GRANULE_SIZE), value);
+}
+
+static inline bool check_invalid_free(void *addr)
+{
+       u8 ptr_tag = get_tag(addr);
+       u8 mem_tag = get_mem_tag(addr);
+
+       return (mem_tag == KASAN_TAG_INVALID) ||
+               (ptr_tag != KASAN_TAG_KERNEL && ptr_tag != mem_tag);
+}
+
+#else /* CONFIG_KASAN_HW_TAGS */
+
+void kasan_poison_memory(const void *address, size_t size, u8 value);
+bool check_invalid_free(void *addr);
+
+#endif /* CONFIG_KASAN_HW_TAGS */
+
 /*
  * Exported functions for interfaces called from assembly or from generated
  * code. Declarations here to avoid warning about missing declarations.
--

2.29.0.rc1.297.gfa9743e501-goog

--
You received this message because you are subscribed to the Google Groups "kasan-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/a3cd7d83cc1f9ca06ef6d8c84e70f122212bf8ef.1603372719.git.andreyknvl%40google.com.