All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/9] bring back stack frame warning with KASAN
@ 2017-09-22 21:29 ` Arnd Bergmann
  0 siblings, 0 replies; 58+ messages in thread
From: Arnd Bergmann @ 2017-09-22 21:29 UTC (permalink / raw)
  Cc: Arnd Bergmann, Mauro Carvalho Chehab, Jiri Pirko,
	Arend van Spriel, Kalle Valo, David S. Miller, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Masahiro Yamada,
	Michal Marek, Andrew Morton, Kees Cook, Geert Uytterhoeven,
	Greg Kroah-Hartman, linux-media, linux-kernel, netdev,
	linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
	kasan-dev, linux-kbuild, Jakub Jelinek, Martin Liška

This is a new version of patches I originally submitted back in March
[1], and last time in June [2]. This time I have basically rewritten
the entire patch series based on a new approach that came out of GCC
PR81715 that I opened[3]. The upcoming gcc-8 release is now much better
at consolidating stack slots for inline function arguments and would
obsolete most of my workaround patches here, but we still need the
workarounds for gcc-5, gcc-6 and gcc-7. Many thanks to Jakub Jelinek
for the analysis and the gcc-8 patch!

This minimal set of patches only makes sure that we do get frame size
warnings in allmodconfig for x86_64 and arm64 again with a 2048 byte
limit, even with KASAN enabled, but without the new KASAN_EXTRA option.

I set the warning limit with KASAN_EXTRA to 3072, limiting the
allmodconfig+KASAN_EXTRA build output to around 50 legitimate warnings.
These are for stack frames up to 31KB that will cause an immediate stack
overflow, and fixing them would require bringing back my older patches
and more. We can debate whether we want to apply those as a follow-up,
or instead remove the option entirely.

Another follow-up series I have reduces the warning limit with
KASAN to 1536, and without KASAN to 1280 for 64-bit architectures.

I hope we can get all patches merged for v4.14 and most of them
backported into stable kernels. Since we no longer have a dependency
on a preparation patch, my preference would be for the respective
subsystem maintainers to pick up the individual patches.
The last patch introduces a couple of "allmodconfig" build warnings
on x86 and arm64 unless the other patches get merged first, I'll
send that again separately once everything else has been taken
care of.

The remaining contents are:
- -fsanitize-address-use-after-scope is moved to a separate
  CONFIG_KASAN_EXTRA option that increases the warning limit
- CONFIG_KASAN_EXTRA is disabled with CONFIG_COMPILE_TEST,
  improving compile speed and disabling code that leads to
  valid warnings on gcc-7.0.1
- KMEMCHECK conflicts with CONFIG_KASAN
- my inline function workaround is applied to netlink, one
  ethernet driver and a few media drivers.
- The rework for the brcmsmac driver from previous versions is
  still there.

Changes since v3:
- I dropped all "noinline_if_stackbloat" annotations and used
  a workaround that introduces additional local variables in the inline
  functions to copy the function arguments, resulting in much better
  object code at the expense of having rather odd-looking functions.
- The v4 patches now don't help with KASAN_EXTRA any more at all,
  CONFIG_KASAN_EXTRA now depends on CONFIG_DEBUG_KERNEL, as it
  is more dangerous in production systems than it was before
- Rewrote the "em28xx" patch to be small enough for a stable backport.
- The rewritten vt-keyboard patches got merged and are now in
  stable kernels as well.

Changes since v2:
- rewrote the vt-keyboard patch based on feedback
- and made KMEMCHECK mutually exclusive with KASAN
  (rather than KASAN_EXTRA)

Changes since v1:
- dropped patches to fix all the CONFIG_KASAN_EXTRA warnings:
 - READ_ONCE/WRITE_ONCE cause problems in lots of code
 - typecheck() causes huge problems in a few places
 - many more uses of noinline_if_stackbloat

     Arnd

[1] https://www.spinics.net/lists/linux-wireless/msg159819.html
[2] https://www.spinics.net/lists/netdev/msg441918.html
[3] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715

Arnd Bergmann (9):
  brcmsmac: make some local variables 'static const' to reduce stack
    size
  brcmsmac: split up wlc_phy_workarounds_nphy
  brcmsmac: reindent split functions
  em28xx: fix em28xx_dvb_init for KASAN
  r820t: fix r820t_write_reg for KASAN
  dvb-frontends: fix i2c access helpers for KASAN
  rocker: fix rocker_tlv_put_* functions for KASAN
  netlink: fix nla_put_{u8,u16,u32} for KASAN
  kasan: rework Kconfig settings

 drivers/media/dvb-frontends/ascot2e.c              |    4 +-
 drivers/media/dvb-frontends/cxd2841er.c            |    4 +-
 drivers/media/dvb-frontends/helene.c               |    4 +-
 drivers/media/dvb-frontends/horus3a.c              |    4 +-
 drivers/media/dvb-frontends/itd1000.c              |    5 +-
 drivers/media/dvb-frontends/mt312.c                |    4 +-
 drivers/media/dvb-frontends/stb0899_drv.c          |    3 +-
 drivers/media/dvb-frontends/stb6100.c              |    6 +-
 drivers/media/dvb-frontends/stv0367.c              |    4 +-
 drivers/media/dvb-frontends/stv090x.c              |    4 +-
 drivers/media/dvb-frontends/stv6110x.c             |    4 +-
 drivers/media/dvb-frontends/zl10039.c              |    4 +-
 drivers/media/tuners/r820t.c                       |   13 +-
 drivers/media/usb/em28xx/em28xx-dvb.c              |   30 +-
 drivers/net/ethernet/rocker/rocker_tlv.h           |   48 +-
 .../broadcom/brcm80211/brcmsmac/phy/phy_n.c        | 1856 ++++++++++----------
 include/net/netlink.h                              |   73 +-
 lib/Kconfig.debug                                  |    4 +-
 lib/Kconfig.kasan                                  |   13 +-
 lib/Kconfig.kmemcheck                              |    1 +
 scripts/Makefile.kasan                             |    3 +
 21 files changed, 1047 insertions(+), 1044 deletions(-)

-- 
2.9.0

Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <mmarek@suse.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: brcm80211-dev-list.pdl@broadcom.com
Cc: brcm80211-dev-list@cypress.com
Cc: kasan-dev@googlegroups.com
Cc: linux-kbuild@vger.kernel.org
Cc: Jakub Jelinek <jakub@gcc.gnu.org>
Cc: Martin Liška <marxin@gcc.gnu.org>

^ permalink raw reply	[flat|nested] 58+ messages in thread
* [PATCH] string.h: work around for increased stack usage
@ 2017-12-05 21:51 Arnd Bergmann
  2017-12-05 22:02 ` Andrew Morton
  0 siblings, 1 reply; 58+ messages in thread
From: Arnd Bergmann @ 2017-12-05 21:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Mauro Carvalho Chehab, linux-media, kasan-dev,
	Dmitry Vyukov, Alexander Potapenko, Andrey Ryabinin,
	linux-kbuild, Arnd Bergmann, stable, Daniel Micay,
	Greg Kroah-Hartman, Martin Wilck, Dan Williams, linux-kernel

The hardened strlen() function causes rather large stack usage in at
least one file in the kernel, in particular when CONFIG_KASAN is enabled:

drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init':
drivers/media/usb/em28xx/em28xx-dvb.c:2062:1: error: the frame size of 3256 bytes is larger than 204 bytes [-Werror=frame-larger-than=]

Analyzing this problem led to the discovery that gcc fails to merge the
stack slots for the i2c_board_info[] structures after we strlcpy() into
them, due to the 'noreturn' attribute on the source string length check.

I reported this as a gcc bug, but it is unlikely to get fixed for gcc-8,
since it is relatively easy to work around, and it gets triggered rarely.
An earlier workaround I did added an empty inline assembly statement
before the call to fortify_panic(), which works surprisingly well,
but is really ugly and unintuitive.

This is a new approach to the same problem, this time addressing it by
not calling the 'extern __real_strnlen()' function for string constants
where __builtin_strlen() is a compile-time constant and therefore known
to be safe. We do this by checking if the last character in the string
is a compile-time constant '\0'. If it is, we can assume that
strlen() of the string is also constant. As a side-effect, this should
also improve the object code output for any other call of strlen()
on a string constant.

Cc: stable@vger.kernel.org
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
Link: https://patchwork.kernel.org/patch/9980413/
Link: https://patchwork.kernel.org/patch/9974047/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v3: don't use an asm barrier but use a constant string change.

Aside from two other patches for drivers/media that I sent last week,
this should fix all stack frames above 2KB, once all three are merged,
I'll send the patch to re-enable the warning.
---
 include/linux/string.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 410ecf17de3c..e5cc3f27f6e0 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -259,7 +259,8 @@ __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
 {
 	__kernel_size_t ret;
 	size_t p_size = __builtin_object_size(p, 0);
-	if (p_size == (size_t)-1)
+	if (p_size == (size_t)-1 ||
+	    (__builtin_constant_p(p[p_size - 1]) && p[p_size - 1] == '\0'))
 		return __builtin_strlen(p);
 	ret = strnlen(p, p_size);
 	if (p_size <= ret)
-- 
2.9.0

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

end of thread, other threads:[~2017-12-05 22:02 UTC | newest]

Thread overview: 58+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-22 21:29 [PATCH v4 0/9] bring back stack frame warning with KASAN Arnd Bergmann
2017-09-22 21:29 ` Arnd Bergmann
2017-09-22 21:29 ` Arnd Bergmann
2017-09-22 21:29 ` Arnd Bergmann
2017-09-22 21:29 ` [PATCH v4 1/9] brcmsmac: make some local variables 'static const' to reduce stack size Arnd Bergmann
2017-09-22 21:29   ` Arnd Bergmann
2017-09-25  4:33   ` Kalle Valo
2017-09-25  4:33     ` Kalle Valo
2017-10-02 13:53   ` [v4, " Kalle Valo
2017-10-02 13:53     ` Kalle Valo
2017-10-02 13:53     ` Kalle Valo
2017-09-22 21:29 ` [PATCH v4 2/9] brcmsmac: split up wlc_phy_workarounds_nphy Arnd Bergmann
2017-09-22 21:29   ` Arnd Bergmann
2017-10-02 13:55   ` [v4,2/9] " Kalle Valo
2017-10-02 13:55     ` Kalle Valo
2017-10-02 13:55     ` Kalle Valo
2017-10-27  7:51   ` Kalle Valo
2017-10-27  7:51     ` Kalle Valo
2017-10-27  7:51     ` Kalle Valo
2017-09-22 21:29 ` [PATCH v4 3/9] brcmsmac: reindent split functions Arnd Bergmann
2017-09-22 21:29   ` Arnd Bergmann
2017-09-22 21:29 ` [PATCH v4 4/9] em28xx: fix em28xx_dvb_init for KASAN Arnd Bergmann
2017-09-22 21:29   ` Arnd Bergmann
2017-09-25 14:41   ` David Laight
2017-09-25 14:41     ` David Laight
2017-09-26  6:32     ` Arnd Bergmann
2017-09-26  6:32       ` Arnd Bergmann
2017-09-26  6:47       ` Arnd Bergmann
2017-09-26  6:47         ` Arnd Bergmann
2017-09-26 16:49         ` Andrey Ryabinin
2017-09-26 16:49           ` Andrey Ryabinin
2017-09-27 13:26           ` Arnd Bergmann
2017-09-27 13:26             ` Arnd Bergmann
2017-09-28 13:09             ` Andrey Ryabinin
2017-09-28 13:09               ` Andrey Ryabinin
2017-09-28 14:30               ` Arnd Bergmann
2017-09-28 14:30                 ` Arnd Bergmann
2017-10-02  8:33                 ` Arnd Bergmann
2017-10-02  8:33                   ` Arnd Bergmann
2017-10-02  8:40                   ` [PATCH] string.h: work around for increased stack usage Arnd Bergmann
2017-10-02  9:02                     ` Arnd Bergmann
2017-10-02 14:07                     ` Andrey Ryabinin
2017-10-03 18:10                     ` kbuild test robot
2017-10-03 18:10                       ` kbuild test robot
2017-09-22 21:29 ` [PATCH v4 5/9] r820t: fix r820t_write_reg for KASAN Arnd Bergmann
2017-09-22 21:29   ` Arnd Bergmann
2017-09-22 21:29 ` [PATCH v4 6/9] dvb-frontends: fix i2c access helpers " Arnd Bergmann
2017-09-22 21:29   ` Arnd Bergmann
2017-09-22 21:29 ` [PATCH v4 7/9] rocker: fix rocker_tlv_put_* functions " Arnd Bergmann
2017-09-22 21:29   ` Arnd Bergmann
2017-09-26  3:19   ` David Miller
2017-09-22 21:29 ` [PATCH v4 8/9] netlink: fix nla_put_{u8,u16,u32} " Arnd Bergmann
2017-09-22 21:29   ` Arnd Bergmann
2017-09-26  3:19   ` David Miller
2017-09-22 21:29 ` [PATCH v4 9/9] kasan: rework Kconfig settings Arnd Bergmann
2017-09-26 19:36   ` Andrey Ryabinin
2017-12-05 21:51 [PATCH] string.h: work around for increased stack usage Arnd Bergmann
2017-12-05 22:02 ` Andrew Morton

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.