Skip to contentSkip to conversation

kasan: common: cleanups and deduplication

PATCH In review Igor Putko 2 participants 7 messages Jul 4 – Jul 6

Subsystems (2)
lore
Share & tools
lore

Igor Putko

author
#0 lore raw

cover kasan: common: cleanups and deduplication

This patch series refactors KASAN common code to improve readability and maintainability.

Specifically, it:
- Introduces a helper for quarantine reduction to eliminate boilerplate.
- Utilizes the standard ALIGN_DOWN macro instead of manual bitwise logic.
- Replaces a magic number with the KASAN_TAG_KERNEL definition.

Igor Putko (3):
kasan: deduplicate quarantine reduction logic
kasan: common: use ALIGN_DOWN macro for stack alignment
kasan: common: replace magic number with KASAN_TAG_KERNEL

 mm/kasan/common.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
Signature · 1 line
2.47.3
author
#1 lore raw

PATCH 1/3 kasan: deduplicate quarantine reduction logic

Multiple allocation functions share the exact same conditional logic for reducing the KASAN quarantine size based on gfp flags. Introduce the kasan_quarantine_reduce_cond() helper to eliminate this boilerplate from the slab, kmalloc, kmalloc_large, and krealloc paths.

---
 mm/kasan/common.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)
mm/kasan/common.c +10 -8
index b7d05c2a6..34aa0fdce 100644
@@ -340,14 +340,19 @@ static inline void unpoison_slab_object(struct kmem_cache *cache, void *object,
 		kasan_save_alloc_info(cache, object, flags);
 }
 
+static inline void kasan_quarantine_reduce_cond(gfp_t flags)
+{
+	if (gfpflags_allow_blocking(flags))
+		kasan_quarantine_reduce();
+}
+
 void * __must_check __kasan_slab_alloc(struct kmem_cache *cache,
 					void *object, gfp_t flags, bool init)
 {
 	u8 tag;
 	void *tagged_object;
 
-	if (gfpflags_allow_blocking(flags))
-		kasan_quarantine_reduce();
+	kasan_quarantine_reduce_cond(flags);
 
 	if (unlikely(object == NULL))
 		return NULL;
@@ -402,8 +407,7 @@ static inline void poison_kmalloc_redzone(struct kmem_cache *cache,
 void * __must_check __kasan_kmalloc(struct kmem_cache *cache, const void *object,
 					size_t size, gfp_t flags)
 {
-	if (gfpflags_allow_blocking(flags))
-		kasan_quarantine_reduce();
+	kasan_quarantine_reduce_cond(flags);
 
 	if (unlikely(object == NULL))
 		return NULL;
@@ -443,8 +447,7 @@ static inline void poison_kmalloc_large_redzone(const void *ptr, size_t size,
 void * __must_check __kasan_kmalloc_large(const void *ptr, size_t size,
 						gfp_t flags)
 {
-	if (gfpflags_allow_blocking(flags))
-		kasan_quarantine_reduce();
+	kasan_quarantine_reduce_cond(flags);
 
 	if (unlikely(ptr == NULL))
 		return NULL;
@@ -460,8 +463,7 @@ void * __must_check __kasan_krealloc(const void *object, size_t size, gfp_t flag
 {
 	struct slab *slab;
 
-	if (gfpflags_allow_blocking(flags))
-		kasan_quarantine_reduce();
+	kasan_quarantine_reduce_cond(flags);
 
 	if (unlikely(object == ZERO_SIZE_PTR))
 		return (void *)object;
Signature · 1 line
2.47.3
#2 lore raw

Re: [PATCH 1/3] kasan: deduplicate quarantine reduction logic

Quoted 16 lines · Igor Putko

Multiple allocation functions share the exact same conditional logic for reducing the KASAN quarantine size based on gfp flags. Introduce the kasan_quarantine_reduce_cond() helper to eliminate this boilerplate from the slab, kmalloc, kmalloc_large, and krealloc paths.

---
 mm/kasan/common.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)
mm/kasan/common.c +0 -0
index b7d05c2a6..34aa0fdce 100644

mm/kasan/common.c static inline void unpoison_slab_object(struct kmem_cache *cache, void *object, · line 347

Quoted 8 lines
mm/kasan/common.c +5 -0
@@ -340,14 +340,19 @@ static inline void unpoison_slab_object(struct kmem_cache *cache, void *object,
                kasan_save_alloc_info(cache, object, flags);
 }
 
+static inline void kasan_quarantine_reduce_cond(gfp_t flags)
+{
+       if (gfpflags_allow_blocking(flags))
+               kasan_quarantine_reduce();
+}

Not sure about this change: it's hiding the condition for when kasan_quarantine_reduce() gets to execute and does not really improve the code readability otherwise.

Quoted 46 lines
mm/kasan/common.c +5 -9
+
 void * __must_check __kasan_slab_alloc(struct kmem_cache *cache,
                                        void *object, gfp_t flags, bool init)
 {
        u8 tag;
        void *tagged_object;
 
-       if (gfpflags_allow_blocking(flags))
-               kasan_quarantine_reduce();
+       kasan_quarantine_reduce_cond(flags);
 
        if (unlikely(object == NULL))
                return NULL;
@@ -402,8 +407,7 @@ static inline void poison_kmalloc_redzone(struct kmem_cache *cache,
 void * __must_check __kasan_kmalloc(struct kmem_cache *cache, const void *object,
                                        size_t size, gfp_t flags)
 {
-       if (gfpflags_allow_blocking(flags))
-               kasan_quarantine_reduce();
+       kasan_quarantine_reduce_cond(flags);
 
        if (unlikely(object == NULL))
                return NULL;
@@ -443,8 +447,7 @@ static inline void poison_kmalloc_large_redzone(const void *ptr, size_t size,
 void * __must_check __kasan_kmalloc_large(const void *ptr, size_t size,
                                                gfp_t flags)
 {
-       if (gfpflags_allow_blocking(flags))
-               kasan_quarantine_reduce();
+       kasan_quarantine_reduce_cond(flags);
 
        if (unlikely(ptr == NULL))
                return NULL;
@@ -460,8 +463,7 @@ void * __must_check __kasan_krealloc(const void *object, size_t size, gfp_t flag
 {
        struct slab *slab;
 
-       if (gfpflags_allow_blocking(flags))
-               kasan_quarantine_reduce();
+       kasan_quarantine_reduce_cond(flags);
 
        if (unlikely(object == ZERO_SIZE_PTR))
                return (void *)object;
--

2.47.3

author
#3 lore raw

PATCH 2/3 kasan: common: use ALIGN_DOWN macro for stack alignment

In reply to Igor Putko

Replace the manual bitwise alignment logic in kasan_unpoison_task_stack_below() with the standard ALIGN_DOWN macro. This improves code readability and conforms to standard kernel idioms.

---
 mm/kasan/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/kasan/common.c +1 -1
index 34aa0fdce..5faf73c8f 100644
@@ -119,7 +119,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)
 	 * because this function is called by early resume code which hasn't
 	 * yet set up the percpu register (%gs).
 	 */
-	void *base = (void *)((unsigned long)watermark & ~(THREAD_SIZE - 1));
+	void *base = (void *)ALIGN_DOWN((unsigned long)watermark, THREAD_SIZE);
 
 	kasan_unpoison(base, watermark - base, false);
 }
Signature · 1 line
2.47.3
reviewer Reviewed-by on v1 2/3
#4 lore raw

Re: [PATCH 2/3] kasan: common: use ALIGN_DOWN macro for stack alignment

Quoted 25 lines · Igor Putko

Replace the manual bitwise alignment logic in kasan_unpoison_task_stack_below() with the standard ALIGN_DOWN macro. This improves code readability and conforms to standard kernel idioms.

---
 mm/kasan/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/kasan/common.c +1 -2
index 34aa0fdce..5faf73c8f 100644
@@ -119,7 +119,7 @@ asmlinkage void kasan_unpoison_task_stack_below(const void *watermark)
         * because this function is called by early resume code which hasn't
         * yet set up the percpu register (%gs).
         */
-       void *base = (void *)((unsigned long)watermark & ~(THREAD_SIZE - 1));
+       void *base = (void *)ALIGN_DOWN((unsigned long)watermark, THREAD_SIZE);
 
        kasan_unpoison(base, watermark - base, false);
 }
--
Quoted 2 lines

2.47.3

author
#5 lore raw

PATCH 3/3 kasan: common: replace magic number with KASAN_TAG_KERNEL

In reply to Igor Putko

Replace the literal 0xff in assign_tag()
with KASAN_TAG_KERNEL to eliminate the
magic number and improve code clarity.

---
 mm/kasan/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/kasan/common.c +1 -1
index 5faf73c8f..3e9b6b8f5 100644
@@ -188,7 +188,7 @@ static inline u8 assign_tag(struct kmem_cache *cache,
 					const void *object, bool init)
 {
 	if (IS_ENABLED(CONFIG_KASAN_GENERIC))
-		return 0xff;
+		return KASAN_TAG_KERNEL;
 
 	/*
 	 * If the cache neither has a constructor nor has SLAB_TYPESAFE_BY_RCU
Signature · 1 line
2.47.3
#6 lore raw

Re: [PATCH 3/3] kasan: common: replace magic number with KASAN_TAG_KERNEL

Quoted 15 lines · Igor Putko

Replace the literal 0xff in assign_tag()
with KASAN_TAG_KERNEL to eliminate the
magic number and improve code clarity.

---
 mm/kasan/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/kasan/common.c +0 -0
index 5faf73c8f..3e9b6b8f5 100644

mm/kasan/common.c static inline u8 assign_tag(struct kmem_cache *cache, · line 191

Quoted 5 lines
mm/kasan/common.c +1 -1
@@ -188,7 +188,7 @@ static inline u8 assign_tag(struct kmem_cache *cache,
                                        const void *object, bool init)
 {
        if (IS_ENABLED(CONFIG_KASAN_GENERIC))
-               return 0xff;
+               return KASAN_TAG_KERNEL;

Not sure about this change either: the notion of a tag does no apply to the Generic mode, so the code uses 0xff in such cases (the second definition of page_kasan_tag() is another case).

Quoted 6 lines
mm/kasan/common.c +0 -1
        /*
         * If the cache neither has a constructor nor has SLAB_TYPESAFE_BY_RCU
--

2.47.3