On Wed, Sep 21, 2022 at 2:03 PM Feng Tang <feng.tang@xxxxxxxxx> wrote:
>
> Agree, it's better not touch the internal fields in slub code.
>
> How about the following patch, it merge the 2 functions with one flag
> indicating in meta data or object. (I'm fine with 2 separate functions)
The overall approach sounds good. See some comments below.
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index b092277bf48d..0ad05a34e708 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -150,11 +150,12 @@ static __always_inline void kasan_cache_create_kmalloc(struct kmem_cache *cache)
> __kasan_cache_create_kmalloc(cache);
> }
>
> -size_t __kasan_metadata_size(struct kmem_cache *cache);
> -static __always_inline size_t kasan_metadata_size(struct kmem_cache *cache)
> +size_t __kasan_meta_size(struct kmem_cache *cache, bool in_slab_object);
> +static __always_inline size_t kasan_meta_size(struct kmem_cache *cache,
> + bool in_slab_object)
I would keep the name as kasan_metadata_size as it's more clear to
external users but rename in_slab_object to in_object to make the
declaration shorter.
> {
> if (kasan_enabled())
> - return __kasan_metadata_size(cache);
> + return __kasan_meta_size(cache, in_slab_object);
> return 0;
> }
>
> diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> index 69f583855c8b..2a8710461ebb 100644
> --- a/mm/kasan/common.c
> +++ b/mm/kasan/common.c
> @@ -218,14 +218,21 @@ void __kasan_cache_create_kmalloc(struct kmem_cache *cache)
> cache->kasan_info.is_kmalloc = true;
> }
>
> -size_t __kasan_metadata_size(struct kmem_cache *cache)
> +size_t __kasan_meta_size(struct kmem_cache *cache, bool in_slab_object)
> {
> if (!kasan_stack_collection_enabled())
> return 0;
> - return (cache->kasan_info.alloc_meta_offset ?
> - sizeof(struct kasan_alloc_meta) : 0) +
> - (cache->kasan_info.free_meta_offset ?
> - sizeof(struct kasan_free_meta) : 0);
> +
> + if (in_slab_object)
> + return (cache->kasan_info.alloc_meta_offset == 0 ?
> + sizeof(struct kasan_alloc_meta) : 0) +
> + (cache->kasan_info.free_meta_offset ?
> + sizeof(struct kasan_free_meta) : 0);
> + else
> + return (cache->kasan_info.alloc_meta_offset == 0 ?
> + sizeof(struct kasan_alloc_meta) : 0) +
> + (cache->kasan_info.free_meta_offset ?
> + sizeof(struct kasan_free_meta) : 0);
Something weird here: both if and else cases are the same.
The change also needs to be rebased onto [1].
Thanks!
[1] https://lore.kernel.org/linux-mm/c7b316d30d90e5947eb8280f4dc78856a49298cf.1662411799.git.andreyknvl@xxxxxxxxxx/
