Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 25 Jun 2018 12:48:20 +0100
From:   Mark Rutland <mark.rutland@arm.com>
To:     Andrea Parri <andrea.parri@amarulasolutions.com>
Cc:     linux-kernel@vger.kernel.org, will.deacon@arm.com,
        peterz@infradead.org, boqun.feng@gmail.com,
        Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCHv2 03/11] atomics: simplify cmpxchg() instrumentation
Message-ID: <20180625114820.zhrhcyy4s47n4r5e@lakrids.cambridge.arm.com>
References: <20180625105952.3756-1-mark.rutland@arm.com>
 <20180625105952.3756-4-mark.rutland@arm.com>
 <20180625113803.GA13628@andrea>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180625113803.GA13628@andrea>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.