All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: George Popescu <georgepope@google.com>
Cc: Marco Elver <elver@google.com>,
	maz@kernel.org, Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	kvmarm@lists.cs.columbia.edu, LKML <linux-kernel@vger.kernel.org>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	james.morse@arm.com, julien.thierry.kdev@gmail.com,
	suzuki.poulose@arm.com,
	Nathan Chancellor <natechancellor@gmail.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	David Brazdil <dbrazdil@google.com>,
	broonie@kernel.org, Fangrui Song <maskray@google.com>,
	Andrew Scull <ascull@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arnd Bergmann <arnd@arndb.de>,
	kasan-dev <kasan-dev@googlegroups.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Alexander Potapenko <glider@google.com>
Subject: Re: [PATCH 06/14] Fix CFLAGS for UBSAN_BOUNDS on Clang
Date: Thu, 17 Sep 2020 15:21:47 -0700	[thread overview]
Message-ID: <202009171519.951D26DB@keescook> (raw)
In-Reply-To: <20200917113540.GA1742660@google.com>

On Thu, Sep 17, 2020 at 11:35:40AM +0000, George Popescu wrote:
> On Thu, Sep 17, 2020 at 08:37:07AM +0200, Marco Elver wrote:
> > So, it seems that local-bounds can still catch some rare OOB accesses,
> > where KASAN fails to catch it because the access might skip over the
> > redzone.
> > 
> > The other more interesting bit of history is that
> > -fsanitize=local-bounds used to be -fbounds-checking, and meant for
> > production use as a hardening feature:
> > http://lists.llvm.org/pipermail/llvm-dev/2012-May/049972.html
> > 
> > And local-bounds just does not behave like any other sanitizer as a
> > result, it just traps. The fact that it's enabled via
> > -fsanitize=local-bounds (or just bounds) but hasn't much changed in
> > behaviour is a little unfortunate.
> 
> > I suppose there are 3 options:
> > 
> > 1. George implements trap handling somehow. Is this feasible? If not,
> > why not? Maybe that should also have been explained in the commit
> > message.
> > 
> > 2. Only enable -fsanitize=local-bounds if UBSAN_TRAP was selected, at
> > least for as long as Clang traps for local-bounds. I think this makes
> > sense either way, because if we do not expect UBSAN to trap, it really
> > should not trap!
> > 
> > 3. Change the compiler. As always, this will take a while to implement
> > and then to reach whoever should have that updated compiler.
> > 
> > Preferences?
> Considering of what you said above, I find option 2 the most elegant.
> The first one doesn't sound doable for the moment, also the third.
> I will edit this patch considering your comments and resend it to the
> list.

I have a slightly different suggestion that is very nearly #2 above:
split local-bounds into a separate CONFIG that requires UBSAN_TRAP, and
then carefully document both:
- what does it catch that "bounds" doesn't
- why it only operates in trap mode

The rationale I have is that I don't like the coverage of some
mitigation or detection to "silently" vary between builds. e.g. someone
would build with/without UBSAN_TRAP and end up with unexpectedly
different coverage. I'd rather there be a separate CONFIG that appears.

-- 
Kees Cook

WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: George Popescu <georgepope@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Alexander Potapenko <glider@google.com>,
	Will Deacon <will@kernel.org>,
	kvmarm@lists.cs.columbia.edu, Fangrui Song <maskray@google.com>,
	maz@kernel.org, Masahiro Yamada <masahiroy@kernel.org>,
	kasan-dev <kasan-dev@googlegroups.com>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Marco Elver <elver@google.com>, Arnd Bergmann <arnd@arndb.de>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Andrey Konovalov <andreyknvl@google.com>,
	broonie@kernel.org, Nathan Chancellor <natechancellor@gmail.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Michal Marek <michal.lkml@markovi.net>,
	Nick Desaulniers <ndesaulniers@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 06/14] Fix CFLAGS for UBSAN_BOUNDS on Clang
Date: Thu, 17 Sep 2020 15:21:47 -0700	[thread overview]
Message-ID: <202009171519.951D26DB@keescook> (raw)
In-Reply-To: <20200917113540.GA1742660@google.com>

On Thu, Sep 17, 2020 at 11:35:40AM +0000, George Popescu wrote:
> On Thu, Sep 17, 2020 at 08:37:07AM +0200, Marco Elver wrote:
> > So, it seems that local-bounds can still catch some rare OOB accesses,
> > where KASAN fails to catch it because the access might skip over the
> > redzone.
> > 
> > The other more interesting bit of history is that
> > -fsanitize=local-bounds used to be -fbounds-checking, and meant for
> > production use as a hardening feature:
> > http://lists.llvm.org/pipermail/llvm-dev/2012-May/049972.html
> > 
> > And local-bounds just does not behave like any other sanitizer as a
> > result, it just traps. The fact that it's enabled via
> > -fsanitize=local-bounds (or just bounds) but hasn't much changed in
> > behaviour is a little unfortunate.
> 
> > I suppose there are 3 options:
> > 
> > 1. George implements trap handling somehow. Is this feasible? If not,
> > why not? Maybe that should also have been explained in the commit
> > message.
> > 
> > 2. Only enable -fsanitize=local-bounds if UBSAN_TRAP was selected, at
> > least for as long as Clang traps for local-bounds. I think this makes
> > sense either way, because if we do not expect UBSAN to trap, it really
> > should not trap!
> > 
> > 3. Change the compiler. As always, this will take a while to implement
> > and then to reach whoever should have that updated compiler.
> > 
> > Preferences?
> Considering of what you said above, I find option 2 the most elegant.
> The first one doesn't sound doable for the moment, also the third.
> I will edit this patch considering your comments and resend it to the
> list.

I have a slightly different suggestion that is very nearly #2 above:
split local-bounds into a separate CONFIG that requires UBSAN_TRAP, and
then carefully document both:
- what does it catch that "bounds" doesn't
- why it only operates in trap mode

The rationale I have is that I don't like the coverage of some
mitigation or detection to "silently" vary between builds. e.g. someone
would build with/without UBSAN_TRAP and end up with unexpectedly
different coverage. I'd rather there be a separate CONFIG that appears.

-- 
Kees Cook
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: George Popescu <georgepope@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Alexander Potapenko <glider@google.com>,
	Will Deacon <will@kernel.org>,
	kvmarm@lists.cs.columbia.edu, Fangrui Song <maskray@google.com>,
	maz@kernel.org, Masahiro Yamada <masahiroy@kernel.org>,
	suzuki.poulose@arm.com, kasan-dev <kasan-dev@googlegroups.com>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	David Brazdil <dbrazdil@google.com>,
	julien.thierry.kdev@gmail.com, Marco Elver <elver@google.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Andrey Konovalov <andreyknvl@google.com>,
	broonie@kernel.org, Andrew Scull <ascull@google.com>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Michal Marek <michal.lkml@markovi.net>,
	Nick Desaulniers <ndesaulniers@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	james.morse@arm.com, Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 06/14] Fix CFLAGS for UBSAN_BOUNDS on Clang
Date: Thu, 17 Sep 2020 15:21:47 -0700	[thread overview]
Message-ID: <202009171519.951D26DB@keescook> (raw)
In-Reply-To: <20200917113540.GA1742660@google.com>

On Thu, Sep 17, 2020 at 11:35:40AM +0000, George Popescu wrote:
> On Thu, Sep 17, 2020 at 08:37:07AM +0200, Marco Elver wrote:
> > So, it seems that local-bounds can still catch some rare OOB accesses,
> > where KASAN fails to catch it because the access might skip over the
> > redzone.
> > 
> > The other more interesting bit of history is that
> > -fsanitize=local-bounds used to be -fbounds-checking, and meant for
> > production use as a hardening feature:
> > http://lists.llvm.org/pipermail/llvm-dev/2012-May/049972.html
> > 
> > And local-bounds just does not behave like any other sanitizer as a
> > result, it just traps. The fact that it's enabled via
> > -fsanitize=local-bounds (or just bounds) but hasn't much changed in
> > behaviour is a little unfortunate.
> 
> > I suppose there are 3 options:
> > 
> > 1. George implements trap handling somehow. Is this feasible? If not,
> > why not? Maybe that should also have been explained in the commit
> > message.
> > 
> > 2. Only enable -fsanitize=local-bounds if UBSAN_TRAP was selected, at
> > least for as long as Clang traps for local-bounds. I think this makes
> > sense either way, because if we do not expect UBSAN to trap, it really
> > should not trap!
> > 
> > 3. Change the compiler. As always, this will take a while to implement
> > and then to reach whoever should have that updated compiler.
> > 
> > Preferences?
> Considering of what you said above, I find option 2 the most elegant.
> The first one doesn't sound doable for the moment, also the third.
> I will edit this patch considering your comments and resend it to the
> list.

I have a slightly different suggestion that is very nearly #2 above:
split local-bounds into a separate CONFIG that requires UBSAN_TRAP, and
then carefully document both:
- what does it catch that "bounds" doesn't
- why it only operates in trap mode

The rationale I have is that I don't like the coverage of some
mitigation or detection to "silently" vary between builds. e.g. someone
would build with/without UBSAN_TRAP and end up with unexpectedly
different coverage. I'd rather there be a separate CONFIG that appears.

-- 
Kees Cook

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-09-17 22:21 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-14 17:27 [PATCH 00/14] UBSan Enablement for hyp/nVHE code George-Aurelian Popescu
2020-09-14 17:27 ` George-Aurelian Popescu
2020-09-14 17:27 ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 01/14] KVM: arm64: Enable UBSan instrumentation in nVHE hyp code George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 02/14] KVM: arm64: Define a macro for storing a value inside a per_cpu variable George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 03/14] KVM: arm64: Add support for creating and checking a logging buffer inside hyp/nVHE George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-10-01 10:07   ` Andrew Scull
2020-10-01 10:07     ` Andrew Scull
2020-10-01 10:07     ` Andrew Scull
2020-09-14 17:27 ` [PATCH 04/14] KVM: arm64: Add support for buffer usage George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 05/14] KVM: arm64: Define a buffer that can pass UBSan data from hyp/nVHE to kernel George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-15 13:25   ` George Popescu
2020-09-15 13:25     ` George Popescu
2020-09-15 13:25     ` George Popescu
2020-10-01 10:51   ` Andrew Scull
2020-10-01 10:51     ` Andrew Scull
2020-10-01 10:51     ` Andrew Scull
2020-09-14 17:27 ` [PATCH 06/14] Fix CFLAGS for UBSAN_BOUNDS on Clang George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 21:17   ` Nick Desaulniers
2020-09-14 21:17     ` Nick Desaulniers
2020-09-14 21:17     ` Nick Desaulniers
2020-09-14 22:13   ` Kees Cook
2020-09-14 22:13     ` Kees Cook
2020-09-14 22:13     ` Kees Cook
2020-09-15 10:24     ` George Popescu
2020-09-15 10:24       ` George Popescu
2020-09-15 10:24       ` George Popescu
2020-09-15 11:18       ` Marco Elver
2020-09-15 11:18         ` Marco Elver
2020-09-15 11:18         ` Marco Elver
2020-09-15 12:01         ` George Popescu
2020-09-15 12:01           ` George Popescu
2020-09-15 12:01           ` George Popescu
2020-09-15 17:32           ` Marco Elver
2020-09-15 17:32             ` Marco Elver
2020-09-15 17:32             ` Marco Elver
2020-09-16  7:40             ` George Popescu
2020-09-16  7:40               ` George Popescu
2020-09-16  7:40               ` George Popescu
2020-09-16  8:32               ` Marco Elver
2020-09-16  8:32                 ` Marco Elver
2020-09-16  8:32                 ` Marco Elver
2020-09-16 12:14                 ` George Popescu
2020-09-16 12:14                   ` George Popescu
2020-09-16 13:40                   ` Marco Elver
2020-09-16 13:40                     ` Marco Elver
2020-09-16 13:40                     ` Marco Elver
2020-09-17  6:37                     ` Marco Elver
2020-09-17  6:37                       ` Marco Elver
2020-09-17  6:37                       ` Marco Elver
2020-09-17 11:35                       ` George Popescu
2020-09-17 11:35                         ` George Popescu
2020-09-17 11:35                         ` George Popescu
2020-09-17 22:21                         ` Kees Cook [this message]
2020-09-17 22:21                           ` Kees Cook
2020-09-17 22:21                           ` Kees Cook
2020-09-17 22:17       ` Kees Cook
2020-09-17 22:17         ` Kees Cook
2020-09-17 22:17         ` Kees Cook
2020-09-14 17:27 ` [PATCH 07/14] KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-10-01 10:57   ` Andrew Scull
2020-10-01 10:57     ` Andrew Scull
2020-10-01 10:57     ` Andrew Scull
2020-09-14 17:27 ` [PATCH 08/14] KVM: arm64: Enable UBsan check for unreachable code inside hyp/nVHE code George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 09/14] KVM: arm64: Enable shift out of bounds undefined behaviour check for hyp/nVHE George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 10/14] KVM: arm64: __ubsan_handle_load_invalid_value hyp/nVHE implementation George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 11/14] KVM: arm64: Detect type mismatch undefined behaviour from hyp/nVHE code George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 12/14] KVM: arm64: Detect arithmetic overflow is inside hyp/nVHE George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 13/14] KVM: arm64: Enable the CONFIG_TEST UBSan for PKVM George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27 ` [PATCH 14/14] DO NOT MERGE: Enable configs to test the patch series George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu
2020-09-14 17:27   ` George-Aurelian Popescu

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=202009171519.951D26DB@keescook \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@google.com \
    --cc=arnd@arndb.de \
    --cc=ascull@google.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=dbrazdil@google.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=georgepope@google.com \
    --cc=glider@google.com \
    --cc=james.morse@arm.com \
    --cc=julien.thierry.kdev@gmail.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=maskray@google.com \
    --cc=maz@kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=natechancellor@gmail.com \
    --cc=ndesaulniers@google.com \
    --cc=suzuki.poulose@arm.com \
    --cc=tglx@linutronix.de \
    --cc=will@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.