[PATCH] kasan: don't emit builtin calls when sanitization is off

16 views
Skip to first unread message

Andrey Konovalov

unread,
Jan 19, 2018, 9:54:10 AM1/19/18
to Masahiro Yamada, Michal Marek, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, linux-...@vger.kernel.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Kostya Serebryany, Evgeniy Stepanov, Nick Desaulniers, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrey Konovalov
With KASAN enabled the kernel has two different memset() functions, one
with KASAN checks (memset) and one without (__memset). KASAN uses some
macro tricks to use the proper version where required. For example memset()
calls in mm/slub.c are without KASAN checks, since they operate on poisoned
slab object metadata.

The issue is that clang emits memset() calls even when there is no memset()
in the source code. They get linked with improper memset() implementation
and the kernel fails to boot due to a huge amount of KASAN reports during
early boot stages.

The solution is to add -fno-builtin flag for files with KASAN_SANITIZE := n
marker.

Signed-off-by: Andrey Konovalov <andre...@google.com>
---

This patch should be applied on top of the v4 "kasan: support alloca, LLVM"
patchset from Paul Lawrence.

Makefile | 3 ++-
scripts/Makefile.kasan | 3 +++
scripts/Makefile.lib | 2 +-
3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index bf5b8cbb9469..d45e31b293d0 100644
--- a/Makefile
+++ b/Makefile
@@ -432,7 +432,8 @@ export MAKE AWK GENKSYMS INSTALLKERNEL PERL PYTHON UTS_MACHINE
export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS

export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
-export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE CFLAGS_KASAN CFLAGS_UBSAN
+export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
+export CFLAGS_KASAN CFLAGS_KASAN_NOSANITIZE CFLAGS_UBSAN
export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULE
export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_LDFLAGS_MODULE
export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index dbbd4382f15a..db473309f136 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -39,4 +39,7 @@ else
endif

CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
+
+CFLAGS_KASAN_NOSANITIZE := $(call cc-option, -fno-builtin)
+
endif
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 1ca4dcd2d500..015aa9dbad86 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -121,7 +121,7 @@ endif
ifeq ($(CONFIG_KASAN),y)
_c_flags += $(if $(patsubst n%,, \
$(KASAN_SANITIZE_$(basetarget).o)$(KASAN_SANITIZE)y), \
- $(CFLAGS_KASAN))
+ $(CFLAGS_KASAN), $(CFLAGS_KASAN_NOSANITIZE))
endif

ifeq ($(CONFIG_UBSAN),y)
--
2.16.0.rc1.238.g530d649a79-goog

Andrey Ryabinin

unread,
Jan 19, 2018, 10:57:29 AM1/19/18
to Andrey Konovalov, Masahiro Yamada, Michal Marek, Alexander Potapenko, Dmitry Vyukov, linux-...@vger.kernel.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Kostya Serebryany, Evgeniy Stepanov, Nick Desaulniers, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrew Morton


On 01/19/2018 05:54 PM, Andrey Konovalov wrote:

> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index dbbd4382f15a..db473309f136 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -39,4 +39,7 @@ else
> endif
>
> CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
> +
> +CFLAGS_KASAN_NOSANITIZE := $(call cc-option, -fno-builtin)
> +

I think it should be fine to drop cc-option here. Even ancient versions of GCC have -fno-builtin.

Andrey Konovalov

unread,
Jan 19, 2018, 12:58:16 PM1/19/18
to Masahiro Yamada, Michal Marek, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, linux-...@vger.kernel.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Kostya Serebryany, Evgeniy Stepanov, Nick Desaulniers, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrew Morton, Andrey Konovalov
With KASAN enabled the kernel has two different memset() functions, one
with KASAN checks (memset) and one without (__memset). KASAN uses some
macro tricks to use the proper version where required. For example memset()
calls in mm/slub.c are without KASAN checks, since they operate on poisoned
slab object metadata.

The issue is that clang emits memset() calls even when there is no memset()
in the source code. They get linked with improper memset() implementation
and the kernel fails to boot due to a huge amount of KASAN reports during
early boot stages.

The solution is to add -fno-builtin flag for files with KASAN_SANITIZE := n
marker.

Signed-off-by: Andrey Konovalov <andre...@google.com>
---

Changed in v2:
- dropped cc-option for -fno-builtin

Makefile | 3 ++-
scripts/Makefile.kasan | 3 +++
scripts/Makefile.lib | 2 +-
3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index bf5b8cbb9469..d45e31b293d0 100644
--- a/Makefile
+++ b/Makefile
@@ -432,7 +432,8 @@ export MAKE AWK GENKSYMS INSTALLKERNEL PERL PYTHON UTS_MACHINE
export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS

export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
-export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE CFLAGS_KASAN CFLAGS_UBSAN
+export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
+export CFLAGS_KASAN CFLAGS_KASAN_NOSANITIZE CFLAGS_UBSAN
export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULE
export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_LDFLAGS_MODULE
export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index dbbd4382f15a..3fb382c69ff6 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -39,4 +39,7 @@ else
endif

CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
+
+CFLAGS_KASAN_NOSANITIZE := -fno-builtin
+
endif
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 1ca4dcd2d500..015aa9dbad86 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -121,7 +121,7 @@ endif
ifeq ($(CONFIG_KASAN),y)
_c_flags += $(if $(patsubst n%,, \
$(KASAN_SANITIZE_$(basetarget).o)$(KASAN_SANITIZE)y), \
- $(CFLAGS_KASAN))
+ $(CFLAGS_KASAN), $(CFLAGS_KASAN_NOSANITIZE))
endif

ifeq ($(CONFIG_UBSAN),y)
--
2.16.0.rc1.238.g530d649a79-goog

Andrey Konovalov

unread,
Jan 19, 2018, 12:58:47 PM1/19/18
to Andrey Ryabinin, Masahiro Yamada, Michal Marek, Alexander Potapenko, Dmitry Vyukov, linux-...@vger.kernel.org, LKML, kasan-dev, Kostya Serebryany, Evgeniy Stepanov, Nick Desaulniers, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrew Morton
On Fri, Jan 19, 2018 at 4:57 PM, Andrey Ryabinin
<arya...@virtuozzo.com> wrote:
>
>
> On 01/19/2018 05:54 PM, Andrey Konovalov wrote:
>
>> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
>> index dbbd4382f15a..db473309f136 100644
>> --- a/scripts/Makefile.kasan
>> +++ b/scripts/Makefile.kasan
>> @@ -39,4 +39,7 @@ else
>> endif
>>
>> CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
>> +
>> +CFLAGS_KASAN_NOSANITIZE := $(call cc-option, -fno-builtin)
>> +
>
> I think it should be fine to drop cc-option here. Even ancient versions of GCC have -fno-builtin.
>

Mailed v2.

Thanks!

Nick Desaulniers

unread,
Jan 19, 2018, 2:06:16 PM1/19/18
to Andrey Konovalov, Masahiro Yamada, Michal Marek, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, Linux Kbuild mailing list, LKML, kasan-dev, Kostya Serebryany, Evgeniy Stepanov, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrew Morton, David Majnemer, Chandler Carruth, Michael Kuperstein
Hmm... I had mentioned this patch to some coworkers who have much more
knowledge about LLVM than me. They had concern that LLVM needs memset
to be defined, and that there were discussions on the llvm mailing
list about this.

I'm digging through trying to find anything relevant, maybe this one
about memcpy: https://lists.llvm.org/pipermail/llvm-dev/2012-May/050108.html

I wonder if -ffreestanding is more appropriate?

Nick Desaulniers

unread,
Jan 19, 2018, 2:08:30 PM1/19/18
to Andrey Konovalov, Masahiro Yamada, Michal Marek, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, Linux Kbuild mailing list, LKML, kasan-dev, Kostya Serebryany, Evgeniy Stepanov, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrew Morton, David Majnemer, Chandler Carruth, Michael Kuperstein
sorry, for new people I just cc'ed: https://patchwork.kernel.org/patch/10175831/
--
Thanks,
~Nick Desaulniers

Andrey Konovalov

unread,
Jan 22, 2018, 12:22:39 PM1/22/18
to Nick Desaulniers, Masahiro Yamada, Michal Marek, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, Linux Kbuild mailing list, LKML, kasan-dev, Kostya Serebryany, Evgeniy Stepanov, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrew Morton, David Majnemer, Chandler Carruth, Michael Kuperstein
I don't mind using either of those, they both fix the issue.

I'm struggling to understand the difference though. GCC documentation
doesn't really explain it [1] and Clang documentation [2] is
completely useless in this case.

[1] https://gcc.gnu.org/onlinedocs/gcc/Standards.html
[2] https://clang.llvm.org/docs/CommandGuide/clang.html

Nick Desaulniers

unread,
Jan 22, 2018, 1:12:35 PM1/22/18
to Andrey Konovalov, Masahiro Yamada, Michal Marek, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, Linux Kbuild mailing list, LKML, kasan-dev, Kostya Serebryany, Evgeniy Stepanov, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrew Morton, David Majnemer, Chandler Carruth, Michael Kuperstein
There's always the source, which is the ultimate source of truth. In LLVM,

lib/Transforms/Utils/SimplifyLibCalls.cpp
lib/Transforms/Scalar/MemCpyOptimizer.cpp
include/llvm/Analysis/TargetLibraryInfo.h

are the only things with mentions of `no-builtin` or `freestanding`.

The driver is what parses these options:

build/tools/clang/include/clang/Driver/Options.inc:
774 OPTION(prefix_1, "ffreestanding", ffreestanding, Flag, f_Group,
INVALID, nullptr, CC1Option, 0,
775 "Assert that the compilation takes place in a freestanding
environment", nullptr, nullptr)
...
1004 OPTION(prefix_1, "fno-builtin-", fno_builtin_, Joined, f_Group,
INVALID, nullptr, CC1Option, 0,
1005 "Disable implicit builtin knowledge of a specific
function", nullptr, nullptr)
1006 OPTION(prefix_1, "fno-builtin", fno_builtin, Flag, f_Group,
INVALID, nullptr, CC1Option, 0,
1007 "Disable implicit builtin knowledge of functions", nullptr, nullptr)

It looks like the TargetLibraryInfo#disableAllFunctions() method is
called in a few different drivers. The options table generates all
kinds of code, but places that parse command line flags look for
`OPT_` + 3rd arg to OPTION.

In Clang, looking for OPT_no_fbuiltin and OPT_ffreestanding:

lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp

the second seems to set:

559 Opts.SimplifyLibCalls = !(Args.hasArg(OPT_fno_builtin) ||
560 Args.hasArg(OPT_ffreestanding));
...
2201 Opts.Freestanding = Args.hasArg(OPT_ffreestanding);

which are then used back in LLVM.

Opts.Freestanding seems to be used from lots of LTO handling code in LLVM.

If ffreestanding implies fno-builtin, but does additional things with
LTO, then we might regress LTO (there's 2 patch sets in the works that
I know of) with KASAN by using ffreestanding rather than fno-builtin.
But lets cross that bridge when we get there (if we decide for some
reason that we need LTO+KASAN; I like both and if we start shipping
LTO kernels then you can bet we'd want to test them with KASAN).

So I don't think it hurts to keep v2 as is (sorry for the noise).

Acked-by: Nick Desaulniers <ndesau...@google.com>

--
Thanks,
~Nick Desaulniers

Andrew Morton

unread,
Jan 22, 2018, 9:20:50 PM1/22/18
to Andrey Konovalov, Masahiro Yamada, Michal Marek, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, linux-...@vger.kernel.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Kostya Serebryany, Evgeniy Stepanov, Nick Desaulniers, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Arnd Bergmann
On Fri, 19 Jan 2018 18:58:12 +0100 Andrey Konovalov <andre...@google.com> wrote:

> With KASAN enabled the kernel has two different memset() functions, one
> with KASAN checks (memset) and one without (__memset). KASAN uses some
> macro tricks to use the proper version where required. For example memset()
> calls in mm/slub.c are without KASAN checks, since they operate on poisoned
> slab object metadata.
>
> The issue is that clang emits memset() calls even when there is no memset()
> in the source code. They get linked with improper memset() implementation
> and the kernel fails to boot due to a huge amount of KASAN reports during
> early boot stages.
>
> The solution is to add -fno-builtin flag for files with KASAN_SANITIZE := n
> marker.

This clashes somewhat with Arnd's asan-rework-kconfig-settings.patch.
Could you two please put heads together and decide what we want for a
final result?

Meanwhile I'll "fix" the reject with

+ifdef CONFIG_KASAN_EXTRA
CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
+endif


From: Arnd Bergmann <ar...@arndb.de>
Subject: kasan: rework Kconfig settings

We get a lot of very large stack frames using gcc-7.0.1 with the default
-fsanitize-address-use-after-scope --param asan-stack=1 options, which can
easily cause an overflow of the kernel stack, e.g.

drivers/gpu/drm/i915/gvt/handlers.c:2434:1: warning: the frame size of 46176 bytes is larger than 3072 bytes
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5650:1: warning: the frame size of 23632 bytes is larger than 3072 bytes
lib/atomic64_test.c:250:1: warning: the frame size of 11200 bytes is larger than 3072 bytes
drivers/gpu/drm/i915/gvt/handlers.c:2621:1: warning: the frame size of 9208 bytes is larger than 3072 bytes
drivers/media/dvb-frontends/stv090x.c:3431:1: warning: the frame size of 6816 bytes is larger than 3072 bytes
fs/fscache/stats.c:287:1: warning: the frame size of 6536 bytes is larger than 3072 bytes

To reduce this risk, -fsanitize-address-use-after-scope is now split out
into a separate CONFIG_KASAN_EXTRA Kconfig option, leading to stack frames
that are smaller than 2 kilobytes most of the time on x86_64. An earlier
version of this patch also prevented combining KASAN_EXTRA with
KASAN_INLINE, but that is no longer necessary with gcc-7.0.1.

All patches to get the frame size below 2048 bytes with CONFIG_KASAN=y and
CONFIG_KASAN_EXTRA=n have been merged by maintainers now, so we can bring
back that default now. KASAN_EXTRA=y still causes lots of warnings but
now defaults to !COMPILE_TEST to disable it in allmodconfig, and it
remains disabled in all other defconfigs since it is a new option. I
arbitrarily raise the warning limit for KASAN_EXTRA to 3072 to reduce the
noise, but an allmodconfig kernel still has around 50 warnings on gcc-7.

I experimented a bit more with smaller stack frames and have another
follow-up series that reduces the warning limit for 64-bit architectures
to 1280 bytes (without CONFIG_KASAN).

With earlier versions of this patch series, I also had patches to address
the warnings we get with KASAN and/or KASAN_EXTRA, using a
"noinline_if_stackbloat" annotation. That annotation now got replaced
with a gcc-8 bugfix (see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715) and a workaround for
older compilers, which means that KASAN_EXTRA is now just as bad as before
and will lead to an instant stack overflow in a few extreme cases.

This reverts parts of commit commit 3f181b4 ("lib/Kconfig.debug: disable
-Wframe-larger-than warnings with KASAN=y"). Two patches in linux-next
should be merged first to avoid introducing warnings in an allmodconfig
build:
3cd890dbe2a4 ("media: dvb-frontends: fix i2c access helpers for KASAN")
16c3ada89cff ("media: r820t: fix r820t_write_reg for KASAN")


Do we really need to backport this?

I think we do: without this patch, enabling KASAN will lead to
unavoidable kernel stack overflow in certain device drivers when built
with gcc-7 or higher on linux-4.10+ or any version that contains a
backport of commit c5caf21ab0cf8. Most people are probably still on
older compilers, but it will get worse over time as they upgrade their
distros.

The warnings we get on kernels older than this should all be for code
that uses dangerously large stack frames, though most of them do not
cause an actual stack overflow by themselves.The asan-stack option was
added in linux-4.0, and commit 3f181b4d8652 ("lib/Kconfig.debug:
disable -Wframe-larger-than warnings with KASAN=y") effectively turned
off the warning for allmodconfig kernels, so I would like to see this
fix backported to any kernels later than 4.0.

I have done dozens of fixes for individual functions with stack frames
larger than 2048 bytes with asan-stack, and I plan to make sure that
all those fixes make it into the stable kernels as well (most are
already there).

Part of the complication here is that asan-stack (from 4.0) was
originally assumed to always require much larger stacks, but that
turned out to be a combination of multiple gcc bugs that we have now
worked around and fixed, but sanitize-address-use-after-scope (from
v4.10) has a much higher inherent stack usage and also suffers from at
least three other problems that we have analyzed but not yet fixed
upstream, each of them makes the stack usage more severe than it should
be.

Link: http://lkml.kernel.org/r/20171221134744...@arndb.de
Signed-off-by: Arnd Bergmann <ar...@arndb.de>
Acked-by: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Mauro Carvalho Chehab <mch...@kernel.org>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: <sta...@vger.kernel.org>

Signed-off-by: Andrew Morton <ak...@linux-foundation.org>
---

lib/Kconfig.debug | 2 +-
lib/Kconfig.kasan | 11 +++++++++++
scripts/Makefile.kasan | 3 +++
3 files changed, 15 insertions(+), 1 deletion(-)

diff -puN lib/Kconfig.debug~kasan-rework-kconfig-settings lib/Kconfig.debug
--- a/lib/Kconfig.debug~kasan-rework-kconfig-settings
+++ a/lib/Kconfig.debug
@@ -217,7 +217,7 @@ config ENABLE_MUST_CHECK
config FRAME_WARN
int "Warn for stack frames larger than (needs gcc 4.4)"
range 0 8192
- default 0 if KASAN
+ default 3072 if KASAN_EXTRA
default 2048 if GCC_PLUGIN_LATENT_ENTROPY
default 1280 if (!64BIT && PARISC)
default 1024 if (!64BIT && !PARISC)
diff -puN lib/Kconfig.kasan~kasan-rework-kconfig-settings lib/Kconfig.kasan
--- a/lib/Kconfig.kasan~kasan-rework-kconfig-settings
+++ a/lib/Kconfig.kasan
@@ -20,6 +20,17 @@ config KASAN
Currently CONFIG_KASAN doesn't work with CONFIG_DEBUG_SLAB
(the resulting kernel does not boot).

+config KASAN_EXTRA
+ bool "KAsan: extra checks"
+ depends on KASAN && DEBUG_KERNEL && !COMPILE_TEST
+ help
+ This enables further checks in the kernel address sanitizer, for now
+ it only includes the address-use-after-scope check that can lead
+ to excessive kernel stack usage, frame size warnings and longer
+ compile time.
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 has more
+
+
choice
prompt "Instrumentation type"
depends on KASAN
diff -puN scripts/Makefile.kasan~kasan-rework-kconfig-settings scripts/Makefile.kasan
--- a/scripts/Makefile.kasan~kasan-rework-kconfig-settings
+++ a/scripts/Makefile.kasan
@@ -38,5 +38,8 @@ else

endif

+ifdef CONFIG_KASAN_EXTRA
CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
endif
+
+endif
_

Andrey Ryabinin

unread,
Jan 23, 2018, 4:24:48 AM1/23/18
to Andrey Konovalov, Masahiro Yamada, Michal Marek, Alexander Potapenko, Dmitry Vyukov, linux-...@vger.kernel.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Kostya Serebryany, Evgeniy Stepanov, Nick Desaulniers, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrew Morton


On 01/19/2018 08:58 PM, Andrey Konovalov wrote:
> With KASAN enabled the kernel has two different memset() functions, one
> with KASAN checks (memset) and one without (__memset). KASAN uses some
> macro tricks to use the proper version where required. For example memset()
> calls in mm/slub.c are without KASAN checks, since they operate on poisoned
> slab object metadata.
>
> The issue is that clang emits memset() calls even when there is no memset()
> in the source code. They get linked with improper memset() implementation
> and the kernel fails to boot due to a huge amount of KASAN reports during
> early boot stages.
>

So how did you observe this? Why am I not seeing this problem?

> The solution is to add -fno-builtin flag for files with KASAN_SANITIZE := n
> marker.
>

I'm not sure I understand how is this solves the problem. And what clang does
instead of memset()? Does it inlines memset()? But according to GCC's man (clang's man is very vague about this)
-fno-builtin should do exactly the opposite - prevent inlining builtin functions
and always generate a function call.

Andrey Ryabinin

unread,
Jan 23, 2018, 4:35:04 AM1/23/18
to Andrew Morton, Andrey Konovalov, Masahiro Yamada, Michal Marek, Alexander Potapenko, Dmitry Vyukov, linux-...@vger.kernel.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Kostya Serebryany, Evgeniy Stepanov, Nick Desaulniers, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Arnd Bergmann


On 01/23/2018 05:20 AM, Andrew Morton wrote:
> On Fri, 19 Jan 2018 18:58:12 +0100 Andrey Konovalov <andre...@google.com> wrote:
>
>> With KASAN enabled the kernel has two different memset() functions, one
>> with KASAN checks (memset) and one without (__memset). KASAN uses some
>> macro tricks to use the proper version where required. For example memset()
>> calls in mm/slub.c are without KASAN checks, since they operate on poisoned
>> slab object metadata.
>>
>> The issue is that clang emits memset() calls even when there is no memset()
>> in the source code. They get linked with improper memset() implementation
>> and the kernel fails to boot due to a huge amount of KASAN reports during
>> early boot stages.
>>
>> The solution is to add -fno-builtin flag for files with KASAN_SANITIZE := n
>> marker.
>
> This clashes somewhat with Arnd's asan-rework-kconfig-settings.patch.
> Could you two please put heads together and decide what we want for a
> final result?
>
> Meanwhile I'll "fix" the reject with
>
> +ifdef CONFIG_KASAN_EXTRA
> CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
> +endif
>

Looks correct.

Andrey Konovalov

unread,
Jan 23, 2018, 11:00:01 AM1/23/18
to Andrew Morton, Masahiro Yamada, Michal Marek, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, Linux Kbuild mailing list, LKML, kasan-dev, Kostya Serebryany, Evgeniy Stepanov, Nick Desaulniers, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Arnd Bergmann
On Tue, Jan 23, 2018 at 3:20 AM, Andrew Morton
<ak...@linux-foundation.org> wrote:
> On Fri, 19 Jan 2018 18:58:12 +0100 Andrey Konovalov <andre...@google.com> wrote:
>
>> With KASAN enabled the kernel has two different memset() functions, one
>> with KASAN checks (memset) and one without (__memset). KASAN uses some
>> macro tricks to use the proper version where required. For example memset()
>> calls in mm/slub.c are without KASAN checks, since they operate on poisoned
>> slab object metadata.
>>
>> The issue is that clang emits memset() calls even when there is no memset()
>> in the source code. They get linked with improper memset() implementation
>> and the kernel fails to boot due to a huge amount of KASAN reports during
>> early boot stages.
>>
>> The solution is to add -fno-builtin flag for files with KASAN_SANITIZE := n
>> marker.
>
> This clashes somewhat with Arnd's asan-rework-kconfig-settings.patch.
> Could you two please put heads together and decide what we want for a
> final result?
>
> Meanwhile I'll "fix" the reject with
>
> +ifdef CONFIG_KASAN_EXTRA
> CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
> +endif

With both patches applied scripts/Makefile.kasan should end like this:

"""
ifdef CONFIG_KASAN_EXTRA
CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
endif

CFLAGS_KASAN_NOSANITIZE := -fno-builtin

endif
"""

If that is what you mean, then the "fix" is correct.

Let me know if you need me to send v3 that can be applied on top of
Arnd's patches.
> --
> 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+...@googlegroups.com.
> To post to this group, send email to kasa...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20180122182047.1da66312c5d94cb4632efabd%40linux-foundation.org.
> For more options, visit https://groups.google.com/d/optout.

Andrey Konovalov

unread,
Jan 23, 2018, 11:34:25 AM1/23/18
to Andrey Ryabinin, Masahiro Yamada, Michal Marek, Alexander Potapenko, Dmitry Vyukov, Linux Kbuild mailing list, LKML, kasan-dev, Kostya Serebryany, Evgeniy Stepanov, Nick Desaulniers, Stephen Hines, Pirama Arumuga Nainar, Manoj Gupta, Greg Hackmann, Andrew Morton
On Tue, Jan 23, 2018 at 10:24 AM, Andrey Ryabinin
<arya...@virtuozzo.com> wrote:
>
>
> On 01/19/2018 08:58 PM, Andrey Konovalov wrote:
>> With KASAN enabled the kernel has two different memset() functions, one
>> with KASAN checks (memset) and one without (__memset). KASAN uses some
>> macro tricks to use the proper version where required. For example memset()
>> calls in mm/slub.c are without KASAN checks, since they operate on poisoned
>> slab object metadata.
>>
>> The issue is that clang emits memset() calls even when there is no memset()
>> in the source code. They get linked with improper memset() implementation
>> and the kernel fails to boot due to a huge amount of KASAN reports during
>> early boot stages.
>>
>
> So how did you observe this? Why am I not seeing this problem?

I only observed this when I tried to cross-compile the kernel for
arm64 with clang. I suspect this can happen on x86 as well.

>
>> The solution is to add -fno-builtin flag for files with KASAN_SANITIZE := n
>> marker.
>>
>
> I'm not sure I understand how is this solves the problem. And what clang does
> instead of memset()? Does it inlines memset()? But according to GCC's man (clang's man is very vague about this)
> -fno-builtin should do exactly the opposite - prevent inlining builtin functions
> and always generate a function call.

The issue is that the compiler is allowed to replace plain assignment
with memset() calls. And -fno-builtin suppresses that behavior, but
there are no mentions of this in the documentation that I could find.
I also failed to find the exact place in the source code that leads to
this, perhaps Nick can help with that.
Reply all
Reply to author
Forward
0 new messages