On Mon, Sep 9, 2024 at 3:30 AM Feng Tang <feng.tang@xxxxxxxxx> wrote:
>
> For a kmalloc object, when both kasan and slub redzone sanity check
> are enabled, they could both manipulate its data space like storing
> kasan free meta data and setting up kmalloc redzone, and may affect
> accuracy of that object's 'orig_size'.
>
> As an accurate 'orig_size' will be needed by some function like
> krealloc() soon, save kasan's free meta data in slub's metadata area
> instead of inside object when 'orig_size' is enabled.
>
> This will make it easier to maintain/understand the code. Size wise,
> when these two options are both enabled, the slub meta data space is
> already huge, and this just slightly increase the overall size.
>
> Signed-off-by: Feng Tang <feng.tang@xxxxxxxxx>
> ---
> mm/kasan/generic.c | 5 ++++-
> mm/slab.h | 6 ++++++
> mm/slub.c | 17 -----------------
> 3 files changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> index 6310a180278b..cad376199d47 100644
> --- a/mm/kasan/generic.c
> +++ b/mm/kasan/generic.c
> @@ -393,8 +393,11 @@ 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.
Nit: ", or" above.
> + * 3. It is from a kmalloc cache which enables the debug option
> + * to store original size.
> */
> - if ((cache->flags & SLAB_TYPESAFE_BY_RCU) || cache->ctor) {
> + if ((cache->flags & SLAB_TYPESAFE_BY_RCU) || cache->ctor ||
> + slub_debug_orig_size(cache)) {
> cache->kasan_info.free_meta_offset = *size;
> *size += sizeof(struct kasan_free_meta);
> goto free_meta_added;
> diff --git a/mm/slab.h b/mm/slab.h
> index 90f95bda4571..7a0e9b34ba2a 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -689,6 +689,12 @@ void __kmem_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *slab)
> void __check_heap_object(const void *ptr, unsigned long n,
> const struct slab *slab, bool to_user);
>
> +static inline bool slub_debug_orig_size(struct kmem_cache *s)
> +{
> + return (kmem_cache_debug_flags(s, SLAB_STORE_USER) &&
> + (s->flags & SLAB_KMALLOC));
> +}
> +
> #ifdef CONFIG_SLUB_DEBUG
> void skip_orig_size_check(struct kmem_cache *s, const void *object);
> #endif
> diff --git a/mm/slub.c b/mm/slub.c
> index 23761533329d..996a72fa6f62 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -230,12 +230,6 @@ static inline bool kmem_cache_debug(struct kmem_cache *s)
> return kmem_cache_debug_flags(s, SLAB_DEBUG_FLAGS);
> }
>
> -static inline bool slub_debug_orig_size(struct kmem_cache *s)
> -{
> - return (kmem_cache_debug_flags(s, SLAB_STORE_USER) &&
> - (s->flags & SLAB_KMALLOC));
> -}
> -
> void *fixup_red_left(struct kmem_cache *s, void *p)
> {
> if (kmem_cache_debug_flags(s, SLAB_RED_ZONE))
> @@ -760,21 +754,10 @@ static inline void set_orig_size(struct kmem_cache *s,
> void *object, unsigned int orig_size)
> {
> void *p = kasan_reset_tag(object);
> - unsigned int kasan_meta_size;
>
> if (!slub_debug_orig_size(s))
> return;
>
> - /*
> - * KASAN can save its free meta data inside of the object at offset 0.
> - * If this meta data size is larger than 'orig_size', it will overlap
> - * the data redzone in [orig_size+1, object_size]. Thus, we adjust
> - * 'orig_size' to be as at least as big as KASAN's meta data.
> - */
> - kasan_meta_size = kasan_metadata_size(s, true);
> - if (kasan_meta_size > orig_size)
> - orig_size = kasan_meta_size;
> -
> p += get_info_end(s);
> p += sizeof(struct track) * 2;
>
> --
> 2.34.1
>
Acked-by: Andrey Konovalov <andreyknvl@xxxxxxxxx>
