Re: [PATCH 2/3] kasan: avoid re-poisoning tag-based kmalloc redzones

[Ryan Roberts <ryan.roberts@arm.com>]

On 19/05/2026 09:15, Dev Jain wrote:
> 
> 
> On 13/05/26 4:27 pm, Dev Jain wrote:
>> When we allocate object from slab, kasan will unpoison the entire object.
>> In case of allocation from kmalloc caches, the actual allocation size
>> request can be less than the size of the kmalloc cache. kasan poisons
>> the bytes following allocation size up till object size to catch OOB.
>>
>> We can do this operation in one shot: while unpoisoning the object upon
>> allocation, only unpoison up till allocation size bytes, so that the
>> bytes following that up till object size remain poisoned.
>>
>> Currently when we free an object into the slab, we use KASAN_SLAB_FREE for
>> poisoning, and use KASAN_SLAB_REDZONE for poisoning the tail end. For
>> tag-based kasan, these two are equal, as opposed to generic kasan. So we
>> make this optimization only for tag-based kasan.
>>
>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>> ---
> 
> Sashiko: https://sashiko.dev/#/patchset/20260513105734.3380544-1-dev.jain@arm.com
> 
> So I'll explain what Sashiko is saying with examples. If I am mis-interpreting
> then please correct me!
> 
> Problem 1:
> 
> p = kzalloc(96, GFP_KERNEL);                 // kmalloc-128
> p = krealloc(p, 120, GFP_KERNEL | __GFP_ZERO);
> 
> Before my patch, kzalloc zeroed bytes 0..127, krealloc exposes bytes 96..119
> which are already zero from kzalloc.
> 
> After my patch, kzalloc zeroes only 0..95, krealloc may think old
> size is 128, so it does not zero 96..119, 96..119 may contain stale data.
> 
> Why is this a problem? 

Intuitively, I would say it's a problem if you're leaking when you previously
were not.

> Surely caller of krealloc shouldn't be
> relying on a previous zeroing. krealloc documentation explicitly says that
> GFP_ZERO is ignored.

What would happen if you did:

p = kzalloc(96, GFP_KERNEL);                 // kmalloc-128
p = krealloc(p, 129, GFP_KERNEL | __GFP_ZERO);

Presumably the krealloc() would have to move the whole allocation to the next
biggest bin? Would p[128] be zeroed? If so, then I'd say your change is problematic.

> 
> 
> Problem 2 for SW tags:
> 
> p = kzalloc(97, GFP_KERNEL);   // kmalloc-128, granule = 16
> 
> Before patch:
> 
> KASAN unpoisons 0..111
> memset zeroes 0..127
> p[100] will not be caught by KASAN, but reads zero
> 
> After patch:
> 
> KASAN unpoisons 0..111
> memset zeroes only 0..96
> p[100] will not be caught by KASAN and reads stale data
> 
> Again why is this a problem?

Because you have defeated KASAN and are leaking data? I think you'd want to zero
at least to the end of the KASAN granule?

Thanks,
Ryan


> 
>>  include/linux/kasan.h | 17 +++++++++----
>>  mm/kasan/common.c     | 55 +++++++++++++++++++++++++++++++++----------
>>  mm/slub.c             | 11 +++++----
>>  3 files changed, 61 insertions(+), 22 deletions(-)
>>
>> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
>> index bf233bde68c7..fd7c1f5f9fd6 100644
>> --- a/include/linux/kasan.h
>> +++ b/include/linux/kasan.h
>> @@ -102,6 +102,12 @@ static inline bool kasan_has_integrated_init(void)
>>  	return kasan_hw_tags_enabled();
>>  }
>>  
>> +static inline bool kasan_has_tag_based_kmalloc_redzones(void)
>> +{
>> +	return kasan_enabled() &&
>> +	       (IS_ENABLED(CONFIG_KASAN_SW_TAGS) || kasan_hw_tags_enabled());
>> +}
>> +
>>  #ifdef CONFIG_KASAN
>>  void __kasan_unpoison_range(const void *addr, size_t size);
>>  static __always_inline void kasan_unpoison_range(const void *addr, size_t size)
>> @@ -244,13 +250,14 @@ static __always_inline void kasan_kfree_large(void *ptr)
>>  		__kasan_kfree_large(ptr, _RET_IP_);
>>  }
>>  
>> -void * __must_check __kasan_slab_alloc(struct kmem_cache *s,
>> -				       void *object, gfp_t flags, bool init);
>> +void * __must_check __kasan_slab_alloc(struct kmem_cache *s, void *object,
>> +				       size_t size, gfp_t flags, bool init);
>>  static __always_inline void * __must_check kasan_slab_alloc(
>> -		struct kmem_cache *s, void *object, gfp_t flags, bool init)
>> +		struct kmem_cache *s, void *object, size_t size,
>> +		gfp_t flags, bool init)
>>  {
>>  	if (kasan_enabled())
>> -		return __kasan_slab_alloc(s, object, flags, init);
>> +		return __kasan_slab_alloc(s, object, size, flags, init);
>>  	return object;
>>  }
>>  
>> @@ -437,7 +444,7 @@ static inline bool kasan_slab_free(struct kmem_cache *s, void *object,
>>  }
>>  static inline void kasan_kfree_large(void *ptr) {}
>>  static inline void *kasan_slab_alloc(struct kmem_cache *s, void *object,
>> -				   gfp_t flags, bool init)
>> +				   size_t size, gfp_t flags, bool init)
>>  {
>>  	return object;
>>  }
>> diff --git a/mm/kasan/common.c b/mm/kasan/common.c
>> index b7d05c2a6d93..9a4db9c21aaf 100644
>> --- a/mm/kasan/common.c
>> +++ b/mm/kasan/common.c
>> @@ -326,14 +326,25 @@ void __kasan_kfree_large(void *ptr, unsigned long ip)
>>  	/* The object will be poisoned by kasan_poison_pages(). */
>>  }
>>  
>> +static inline size_t slab_unpoison_size(struct kmem_cache *cache, size_t size)
>> +{
>> +	if (kasan_has_tag_based_kmalloc_redzones() && is_kmalloc_cache(cache))
>> +		return min_t(size_t, size, cache->object_size);
>> +
>> +	return cache->object_size;
>> +}
>> +
>>  static inline void unpoison_slab_object(struct kmem_cache *cache, void *object,
>> -					gfp_t flags, bool init)
>> +					size_t size, gfp_t flags, bool init)
>>  {
>>  	/*
>> -	 * Unpoison the whole object. For kmalloc() allocations,
>> -	 * poison_kmalloc_redzone() will do precise poisoning.
>> +	 * For tag-based modes, kmalloc redzones all use the same invalid tag.
>> +	 * Keep the tail poisoned and only unpoison the requested allocation
>> +	 * size. Generic KASAN keeps distinct shadow values for free objects and
>> +	 * redzones, so it still unpoisons the whole object and later poisons
>> +	 * the precise redzone.
>>  	 */
>> -	kasan_unpoison(object, cache->object_size, init);
>> +	kasan_unpoison(object, slab_unpoison_size(cache, size), init);
>>  
>>  	/* Save alloc info (if possible) for non-kmalloc() allocations. */
>>  	if (kasan_stack_collection_enabled() && !is_kmalloc_cache(cache))
>> @@ -341,7 +352,8 @@ static inline void unpoison_slab_object(struct kmem_cache *cache, void *object,
>>  }
>>  
>>  void * __must_check __kasan_slab_alloc(struct kmem_cache *cache,
>> -					void *object, gfp_t flags, bool init)
>> +					void *object, size_t size,
>> +					gfp_t flags, bool init)
>>  {
>>  	u8 tag;
>>  	void *tagged_object;
>> @@ -363,11 +375,18 @@ void * __must_check __kasan_slab_alloc(struct kmem_cache *cache,
>>  	tagged_object = set_tag(object, tag);
>>  
>>  	/* Unpoison the object and save alloc info for non-kmalloc() allocations. */
>> -	unpoison_slab_object(cache, tagged_object, flags, init);
>> +	unpoison_slab_object(cache, tagged_object, size, flags, init);
>>  
>>  	return tagged_object;
>>  }
>>  
>> +static inline void save_kmalloc_alloc_info(struct kmem_cache *cache,
>> +					   void *object, gfp_t flags)
>> +{
>> +	if (kasan_stack_collection_enabled() && is_kmalloc_cache(cache))
>> +		kasan_save_alloc_info(cache, object, flags);
>> +}
>> +
>>  static inline void poison_kmalloc_redzone(struct kmem_cache *cache,
>>  				const void *object, size_t size, gfp_t flags)
>>  {
>> @@ -394,8 +413,7 @@ static inline void poison_kmalloc_redzone(struct kmem_cache *cache,
>>  	 * Save alloc info (if possible) for kmalloc() allocations.
>>  	 * This also rewrites the alloc info when called from kasan_krealloc().
>>  	 */
>> -	if (kasan_stack_collection_enabled() && is_kmalloc_cache(cache))
>> -		kasan_save_alloc_info(cache, (void *)object, flags);
>> +	save_kmalloc_alloc_info(cache, (void *)object, flags);
>>  
>>  }
>>  
>> @@ -411,8 +429,14 @@ void * __must_check __kasan_kmalloc(struct kmem_cache *cache, const void *object
>>  	if (is_kfence_address(object))
>>  		return (void *)object;
>>  
>> -	/* The object has already been unpoisoned by kasan_slab_alloc(). */
>> -	poison_kmalloc_redzone(cache, object, size, flags);
>> +	/*
>> +	 * For tag-based modes, the object has already been precisely
>> +	 * unpoisoned by kasan_slab_alloc(). The tail remains poisoned.
>> +	 */
>> +	if (kasan_has_tag_based_kmalloc_redzones())
>> +		save_kmalloc_alloc_info(cache, (void *)object, flags);
>> +	else
>> +		poison_kmalloc_redzone(cache, object, size, flags);
>>  
>>  	/* Keep the tag that was set by kasan_slab_alloc(). */
>>  	return (void *)object;
>> @@ -561,11 +585,16 @@ void __kasan_mempool_unpoison_object(void *ptr, size_t size, unsigned long ip)
>>  		return;
>>  
>>  	/* Unpoison the object and save alloc info for non-kmalloc() allocations. */
>> -	unpoison_slab_object(slab->slab_cache, ptr, flags, false);
>> +	unpoison_slab_object(slab->slab_cache, ptr, size, flags, false);
>>  
>>  	/* Poison the redzone and save alloc info for kmalloc() allocations. */
>> -	if (is_kmalloc_cache(slab->slab_cache))
>> -		poison_kmalloc_redzone(slab->slab_cache, ptr, size, flags);
>> +	if (is_kmalloc_cache(slab->slab_cache)) {
>> +		if (kasan_has_tag_based_kmalloc_redzones())
>> +			save_kmalloc_alloc_info(slab->slab_cache, ptr, flags);
>> +		else
>> +			poison_kmalloc_redzone(slab->slab_cache, ptr, size,
>> +					       flags);
>> +	}
>>  }
>>  
>>  bool __kasan_check_byte(const void *address, unsigned long ip)
>> diff --git a/mm/slub.c b/mm/slub.c
>> index da3520769d1f..15144b2e078c 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -4550,8 +4550,9 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
>>  	 * replacement of current poisoning under certain debug option, and
>>  	 * won't break other sanity checks.
>>  	 */
>> -	if (kmem_cache_debug_flags(s, SLAB_STORE_USER | SLAB_RED_ZONE) &&
>> -	    (s->flags & SLAB_KMALLOC))
>> +	if ((s->flags & SLAB_KMALLOC) &&
>> +	    (kmem_cache_debug_flags(s, SLAB_STORE_USER | SLAB_RED_ZONE) ||
>> +	     kasan_has_tag_based_kmalloc_redzones()))
>>  		zero_size = orig_size;
>>  
>>  	/*
>> @@ -4573,7 +4574,8 @@ bool slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
>>  	 * As p[i] might get tagged, memset and kmemleak hook come after KASAN.
>>  	 */
>>  	for (i = 0; i < size; i++) {
>> -		p[i] = kasan_slab_alloc(s, p[i], init_flags, kasan_init);
>> +		p[i] = kasan_slab_alloc(s, p[i], orig_size, init_flags,
>> +					kasan_init);
>>  		if (p[i] && init && (!kasan_init ||
>>  				     !kasan_has_integrated_init()))
>>  			memset(p[i], 0, zero_size);
>> @@ -7615,7 +7617,8 @@ static void early_kmem_cache_node_alloc(int node)
>>  #ifdef CONFIG_SLUB_DEBUG
>>  	init_object(kmem_cache_node, n, SLUB_RED_ACTIVE);
>>  #endif
>> -	n = kasan_slab_alloc(kmem_cache_node, n, GFP_KERNEL, false);
>> +	n = kasan_slab_alloc(kmem_cache_node, n, kmem_cache_node->object_size,
>> +			     GFP_KERNEL, false);
>>  	slab->freelist = get_freepointer(kmem_cache_node, n);
>>  	slab->inuse = 1;
>>  	kmem_cache_node->per_node[node].node = n;
>