Re: [PATCH] kasan: Fix tests by removing -ffreestanding

[Marco Elver]

On Thu, 13 Jul 2023 at 06:33, Huacai Chen <chenhuacai@xxxxxxxxxx> wrote:
>
> Hi, Andrey,
>
> On Thu, Jul 13, 2023 at 12:12 AM Andrey Konovalov <andreyknvl@xxxxxxxxx> wrote:
> > On Wed, Jul 12, 2023 at 12:14 PM Huacai Chen <chenhuacai@xxxxxxxxxxx> wrote:
> > >
> > > CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX hopes -fbuiltin for memset()/
> > > memcpy()/memmove() if instrumentation is needed. This is the default
> > > behavior but some archs pass -ffreestanding which implies -fno-builtin,
> > > and then causes some kasan tests fail. So we remove -ffreestanding for
> > > kasan tests.
> >
> > Could you clarify on which architecture you observed tests failures?
> Observed on LoongArch [1], KASAN for LoongArch was planned to be
> merged in 6.5, but at the last minute I found some tests fail with
> GCC14 (CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX) so the patches are
> dropped. After some debugging we found the root cause is
> -ffreestanding.
[...]
> > >  CFLAGS_kasan_test.o := $(CFLAGS_KASAN_TEST)
> > > +CFLAGS_REMOVE_kasan_test.o := -ffreestanding
> > >  CFLAGS_kasan_test_module.o := $(CFLAGS_KASAN_TEST)
> > > +CFLAGS_REMOVE_kasan_test_module.o := -ffreestanding

It makes sense that if -ffreestanding is added everywhere, that this
patch fixes the test. Also see:
https://lkml.kernel.org/r/20230224085942.1791837-3-elver@xxxxxxxxxx

-ffreestanding implies -fno-builtin, which used to be added to the
test where !CC_HAS_KASAN_MEMINTRINSIC_PREFIX (old compilers).

But ideally, the test doesn't have any special flags to make it pass,
because ultimately we want the test setup to be as close to other
normal kernel code.

What this means for LoongArch, is that the test legitimately is
pointing out an issue: namely that with newer compilers, your current
KASAN support for LoongArch is failing to detect bad accesses within
mem*() functions.

The reason newer compilers should emit __asan_mem*() functions and
replace normal mem*() functions, is that making mem*() functions
always instrumented is not safe when e.g. called from uninstrumented
code. One problem is that compilers will happily generate
memcpy/memset calls themselves for e.g. variable initialization or
struct copies - and unfortunately -ffreestanding does _not_ prohibit
compilers from doing so: https://godbolt.org/z/hxGvdo4P9

I would propose 2 options:

1. Removing -ffreestanding from LoongArch. It is unclear to me why
this is required. As said above, -ffreestanding does not actually
prohibit the compiler from generating implicit memset/memcpy. It
prohibits some other optimizations, but in the kernel, you might even
want those optimizations if common libcalls are already implemented
(which they should be?).

2. If KASAN is enabled on LoongArch, make memset/memcpy/memmove
aliases to __asan_memset/__asan_memcpy/__asan_memmove. That means
you'd have to invert how you currently set up __mem and mem functions:
the implementation is in __mem*, and mem* functions alias __mem* -or-
if KASAN is enabled __asan_mem* functions (ifdef
CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX to make old compilers work as
well).

If you go with option #2 you are accepting the risk of using
instrumented mem* functions from uninstrumented files/functions. This
has been an issue for other architectures. In many cases you might get
lucky enough that it doesn't cause issues, but that's not guaranteed.