linux-kernel - Re: [PATCHv2 03/11] atomics: simplify cmpxchg() instrumentation

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 25 Jun 2018 12:48:20 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Andrea Parri <andrea.parri@...rulasolutions.com>
Cc:     linux-kernel@...r.kernel.org, will.deacon@....com,
        peterz@...radead.org, boqun.feng@...il.com,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [PATCHv2 03/11] atomics: simplify cmpxchg() instrumentation

On Mon, Jun 25, 2018 at 01:38:03PM +0200, Andrea Parri wrote:
> On Mon, Jun 25, 2018 at 11:59:44AM +0100, Mark Rutland wrote:
> > Currently we define some fairly verbose wrappers for the cmpxchg()
> > family so that we can pass a pointer and size into kasan_check_write().
> > 
> > The wrapper duplicate the size-switching logic necessary in arch code,
> > and only work for scalar types. On some architectures, (cmp)xchg are
> > used on non-scalar types, and thus the instrumented wrappers need to be
> > able to handle this.
> > 
> > We could take the type-punning logic form {READ,WRITE}_ONCE(), but this
> > makes the wrappers even more verbose, and requires several local
> > variables in the macros.
> > 
> > Instead, let's simplify the wrappers into simple macros which:
> > 
> >  * snapshot the pointer into a single local variable, called __ai_ptr to
> >    avoid conflicts with variables in the scope of the caller.
> > 
> >  * call kasan_check_read() on __ai_ptr.
> 
> Maybe I'm misreading the diff: aren't you calling kasan_check_write()?

Sorry, yes -- I'll update the commit message.

> (not sure if it makes a difference in this case/for KTSan, but CMPXCHG
> does not necessarily perform a write...)

For KASAN, it shouldn't matter -- it'll only be used to report whether
the access was a read or write, and it's fine to say that it's a
potential write. KTSAN's not yet upstream, so I'll leave that detail to
Dmitry.

Thanks,
Mark.