linux-kernel - [PATCH] structleak: disable BYREF_ALL in combination with KASAN

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-next>] [day] [month] [year] [list]

Date:   Tue, 18 Jun 2019 11:47:13 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Kees Cook <keescook@...omium.org>
Cc:     Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>, kasan-dev@...glegroups.com,
        Alexander Popov <alex.popov@...ux.com>,
        Arnd Bergmann <arnd@...db.de>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] structleak: disable BYREF_ALL in combination with KASAN_STACK

The combination of KASAN_STACK and GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
leads to much larger kernel stack usage, as seen from the warnings
about functions that now exceed the 2048 byte limit:

drivers/media/i2c/tvp5150.c:253:1: error: the frame size of 3936 bytes is larger than 2048 bytes
fs/ocfs2/dlm/dlmrecovery.c:737:1: error: the frame size of 2088 bytes is larger than 2048 bytes
fs/ocfs2/aops.c:1892:1: error: the frame size of 2088 bytes is larger than 2048 bytes
fs/ocfs2/namei.c:1677:1: error: the frame size of 2584 bytes is larger than 2048 bytes
fs/ocfs2/super.c:1186:1: error: the frame size of 2640 bytes is larger than 2048 bytes
fs/ocfs2/xattr.c:3678:1: error: the frame size of 2176 bytes is larger than 2048 bytes
net/bridge/br_netlink.c:1505:1: error: the frame size of 2448 bytes is larger than 2048 bytes
net/ieee802154/nl802154.c:548:1: error: the frame size of 2232 bytes is larger than 2048 bytes
net/wireless/nl80211.c:1726:1: error: the frame size of 2224 bytes is larger than 2048 bytes
net/wireless/nl80211.c:6472:1: error: the frame size of 2112 bytes is larger than 2048 bytes
net/wireless/nl80211.c:2357:1: error: the frame size of 4584 bytes is larger than 2048 bytes
net/wireless/nl80211.c:5108:1: error: the frame size of 2760 bytes is larger than 2048 bytes
drivers/media/tuners/r820t.c:1327:1: error: the frame size of 2816 bytes is larger than 2048 bytes

The warnings are distracting, and risking a kernel stack overflow is
generally not beneficial to performance, so it may be best to disallow
that particular combination. This can be done by turning off either
one. I picked the dependency in GCC_PLUGIN_STRUCTLEAK_BYREF_ALL, as
this option is designed to make uninitialized stack usage less harmful
when enabled on its own, but it also prevents KASAN from detecting those
cases in which it was in fact needed.

KASAN_STACK is currently implied by KASAN on gcc, but could be made a
user selectable option if we want to allow combining (non-stack) KASAN
wtih GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.

Note that it woult be possible to specifically address the files that
print the warning, but presumably the overall stack usage is still
significantly higher than in other configurations, so this would not
address the full problem.

I could not test this with CONFIG_INIT_STACK_ALL, which may or may not
suffer from a similar problem.

Fixes: 81a56f6dcd20 ("gcc-plugins: structleak: Generalize to all variable types")
Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
 security/Kconfig.hardening | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index c6cb2d9b2905..e742d1006a4b 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -73,6 +73,7 @@ choice
 	config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
 		bool "zero-init anything passed by reference (very strong)"
 		depends on GCC_PLUGINS
+		depends on !(KASAN && KASAN_STACK=1)
 		select GCC_PLUGIN_STRUCTLEAK
 		help
 		  Zero-initialize any stack variables that may be passed
@@ -80,6 +81,10 @@ choice
 		  initialized. This is intended to eliminate all classes
 		  of uninitialized stack variable exploits and information
 		  exposures.
+		  As a side-effect, this keeps a lot of variables on the
+		  stack that can otherwise be optimized out, so combining
+		  this with CONFIG_KASAN_STACK can lead to a stack overflow
+		  and is disallowed.
 
 	config INIT_STACK_ALL
 		bool "0xAA-init everything on the stack (strongest)"
-- 
2.20.0