| Message ID | 20231221183540.168428-1-andrey.konovalov@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [mm,1/4] kasan: clean up kasan_cache_create | expand |
On Thu, 21 Dec 2023 at 19:35, <andrey.konovalov@linux.dev> wrote: > > From: Andrey Konovalov <andreyknvl@google.com> > > Reorganize the code to avoid nested if/else checks to improve the > readability. > > Also drop the confusing comments about KMALLOC_MAX_SIZE checks: they > are relevant for both SLUB and SLAB (originally, the comments likely > confused KMALLOC_MAX_SIZE with KMALLOC_MAX_CACHE_SIZE). > > Fixes: a5989d4ed40c ("kasan: improve free meta storage in Generic KASAN") > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Reviewed-by: Marco Elver <elver@google.com> > --- > mm/kasan/generic.c | 67 +++++++++++++++++++++++++++------------------- > 1 file changed, 39 insertions(+), 28 deletions(-) > > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c > index 54e20b2bc3e1..769e43e05d0b 100644 > --- a/mm/kasan/generic.c > +++ b/mm/kasan/generic.c > @@ -381,16 +381,11 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, > > ok_size = *size; > > - /* Add alloc meta into redzone. */ > + /* Add alloc meta into the redzone. */ > cache->kasan_info.alloc_meta_offset = *size; > *size += sizeof(struct kasan_alloc_meta); > > - /* > - * If alloc meta doesn't fit, don't add it. > - * This can only happen with SLAB, as it has KMALLOC_MAX_SIZE equal > - * to KMALLOC_MAX_CACHE_SIZE and doesn't fall back to page_alloc for > - * larger sizes. > - */ > + /* If alloc meta doesn't fit, don't add it. */ > if (*size > KMALLOC_MAX_SIZE) { > cache->kasan_info.alloc_meta_offset = 0; > *size = ok_size; > @@ -401,36 +396,52 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, > orig_alloc_meta_offset = cache->kasan_info.alloc_meta_offset; > > /* > - * Add free meta into redzone when it's not possible to store > + * Store free meta in the redzone when it's not possible to store > * it in the object. This is the case when: > * 1. Object is SLAB_TYPESAFE_BY_RCU, which means that it can > * 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 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. > + * retain its content until the next allocation. > */ > if ((cache->flags & SLAB_TYPESAFE_BY_RCU) || cache->ctor) { > cache->kasan_info.free_meta_offset = *size; > *size += sizeof(struct kasan_free_meta); > - } 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; > - } > + goto free_meta_added; > + } > + > + /* > + * Otherwise, if the object is large enough to contain free meta, > + * store it within the object. > + */ > + if (sizeof(struct kasan_free_meta) <= cache->object_size) { > + /* cache->kasan_info.free_meta_offset = 0 is implied. */ > + goto free_meta_added; > } > > + /* > + * For smaller objects, store the beginning of free meta within the > + * object and the end in the redzone. And thus shift the location of > + * alloc meta to free up space for free meta. > + * This is only possible when slub_debug is disabled, as otherwise > + * the end of free meta will overlap with slub_debug metadata. > + */ > + if (!__slub_debug_enabled()) { > + 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; > + goto free_meta_added; > + } > + > + /* > + * If the object is small and slub_debug is enabled, store free meta > + * in the redzone after alloc meta. > + */ > + cache->kasan_info.free_meta_offset = *size; > + *size += sizeof(struct kasan_free_meta); > + > +free_meta_added: > /* 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; > @@ -440,7 +451,7 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *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). */ > + /* Limit it with KMALLOC_MAX_SIZE. */ > if (optimal_size > KMALLOC_MAX_SIZE) > optimal_size = KMALLOC_MAX_SIZE; > /* Use optimal size if the size with added metas is not large enough. */ > -- > 2.25.1 >
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c index 54e20b2bc3e1..769e43e05d0b 100644 --- a/mm/kasan/generic.c +++ b/mm/kasan/generic.c @@ -381,16 +381,11 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, ok_size = *size; - /* Add alloc meta into redzone. */ + /* Add alloc meta into the redzone. */ cache->kasan_info.alloc_meta_offset = *size; *size += sizeof(struct kasan_alloc_meta); - /* - * If alloc meta doesn't fit, don't add it. - * This can only happen with SLAB, as it has KMALLOC_MAX_SIZE equal - * to KMALLOC_MAX_CACHE_SIZE and doesn't fall back to page_alloc for - * larger sizes. - */ + /* If alloc meta doesn't fit, don't add it. */ if (*size > KMALLOC_MAX_SIZE) { cache->kasan_info.alloc_meta_offset = 0; *size = ok_size; @@ -401,36 +396,52 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, orig_alloc_meta_offset = cache->kasan_info.alloc_meta_offset; /* - * Add free meta into redzone when it's not possible to store + * Store free meta in the redzone when it's not possible to store * it in the object. This is the case when: * 1. Object is SLAB_TYPESAFE_BY_RCU, which means that it can * 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 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. + * retain its content until the next allocation. */ if ((cache->flags & SLAB_TYPESAFE_BY_RCU) || cache->ctor) { cache->kasan_info.free_meta_offset = *size; *size += sizeof(struct kasan_free_meta); - } 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; - } + goto free_meta_added; + } + + /* + * Otherwise, if the object is large enough to contain free meta, + * store it within the object. + */ + if (sizeof(struct kasan_free_meta) <= cache->object_size) { + /* cache->kasan_info.free_meta_offset = 0 is implied. */ + goto free_meta_added; } + /* + * For smaller objects, store the beginning of free meta within the + * object and the end in the redzone. And thus shift the location of + * alloc meta to free up space for free meta. + * This is only possible when slub_debug is disabled, as otherwise + * the end of free meta will overlap with slub_debug metadata. + */ + if (!__slub_debug_enabled()) { + 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; + goto free_meta_added; + } + + /* + * If the object is small and slub_debug is enabled, store free meta + * in the redzone after alloc meta. + */ + cache->kasan_info.free_meta_offset = *size; + *size += sizeof(struct kasan_free_meta); + +free_meta_added: /* 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; @@ -440,7 +451,7 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *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). */ + /* Limit it with KMALLOC_MAX_SIZE. */ if (optimal_size > KMALLOC_MAX_SIZE) optimal_size = KMALLOC_MAX_SIZE; /* Use optimal size if the size with added metas is not large enough. */