[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191216102655.GA11082@willie-the-truck>
Date: Mon, 16 Dec 2019 10:26:56 +0000
From: Will Deacon <will@...nel.org>
To: Kees Cook <keescook@...omium.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Andrey Ryabinin <aryabinin@...tuozzo.com>,
Elena Petrova <lenaptr@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Dan Carpenter <dan.carpenter@...cle.com>,
"Gustavo A. R. Silva" <gustavo@...eddedor.com>,
Arnd Bergmann <arnd@...db.de>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
kasan-dev@...glegroups.com, linux-kernel@...r.kernel.org,
kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH v2 1/3] ubsan: Add trap instrumentation option
Hi Kees,
On Thu, Nov 21, 2019 at 10:15:17AM -0800, Kees Cook wrote:
> The Undefined Behavior Sanitizer can operate in two modes: warning
> reporting mode via lib/ubsan.c handler calls, or trap mode, which uses
> __builtin_trap() as the handler. Using lib/ubsan.c means the kernel
> image is about 5% larger (due to all the debugging text and reporting
> structures to capture details about the warning conditions). Using the
> trap mode, the image size changes are much smaller, though at the loss
> of the "warning only" mode.
>
> In order to give greater flexibility to system builders that want
> minimal changes to image size and are prepared to deal with kernel code
> being aborted and potentially destabilizing the system, this introduces
> CONFIG_UBSAN_TRAP. The resulting image sizes comparison:
>
> text data bss dec hex filename
> 19533663 6183037 18554956 44271656 2a38828 vmlinux.stock
> 19991849 7618513 18874448 46484810 2c54d4a vmlinux.ubsan
> 19712181 6284181 18366540 44362902 2a4ec96 vmlinux.ubsan-trap
>
> CONFIG_UBSAN=y: image +4.8% (text +2.3%, data +18.9%)
> CONFIG_UBSAN_TRAP=y: image +0.2% (text +0.9%, data +1.6%)
>
> Additionally adjusts the CONFIG_UBSAN Kconfig help for clarity and
> removes the mention of non-existing boot param "ubsan_handle".
>
> Suggested-by: Elena Petrova <lenaptr@...gle.com>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
> lib/Kconfig.ubsan | 22 ++++++++++++++++++----
> lib/Makefile | 2 ++
> scripts/Makefile.ubsan | 9 +++++++--
> 3 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index 0e04fcb3ab3d..9deb655838b0 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -5,11 +5,25 @@ config ARCH_HAS_UBSAN_SANITIZE_ALL
> config UBSAN
> bool "Undefined behaviour sanity checker"
> help
> - This option enables undefined behaviour sanity checker
> + This option enables the Undefined Behaviour sanity checker.
> Compile-time instrumentation is used to detect various undefined
> - behaviours in runtime. Various types of checks may be enabled
> - via boot parameter ubsan_handle
> - (see: Documentation/dev-tools/ubsan.rst).
> + behaviours at runtime. For more details, see:
> + Documentation/dev-tools/ubsan.rst
> +
> +config UBSAN_TRAP
> + bool "On Sanitizer warnings, abort the running kernel code"
> + depends on UBSAN
> + depends on $(cc-option, -fsanitize-undefined-trap-on-error)
> + help
> + Building kernels with Sanitizer features enabled tends to grow
> + the kernel size by around 5%, due to adding all the debugging
> + text on failure paths. To avoid this, Sanitizer instrumentation
> + can just issue a trap. This reduces the kernel size overhead but
> + turns all warnings (including potentially harmless conditions)
> + into full exceptions that abort the running kernel code
> + (regardless of context, locks held, etc), which may destabilize
> + the system. For some system builders this is an acceptable
> + trade-off.
Slight nit, but I wonder if it would make sense to move all this under a
'menuconfig UBSAN' entry, so the dependencies can be dropped? Then you could
have all of the suboptions default to on and basically choose which
individual compiler options to disable based on your own preferences.
What do you think?
Will
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
