linux-kernel - Re: [PATCH v2 1/3] ubsan: Add trap instrumentation option

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<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