Re: [PATCH 01/26] compiler: introduce noinline_for_kasan annotation
On Fri, Mar 3, 2017 at 3:30 PM, Arnd Bergmann <a...@arndb.de> wrote:
> On Fri, Mar 3, 2017 at 2:55 PM, Alexander Potapenko <gli...@google.com> wrote:
>> On Fri, Mar 3, 2017 at 2:50 PM, Andrey Ryabinin <aryabi...@virtuozzo.com>
>> wrote:
>
>>>> @@ -416,6 +416,17 @@ static __always_inline void
>>>> __write_once_size(volatile void *p, void *res, int s
>>>> */
>>>> #define noinline_for_stack noinline
>>>>
>>>> +/*
>>>> + * CONFIG_KASAN can lead to extreme stack usage with certain patterns when
>>>> + * one function gets inlined many times and each instance requires a stack
>>>> + * ckeck.
>>>> + */
>>>> +#ifdef CONFIG_KASAN
>>>> +#define noinline_for_kasan noinline __maybe_unused
>>>
>>>
>>> noinline_iff_kasan might be a better name. noinline_for_kasan gives the
>>> impression
>>> that we always noinline function for the sake of kasan, while
>>> noinline_iff_kasan
>>> clearly indicates that function is noinline only if kasan is used.
>
> Fine with me. I actually tried to come up with a name that implies that the
> symbol is actually "inline" (or even __always_inline_ without KASAN, but
> couldn't think of any good name for it.
>
>> FWIW we may be facing the same problem with other compiler-based
>> tools, e.g. KMSAN (which isn't there yet).
>> So it might be better to choose a macro name that doesn't use the name
>> "KASAN".
>> E.g. noinline_iff_memtool (or noinline_iff_memory_tool if that's not too
>> long).
>> WDYT?
>
> Would KMSAN also force local variables to be non-overlapping the way that
> asan-stack=1 and -fsanitize-address-use-after-scope do? As I understood it,
> KMSAN would add extra code for maintaining the uninit bits, but in an example
> like this
The thing is that KMSAN (and other tools that insert heavyweight
instrumentation) may cause heavy register spilling which will also
blow up the stack frames.
> int f(int *);
> static inline __attribute__((always_inline)) int g(void)
> {
> int i;
> f();
> return i;
> }
> int f(void)
> {
> return g()+g()+g()+g();
> }
>
> each of the four copies of 'i' could have the same location on the stack
> and get marked uninitialized again before calling f(). We only need
> noinline_for_kasan (whatever we end up calling that) for compiler
> features that force each instance of 'i' to have its own stack redzone.
>
> Arnd
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Re: [PATCH 01/26] compiler: introduce noinline_for_kasan annotation
On Fri, Mar 3, 2017 at 2:50 PM, Andrey Ryabinin <aryabi...@virtuozzo.com> wrote: > > > On 03/02/2017 07:38 PM, Arnd Bergmann wrote: >> When CONFIG_KASAN is set, we can run into some code that uses incredible >> amounts of kernel stack: >> >> drivers/staging/dgnc/dgnc_neo.c:1056:1: error: the frame size of 2 bytes >> is larger than 2048 bytes [-Werror=frame-larger-than=] >> drivers/media/i2c/cx25840/cx25840-core.c:4960:1: error: the frame size of >> 94000 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] >> drivers/media/dvb-frontends/stv090x.c:3430:1: error: the frame size of 5312 >> bytes is larger than 3072 bytes [-Werror=frame-larger-than=] >> >> This happens when a sanitizer uses stack memory each time an inline function >> gets called. This introduces a new annotation for those functions to make >> them either 'inline' or 'noinline' dependning on the CONFIG_KASAN symbol. >> >> Signed-off-by: Arnd Bergmann <a...@arndb.de> >> --- >> include/linux/compiler.h | 11 +++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/include/linux/compiler.h b/include/linux/compiler.h >> index f8110051188f..56b90897a459 100644 >> --- a/include/linux/compiler.h >> +++ b/include/linux/compiler.h >> @@ -416,6 +416,17 @@ static __always_inline void __write_once_size(volatile >> void *p, void *res, int s >> */ >> #define noinline_for_stack noinline >> >> +/* >> + * CONFIG_KASAN can lead to extreme stack usage with certain patterns when >> + * one function gets inlined many times and each instance requires a stack >> + * ckeck. >> + */ >> +#ifdef CONFIG_KASAN >> +#define noinline_for_kasan noinline __maybe_unused > > > noinline_iff_kasan might be a better name. noinline_for_kasan gives the > impression > that we always noinline function for the sake of kasan, while > noinline_iff_kasan > clearly indicates that function is noinline only if kasan is used. FWIW we may be facing the same problem with other compiler-based tools, e.g. KMSAN (which isn't there yet). So it might be better to choose a macro name that doesn't use the name "KASAN". E.g. noinline_iff_memtool (or noinline_iff_memory_tool if that's not too long). WDYT? >> +#else >> +#define noinline_for_kasan inline >> +#endif >> + >> #ifndef __always_inline >> #define __always_inline inline >> #endif >> > > -- > You received this message because you are subscribed to the Google Groups > "kasan-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to kasan-dev+unsubscr...@googlegroups.com. > To post to this group, send email to kasan-...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/kasan-dev/7e7a62de-3b79-6044-72fa-4ade418953d1%40virtuozzo.com. > For more options, visit https://groups.google.com/d/optout. -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
Re: [PATCH 00/26] bring back stack frame warning with KASAN
|2 +- > drivers/media/dvb-frontends/stv6110.c|2 +- > drivers/media/dvb-frontends/stv6110x.c |2 +- > drivers/media/dvb-frontends/tda8083.c|2 +- > drivers/media/dvb-frontends/zl10039.c|2 +- > drivers/media/i2c/adv7604.c |4 +- > drivers/media/i2c/cx25840/cx25840-core.c |4 +- > drivers/media/i2c/ks0127.c |2 +- > drivers/media/tuners/r820t.c |4 +- > drivers/media/tuners/tuner-i2c.h | 15 +- > drivers/media/usb/em28xx/em28xx-dvb.c| 947 > +-- > drivers/mtd/chips/cfi_cmdset_0020.c |8 +- > drivers/net/ethernet/rocker/rocker_tlv.h | 24 +- > drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c | 1860 > + > drivers/net/wireless/realtek/rtl818x/rtl8180/rtl8225se.c |4 +- > drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c | 41 +- > drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c | 26 +- > drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c | 34 +- > drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c | 36 +- > drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c | 38 +- > drivers/net/wireless/wl3501_cs.c | 10 +- > drivers/tty/vt/keyboard.c|6 +- > fs/ocfs2/cluster/masklog.c | 10 +- > fs/ocfs2/cluster/masklog.h |4 +- > fs/overlayfs/util.c |6 +- > include/linux/compiler.h | 58 ++- > include/linux/mtd/map.h |8 +- > include/linux/typecheck.h|7 +- > include/net/netlink.h| 36 +- > lib/Kconfig.debug |9 +- > lib/Kconfig.kasan| 11 +- > lib/Kconfig.kmemcheck|1 + > scripts/Makefile.kasan |3 + > 48 files changed, 1670 insertions(+), 1629 deletions(-) > -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
