All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Segher Boessenkool <segher@kernel.crashing.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Borislav Petkov <bp@alien8.de>,
	Nathan Chancellor <nathan@kernel.org>, x86-ml <x86@kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	llvm@lists.linux.dev, Josh Poimboeuf <jpoimboe@redhat.com>,
	linux-toolchains@vger.kernel.org
Subject: Re: clang memcpy calls
Date: Mon, 28 Mar 2022 13:54:26 +0200	[thread overview]
Message-ID: <20220328115426.GB8939@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <YkGL929QoFiTfMK7@tucnak>

On Mon, Mar 28, 2022 at 12:20:39PM +0200, Jakub Jelinek wrote:
> On Mon, Mar 28, 2022 at 10:52:54AM +0100, Mark Rutland wrote:
> > I think we're talking past each other here, so let me be more precise. :)
> > 
> > The key thing is that when the user passes `-fsantize=address`, instrumentation
> > is added by (a part of) the compiler. That instrumentation is added under some
> > assumptions as to how the compiler as a whole will behave.
> > 
> > With that in mind, the question is how is __attribute__((no_sanitize_address))
> > intended to work when considering all the usual expectations around how the
> > compiler can play with memcpy and similar?
> 
> no_sanitize_address or lack thereof is whether the current function
> shouldn't be or should be ASan instrumented, not on whether other functions
> it calls are instrumented or not.  memcpy/memmove/memset are just a tiny bit
> special case because the compiler can add them on their own even if they
> aren't present in the source (there are a few others the compiler can
> pattern match too) and various builtins can be on the other side expanded
> inline instead of called, so one then gets the sanitization status of the
> function in which it is used rather than whether the out of line
> implementation of the function is sanitized.
> 
> If coexistence of instrumented and non-instrumented memcpy etc. was the goal
> (it clearly wasn't), then the sanitizer libraries wouldn't be overriding
> memcpy calls, but instead the compiler would redirect calls to memcpy in
> instrumented functions to say __asan_memcpy which then would be
> instrumented.

This then leaves us holding the pieces because this behaviour is
actively wrong.

A non-instrumented function *MUST*NOT* call an instrumented function,
ever. This very much violates how we use/expect
__attribute__((no_sanitize_address)) to work.

If we use that on a function, we expect/rely on that function (nor any
compiler tranformation thereof) to *NOT* have instrumentation. This is a
hard correctness requirement that cannot be argued with.


So there's two options:

 A) compiler generates implicit mem*() calls with the knowledge that
    mem*() is not instrumentet, and as such will also emit
    instrumentation for it when so required (or calls mem*_asan() like
    functions.

 B) compiler knows mem*() are instrumented, at which point the implicit
    mem*() calls are no longer equivalent under
    __attribute__((no_sanitize_address)) and will no longer perform
    these substitutions.


At some point this becomes a choice between being able to boot or having
KASAN, choice is simple.

  reply	other threads:[~2022-03-28 11:58 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-24 11:19 clang memcpy calls Borislav Petkov
2022-03-24 15:29 ` Mark Rutland
2022-03-24 18:43 ` Nick Desaulniers
2022-03-24 22:54   ` David Laight
2022-03-25 12:15   ` Mark Rutland
2022-03-25 14:13     ` Peter Zijlstra
2022-03-25 15:12       ` Segher Boessenkool
2022-03-28  9:52         ` Mark Rutland
2022-03-28 10:20           ` Jakub Jelinek
2022-03-28 11:54             ` Peter Zijlstra [this message]
2022-03-28 12:55             ` Mark Rutland
2022-03-28 13:12               ` Jakub Jelinek
2022-03-28 13:44                 ` Mark Rutland
2022-03-30 14:45                   ` Marco Elver
2022-03-28 14:22           ` Segher Boessenkool
2022-03-28 14:58             ` Mark Rutland
2022-03-28 15:59               ` Segher Boessenkool
2022-03-28 16:16                 ` Peter Zijlstra
2022-03-28 16:58                   ` Segher Boessenkool

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=20220328115426.GB8939@worktop.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bp@alien8.de \
    --cc=jakub@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mark.rutland@arm.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=segher@kernel.crashing.org \
    --cc=x86@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.