linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Marco Elver <elver@google.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>,
	Andrey Konovalov <andreyknvl@google.com>,
	Alexander Potapenko <glider@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	kasan-dev <kasan-dev@googlegroups.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	christophe leroy <christophe.leroy@c-s.fr>,
	Daniel Axtens <dja@axtens.net>,
	linux-arch <linux-arch@vger.kernel.org>
Subject: Re: [PATCH -rcu] asm-generic, kcsan: Add KCSAN instrumentation for bitops
Date: Fri, 17 Jan 2020 13:25:08 +0100	[thread overview]
Message-ID: <CAK8P3a066Knr-KC2v4M8Dr1phr0Gbb2KeZZLQ7Ana0fkrgPDPg@mail.gmail.com> (raw)
In-Reply-To: <CANpmjNMk2HbuvmN1RaZ=8OV+tx9qZwKyRySONDRQar6RCGM1SA@mail.gmail.com>

On Wed, Jan 15, 2020 at 9:50 PM Marco Elver <elver@google.com> wrote:
> On Wed, 15 Jan 2020 at 20:55, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wed, Jan 15, 2020 at 8:51 PM Marco Elver <elver@google.com> wrote:
> > > On Wed, 15 Jan 2020 at 20:27, Arnd Bergmann <arnd@arndb.de> wrote:
> > Are there any that really just want kasan_check_write() but not one
> > of the kcsan checks?
>
> If I understood correctly, this suggestion would amount to introducing
> a new header, e.g. 'ksan-checks.h', that provides unified generic
> checks. For completeness, we will also need to consider reads. Since
> KCSAN provides 4 check variants ({read,write} x {plain,atomic}), we
> will need 4 generic check variants.

Yes, that was the idea.

> I certainly do not feel comfortable blindly introducing kcsan_checks
> in all places where we have kasan_checks, but it may be worthwhile
> adding this infrastructure and starting with atomic-instrumented and
> bitops-instrumented wrappers. The other locations you list above would
> need to be evaluated on a case-by-case basis to check if we want to
> report data races for those accesses.

I think the main question to answer is whether it is more likely to go
wrong because we are missing checks when one caller accidentally
only has one but not the other, or whether they go wrong because
we accidentally check both when we should only be checking one.

My guess would be that the first one is more likely to happen, but
the second one is more likely to cause problems when it happens.

> As a minor data point, {READ,WRITE}_ONCE in compiler.h currently only
> has kcsan_checks and not kasan_checks.

Right. This is because we want an explicit "atomic" check for kcsan
but we want to have the function inlined for kasan, right?

> My personal preference would be to keep the various checks explicit,
> clearly opting into either KCSAN and/or KASAN. Since I do not think
> it's obvious if we want both for the existing and potentially new
> locations (in future), the potential for error by blindly using a
> generic 'ksan_check' appears worse than potentially adding a dozen
> lines or so.
>
> Let me know if you'd like to proceed with 'ksan-checks.h'.

Could you have a look at the files I listed and see if there are any
other examples that probably a different set of checks between the
two, besides the READ_ONCE() example?

If you can't find any, I would prefer having the simpler interface
with just one set of annotations.

     Arnd

  reply	other threads:[~2020-01-17 12:25 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-15 16:57 [PATCH -rcu] asm-generic, kcsan: Add KCSAN instrumentation for bitops Marco Elver
2020-01-15 19:27 ` Arnd Bergmann
2020-01-15 19:51   ` Marco Elver
2020-01-15 19:54     ` Arnd Bergmann
2020-01-15 20:50       ` Marco Elver
2020-01-17 12:25         ` Arnd Bergmann [this message]
2020-01-17 13:14           ` Marco Elver
2020-01-20 14:23             ` Marco Elver
2020-01-20 14:40               ` Arnd Bergmann
2020-01-20 15:11                 ` Marco Elver
2020-01-20 19:02                   ` Arnd Bergmann
2020-01-21 16:12                     ` Marco Elver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAK8P3a066Knr-KC2v4M8Dr1phr0Gbb2KeZZLQ7Ana0fkrgPDPg@mail.gmail.com \
    --to=arnd@arndb.de \
    --cc=andreyknvl@google.com \
    --cc=christophe.leroy@c-s.fr \
    --cc=dja@axtens.net \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulmck@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).