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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Sun, 29 May 2016 15:00:05 +0000
From:	"Luruo, Kuthonuzo" <kuthonuzo.luruo@....com>
To:	Dmitry Vyukov <dvyukov@...gle.com>
CC:	Andrey Ryabinin <aryabinin@...tuozzo.com>,
	Alexander Potapenko <glider@...gle.com>,
	Christoph Lameter <cl@...ux.com>,
	Pekka Enberg <penberg@...nel.org>,
	David Rientjes <rientjes@...gle.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	kasan-dev <kasan-dev@...glegroups.com>,
	LKML <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	Yury Norov <ynorov@...iumnetworks.com>
Subject: RE: [PATCH v3 1/2] mm, kasan: improve double-free detection

> >> > +/* 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.
>