linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] ARM: fix kasan link failures
@ 2019-07-03 20:54 Arnd Bergmann
  2019-07-03 20:54 ` [PATCH 2/3] kasan: disable CONFIG_KASAN_STACK with clang on arm32 Arnd Bergmann
  2019-07-03 20:54 ` [PATCH 3/3] kasan: increase 32-bit stack frame warning limit Arnd Bergmann
  0 siblings, 2 replies; 4+ messages in thread
From: Arnd Bergmann @ 2019-07-03 20:54 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrey Ryabinin, Abbott Liu, linux-arm-kernel, kasan-dev,
	Linus Walleij, Arnd Bergmann, Ard Biesheuvel, Nicolas Pitre,
	Stefan Agner, Nathan Chancellor, Masahiro Yamada, linux-kernel,
	linux-efi

Getting the redirects for memcpy/memmove/memset functions right
in the decompressor and the efi stub is a bit tricky. Originally
these were meant to prevent the kasan code from calling itself
recursively. The decompressor is built without kasan but uses
the same redirects when CONFIG_KASAN is enabled, except in a few
cases that now cause link failures:

arch/arm/boot/compressed/fdt_rw.o: In function `fdt_set_name':
fdt_rw.c:(.text+0x3d4): undefined reference to `memcpy'
arch/arm/boot/compressed/fdt_rw.o: In function `fdt_add_property_':
fdt_rw.c:(.text+0x121c): undefined reference to `memmove'
arch/arm/boot/compressed/fdt_rw.o: In function `fdt_splice_':
fdt_rw.c:(.text+0x1460): undefined reference to `memmove'
arch/arm/boot/compressed/fdt_ro.o: In function `fdt_get_path':
fdt_ro.c:(.text+0x1384): undefined reference to `memcpy'
arch/arm/boot/compressed/fdt_wip.o: In function `fdt_setprop_inplace_namelen_partial':
fdt_wip.c:(.text+0x48): undefined reference to `memcpy'
arch/arm/boot/compressed/fdt_wip.o: In function `fdt_setprop_inplace':
fdt_wip.c:(.text+0x100): undefined reference to `memcpy'
arch/arm/boot/compressed/fdt.o: In function `fdt_move':
fdt.c:(.text+0xa04): undefined reference to `memmove'
arch/arm/boot/compressed/atags_to_fdt.o: In function `atags_to_fdt':
atags_to_fdt.c:(.text+0x404): undefined reference to `memcpy'
atags_to_fdt.c:(.text+0x450): undefined reference to `memcpy'

I tried to make everything use them, but ran into other problems:

drivers/firmware/efi/libstub/lib-fdt_sw.stub.o: In function `fdt_create_with_flags':
fdt_sw.c:(.text+0x34): undefined reference to `__memset'
arch/arm/boot/compressed/decompress.o: In function `lzo1x_decompress_safe':
decompress.c:(.text+0x290): undefined reference to `__memset'

This makes all the early boot code not use the redirects, which
works because we don't sanitize that code.

Setting -D__SANITIZE_ADDRESS__ is a bit confusing here, but it
does the trick.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/boot/compressed/Makefile     | 1 +
 arch/arm/boot/compressed/decompress.c | 2 --
 arch/arm/boot/compressed/libfdt_env.h | 2 --
 drivers/firmware/efi/libstub/Makefile | 3 ++-
 4 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index dcc27fb24fbb..d91c2ded0e3d 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -25,6 +25,7 @@ endif
 
 GCOV_PROFILE		:= n
 KASAN_SANITIZE		:= n
+CFLAGS_KERNEL += -D__SANITIZE_ADDRESS__
 
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT		:= n
diff --git a/arch/arm/boot/compressed/decompress.c b/arch/arm/boot/compressed/decompress.c
index 3794fae5f818..aa075d8372ea 100644
--- a/arch/arm/boot/compressed/decompress.c
+++ b/arch/arm/boot/compressed/decompress.c
@@ -47,10 +47,8 @@ extern char * strchrnul(const char *, int);
 #endif
 
 #ifdef CONFIG_KERNEL_XZ
-#ifndef CONFIG_KASAN
 #define memmove memmove
 #define memcpy memcpy
-#endif
 #include "../../../../lib/decompress_unxz.c"
 #endif
 
diff --git a/arch/arm/boot/compressed/libfdt_env.h b/arch/arm/boot/compressed/libfdt_env.h
index 8091efc21407..b36c0289a308 100644
--- a/arch/arm/boot/compressed/libfdt_env.h
+++ b/arch/arm/boot/compressed/libfdt_env.h
@@ -19,6 +19,4 @@ typedef __be64 fdt64_t;
 #define fdt64_to_cpu(x)		be64_to_cpu(x)
 #define cpu_to_fdt64(x)		cpu_to_be64(x)
 
-#undef memset
-
 #endif
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 0460c7581220..fd1d72ea04dd 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -20,7 +20,8 @@ cflags-$(CONFIG_ARM64)		:= $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
 				   -fpie $(DISABLE_STACKLEAK_PLUGIN)
 cflags-$(CONFIG_ARM)		:= $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
 				   -fno-builtin -fpic \
-				   $(call cc-option,-mno-single-pic-base)
+				   $(call cc-option,-mno-single-pic-base) \
+				   -D__SANITIZE_ADDRESS__
 
 cflags-$(CONFIG_EFI_ARMSTUB)	+= -I$(srctree)/scripts/dtc/libfdt
 
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] kasan: disable CONFIG_KASAN_STACK with clang on arm32
  2019-07-03 20:54 [PATCH 1/3] ARM: fix kasan link failures Arnd Bergmann
@ 2019-07-03 20:54 ` Arnd Bergmann
  2019-07-08 14:32   ` Linus Walleij
  2019-07-03 20:54 ` [PATCH 3/3] kasan: increase 32-bit stack frame warning limit Arnd Bergmann
  1 sibling, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2019-07-03 20:54 UTC (permalink / raw)
  To: Florian Fainelli, Andrey Ryabinin
  Cc: Abbott Liu, linux-arm-kernel, kasan-dev, Linus Walleij,
	Arnd Bergmann, Alexander Potapenko, Dmitry Vyukov,
	Masahiro Yamada, Michal Marek, Andrew Morton, Andrey Konovalov,
	Will Deacon, linux-kbuild, linux-kernel, clang-built-linux

The CONFIG_KASAN_STACK symbol tells us whether we should be using the
asan-stack=1 parameter. On clang-8, this causes explosive kernel stack
frame growth, so it is currently disabled, hopefully to be turned back
on when a future clang version is fixed. Examples include

drivers/media/dvb-frontends/mb86a20s.c:1942:12: error: stack frame size of 4128 bytes in function
drivers/net/wireless/atmel/atmel.c:1307:5: error: stack frame size of 4928 bytes in function 'atmel_open'
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c:1521:1: error: stack frame size of 5440 bytes in function
drivers/media/i2c/mt9t112.c:670:12: error: stack frame size of 9344 bytes in function 'mt9t112_init_camera'
drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c:185:12: error: stack frame size of 10048 bytes

For the 32-bit ARM build, the logic I introduced earlier does
not work because $(CFLAGS_KASAN_SHADOW) is empty, and we don't add
those flags.

Moving the asan-stack= parameter down fixes this. No idea of any
of the other parameters should also be moved though.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 scripts/Makefile.kasan | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index 6410bd22fe38..fc57fcf49722 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -26,10 +26,11 @@ else
 	CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
 	 $(call cc-param,asan-globals=1) \
 	 $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
-	 $(call cc-param,asan-stack=$(CONFIG_KASAN_STACK)) \
 	 $(call cc-param,asan-instrument-allocas=1)
 endif
 
+CFLAGS_KASAN += $(call cc-param,asan-stack=$(CONFIG_KASAN_STACK))
+
 endif # CONFIG_KASAN_GENERIC
 
 ifdef CONFIG_KASAN_SW_TAGS
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] kasan: increase 32-bit stack frame warning limit
  2019-07-03 20:54 [PATCH 1/3] ARM: fix kasan link failures Arnd Bergmann
  2019-07-03 20:54 ` [PATCH 2/3] kasan: disable CONFIG_KASAN_STACK with clang on arm32 Arnd Bergmann
@ 2019-07-03 20:54 ` Arnd Bergmann
  1 sibling, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2019-07-03 20:54 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrey Ryabinin, Abbott Liu, linux-arm-kernel, kasan-dev,
	Linus Walleij, Arnd Bergmann, Andrew Morton, Masahiro Yamada,
	Kees Cook, linux-kernel

Enabling kasan on 32-bit ARM introduces some new warnings in the
allmodconfig build due to mildly increased kernel stack usage, even when
asan-stack is disabled:

fs/select.c:621:5: error: stack frame size of 1032 bytes in function 'core_sys_select'
net/mac80211/mlme.c:4047:6: error: stack frame size of 1032 bytes in function 'ieee80211_sta_rx_queued_mgmt'
drivers/infiniband/sw/rxe/rxe_req.c:583:5: error: stack frame size of 1152 bytes in function 'rxe_requester'
fs/ubifs/replay.c:1193:5: error: stack frame size of 1152 bytes in function 'ubifs_replay_journal'
drivers/mtd/chips/cfi_cmdset_0001.c:1868:12: error: stack frame size of 1104 bytes in function 'cfi_intelext_writev'
drivers/ntb/hw/idt/ntb_hw_idt.c:1041:27: error: stack frame size of 1032 bytes in function 'idt_scan_mws'
drivers/mtd/nftlcore.c:674:12: error: stack frame size of 1120 bytes in function 'nftl_writeblock'
drivers/net/wireless/cisco/airo.c:3793:12: error: stack frame size of 1040 bytes in function 'setup_card'
drivers/staging/fbtft/fbtft-core.c:989:5: error: stack frame size of 1232 bytes in function 'fbtft_init_display'
drivers/staging/fbtft/fbtft-core.c:907:12: error: stack frame size of 1072 bytes in function 'fbtft_init_display_dt'
drivers/staging/wlan-ng/cfg80211.c:272:12: error: stack frame size of 1040 bytes in function 'prism2_scan'

Some of these are intentionally high, others are from sloppy coding
practice and should perhaps be reduced a lot.

For 64-bit, the limit is currently much higher at 2048 bytes, which
does not cause many warnings and could even be reduced. Changing the
limit to 1280 bytes with KASAN also takes care of all cases I see.
If we go beyond that with KASAN, or over the normal 1024 byte limit
without it, that is however something we should definitely address
in the code.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 lib/Kconfig.debug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 6d2799190fba..41b0ae9d05d9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -251,7 +251,7 @@ config FRAME_WARN
 	int "Warn for stack frames larger than (needs gcc 4.4)"
 	range 0 8192
 	default 2048 if GCC_PLUGIN_LATENT_ENTROPY
-	default 1280 if (!64BIT && PARISC)
+	default 1280 if (!64BIT && (PARISC || KASAN))
 	default 1024 if (!64BIT && !PARISC)
 	default 2048 if 64BIT
 	help
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/3] kasan: disable CONFIG_KASAN_STACK with clang on arm32
  2019-07-03 20:54 ` [PATCH 2/3] kasan: disable CONFIG_KASAN_STACK with clang on arm32 Arnd Bergmann
@ 2019-07-08 14:32   ` Linus Walleij
  0 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2019-07-08 14:32 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Florian Fainelli, Andrey Ryabinin, Abbott Liu, Linux ARM,
	kasan-dev, Alexander Potapenko, Dmitry Vyukov, Masahiro Yamada,
	Michal Marek, Andrew Morton, Andrey Konovalov, Will Deacon,
	linux-kbuild, linux-kernel, clang-built-linux

On Wed, Jul 3, 2019 at 10:56 PM Arnd Bergmann <arnd@arndb.de> wrote:

> The CONFIG_KASAN_STACK symbol tells us whether we should be using the
> asan-stack=1 parameter. On clang-8, this causes explosive kernel stack
> frame growth, so it is currently disabled, hopefully to be turned back
> on when a future clang version is fixed. Examples include
>
> drivers/media/dvb-frontends/mb86a20s.c:1942:12: error: stack frame size of 4128 bytes in function
> drivers/net/wireless/atmel/atmel.c:1307:5: error: stack frame size of 4928 bytes in function 'atmel_open'
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c:1521:1: error: stack frame size of 5440 bytes in function
> drivers/media/i2c/mt9t112.c:670:12: error: stack frame size of 9344 bytes in function 'mt9t112_init_camera'
> drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c:185:12: error: stack frame size of 10048 bytes
>
> For the 32-bit ARM build, the logic I introduced earlier does
> not work because $(CFLAGS_KASAN_SHADOW) is empty, and we don't add
> those flags.
>
> Moving the asan-stack= parameter down fixes this. No idea of any
> of the other parameters should also be moved though.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

For some reason the RealView doesn't boot after this patch. Trying to figure
out why.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-07-08 14:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-03 20:54 [PATCH 1/3] ARM: fix kasan link failures Arnd Bergmann
2019-07-03 20:54 ` [PATCH 2/3] kasan: disable CONFIG_KASAN_STACK with clang on arm32 Arnd Bergmann
2019-07-08 14:32   ` Linus Walleij
2019-07-03 20:54 ` [PATCH 3/3] kasan: increase 32-bit stack frame warning limit Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).