On Thu, Nov 23, 2023 at 7:35 AM Feng Tang <feng.tang@xxxxxxxxx> wrote:
>
Hi Feng,
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -870,20 +870,20 @@ 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;
> >
> > -#ifdef CONFIG_KASAN_GENERIC
> > /*
> > - * KASAN could save its free meta data in object's data area at
> > - * offset 0, if the size is larger than 'orig_size', it will
> > - * overlap the data redzone in [orig_size+1, object_size], and
> > - * the check should be skipped.
> > + * 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.
> > */
> > - if (kasan_metadata_size(s, true) > orig_size)
> > - orig_size = s->object_size;
> > -#endif
> > + kasan_meta_size = kasan_metadata_size(s, true);
> > + if (kasan_meta_size > orig_size)
> > + orig_size = kasan_meta_size;
>
> 'orig_size' is to save the orignal request size for kmalloc object,
> and its main purpose is to detect the memory wastage of kmalloc
> objects, see commit 6edf2576a6cc "mm/slub: enable debugging memory
> wasting of kmalloc"
>
> Setting "orig_size = s->object_size" was to skip the wastage check
> and the redzone sanity check for this 'wasted space'.
Yes, I get that.
The point of my change was to allow slub_debug detecting overwrites in
the [kasan_meta_size, object_size) range when KASAN stores its free
meta in the [0, kasan_meta_size) range. If orig_size is set to
object_size, writes to that area will not be detected. I also thought
that using kasan_meta_size instead of object_size for orig_size might
give the reader better understanding of the memory layout.
> So it's better not to set 'kasan_meta_size' to orig_size.
I don't have a strong preference here: slub_debug and KASAN are not
really meant to be used together anyway. So if you prefer, I can
revert this change and keep using object_size as before.
> And from the below code, IIUC, the orig_size is not used in fixing
> the boot problem found by Hyeonggon?
No, this is a just a partially-related clean up. It just seemed
natural to include it into the fix, as it also touches the code around
a kasan_metadata_size call.
Thanks!
