RE: [PATCH v3 1/2] mm, kasan: improve double-free detection

["Luruo, Kuthonuzo" <kuthonuzo.luruo@hpe.com>]

> >> > +/* flags shadow for object header if it has been overwritten. */
> >> > +void kasan_mark_bad_meta(struct kasan_alloc_meta *alloc_info,
> >> > +               struct kasan_access_info *info)
> >> > +{
> >> > +       u8 *datap = (u8 *)&alloc_info->data;
> >> > +
> >> > +       if ((((u8 *)info->access_addr + info->access_size) > datap) &&
> >> > +                       ((u8 *)info->first_bad_addr <= datap) &&
> >> > +                       info->is_write)
> >> > +               kasan_poison_shadow((void *)datap,
> KASAN_SHADOW_SCALE_SIZE,
> >> > +                               KASAN_KMALLOC_BAD_META);
> >>
> >>
> >> Is it only to prevent deadlocks in kasan_meta_lock?
> >>
> >> If so, it is still unrelable because an OOB write can happen in
> >> non-instrumented code. Or, kasan_meta_lock can successfully lock
> >> overwritten garbage before noticing KASAN_KMALLOC_BAD_META. Or, two
> >> threads can assume lock ownership after noticing
> >> KASAN_KMALLOC_BAD_META.
> >>
> >> After the first report we continue working in kind of best effort
> >> mode: we can try to mitigate some things, but generally all bets are
> >> off. Because of that there is no need to build something complex,
> >> global (and still unrelable). I would just wait for at most, say, 10
> >> seconds in kasan_meta_lock, if we can't get the lock -- print an error
> >> and return. That's simple, local and won't deadlock under any
> >> circumstances.
> >> The error message will be helpful, because there are chances we will
> >> report a double-free on free of the corrupted object.
> >>  e
> >> Tests can be arranged so that they write 0 (unlocked) into the meta
> >> (if necessary).
> >
> > Dmitry,
> >
> > Thanks very much for review & comments. Yes, the locking scheme in v3
> > is flawed in the presence of OOB writes on header, safety valve
> > notwithstanding. The core issue is that when thread finds lock held, it is
> > difficult to tell whether a legit lock holder exists or lock bit got flipped
> > from OOB. Earlier, I did consider a lock timeout but felt it to be a bit ugly...
> >
> > However, I believe I've found a solution and was about to push out v4
> > when your comments came in. It takes concept from v3 - exploiting
> > shadow memory - to make lock much more reliable/resilient even in the
> > presence of OOB writes. I'll push out v4 within the hour...
> 
> 
> Locking shadow will probably work.

It does; v4 does this ;-)

> Need to think more.
>