linux-kernel - Re: [PATCH v3] kasan: Improve free meta storage in Generic KASAN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <VI1P193MB0752282E559B37F12EB7982599B9A@VI1P193MB0752.EURP193.PROD.OUTLOOK.COM>
Date:   Thu, 23 Nov 2023 18:43:35 +0800
From:   Juntong Deng <juntong.deng@...look.com>
To:     ryabinin.a.a@...il.com, glider@...gle.com, andreyknvl@...il.com,
        dvyukov@...gle.com, vincenzo.frascino@....com,
        akpm@...ux-foundation.org
Cc:     kasan-dev@...glegroups.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH v3] kasan: Improve free meta storage in Generic KASAN

On 2023/11/23 2:46, Juntong Deng wrote:
> Currently free meta can only be stored in object if the object is
> not smaller than free meta.
> 
> After the improvement, when the object is smaller than free meta and
> SLUB DEBUG is not enabled, it is possible to store part of the free
> meta in the object, reducing the increased size of the red zone.
> 
> Example:
> 
> free meta size: 16 bytes
> alloc meta size: 16 bytes
> object size: 8 bytes
> optimal redzone size (object_size <= 64): 16 bytes
> 
> Before improvement:
> actual redzone size = alloc meta size + free meta size = 32 bytes
> 
> After improvement:
> actual redzone size = alloc meta size + (free meta size - object size)
>                      = 24 bytes
> 
> Suggested-by: Dmitry Vyukov <dvyukov@...gle.com>
> Signed-off-by: Juntong Deng <juntong.deng@...look.com>
> ---
> V2 -> V3: When SLUB DEBUG is enabled, the previous free meta
> storage method continues to be used. Cancel the change to
> kasan_metadata_size().
> 
> V1 -> V2: Make kasan_metadata_size() adapt to the improved
> free meta storage
> 
>   mm/kasan/generic.c | 39 +++++++++++++++++++++++++++++----------
>   1 file changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> index 4d837ab83f08..97713251053c 100644
> --- a/mm/kasan/generic.c
> +++ b/mm/kasan/generic.c
> @@ -361,6 +361,8 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
>   {
>   	unsigned int ok_size;
>   	unsigned int optimal_size;
> +	unsigned int rem_free_meta_size;
> +	unsigned int orig_alloc_meta_offset;
>   
>   	if (!kasan_requires_meta())
>   		return;
> @@ -394,6 +396,9 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
>   		/* Continue, since free meta might still fit. */
>   	}
>   
> +	ok_size = *size;
> +	orig_alloc_meta_offset = cache->kasan_info.alloc_meta_offset;
> +
>   	/*
>   	 * Add free meta into redzone when it's not possible to store
>   	 * it in the object. This is the case when:
> @@ -401,23 +406,37 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
>   	 *    be touched after it was freed, or
>   	 * 2. Object has a constructor, which means it's expected to
>   	 *    retain its content until the next allocation, or
> -	 * 3. Object is too small.
> +	 * 3. Object is too small and SLUB DEBUG is enabled. Avoid
> +	 *    free meta that exceeds the object size corrupts the
> +	 *    SLUB DEBUG metadata.
>   	 * Otherwise cache->kasan_info.free_meta_offset = 0 is implied.
> +	 * If the object is smaller than the free meta and SLUB DEBUG
> +	 * is not enabled, it is still possible to store part of the
> +	 * free meta in the object.
>   	 */
> -	if ((cache->flags & SLAB_TYPESAFE_BY_RCU) || cache->ctor ||
> -	    cache->object_size < sizeof(struct kasan_free_meta)) {
> -		ok_size = *size;
> -
> +	if ((cache->flags & SLAB_TYPESAFE_BY_RCU) || cache->ctor) {
>   		cache->kasan_info.free_meta_offset = *size;
>   		*size += sizeof(struct kasan_free_meta);
> -
> -		/* If free meta doesn't fit, don't add it. */
> -		if (*size > KMALLOC_MAX_SIZE) {
> -			cache->kasan_info.free_meta_offset = KASAN_NO_FREE_META;
> -			*size = ok_size;
> +	} else if (cache->object_size < sizeof(struct kasan_free_meta)) {
> +		if (__slub_debug_enabled()) {
> +			cache->kasan_info.free_meta_offset = *size;
> +			*size += sizeof(struct kasan_free_meta);
> +		} else {
> +			rem_free_meta_size = sizeof(struct kasan_free_meta) -
> +									cache->object_size;
> +			*size += rem_free_meta_size;
> +			if (cache->kasan_info.alloc_meta_offset != 0)
> +				cache->kasan_info.alloc_meta_offset += rem_free_meta_size;
>   		}
>   	}
>   
> +	/* If free meta doesn't fit, don't add it. */
> +	if (*size > KMALLOC_MAX_SIZE) {
> +		cache->kasan_info.free_meta_offset = KASAN_NO_FREE_META;
> +		cache->kasan_info.alloc_meta_offset = orig_alloc_meta_offset;
> +		*size = ok_size;
> +	}
> +
>   	/* Calculate size with optimal redzone. */
>   	optimal_size = cache->object_size + optimal_redzone(cache->object_size);
>   	/* Limit it with KMALLOC_MAX_SIZE (relevant for SLAB only). */


Can someone help to apply the new version of the patch to linux-next?
to replace the buggy version of the patch.