Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230712101344.2714626-1-chenhuacai@loongson.cn>
 <CA+fCnZd1nhG9FDzkeW42jFbPuGKZms-HzHXBiO5YTSnkmsZoZQ@mail.gmail.com>
 <CAAhV-H4nuwBJHE3VPj6yE2HUw3tDaLtgeRQ5mj0SRV6RoD8-9Q@mail.gmail.com> <CANpmjNM_FEpXPVgoAbUwEK+9m90X54ykWnMvpUP2ZQ8sjoSByg@mail.gmail.com>
In-Reply-To: <CANpmjNM_FEpXPVgoAbUwEK+9m90X54ykWnMvpUP2ZQ8sjoSByg@mail.gmail.com>
From:   Huacai Chen <chenhuacai@kernel.org>
Date:   Thu, 13 Jul 2023 17:58:31 +0800
Message-ID: <CAAhV-H4WUXVYv5er7UpPHKQDdBheT-UgEsOnBmPGPJ=LKWh4PQ@mail.gmail.com>
Subject: Re: [PATCH] kasan: Fix tests by removing -ffreestanding
To:     Marco Elver <elver@google.com>
Cc:     Andrey Konovalov <andreyknvl@gmail.com>,
        Huacai Chen <chenhuacai@loongson.cn>,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>,
        Alexander Potapenko <glider@google.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi, Marco,

On Thu, Jul 13, 2023 at 4:12=E2=80=AFPM Marco Elver <elver@google.com> wrot=
e:
>
> On Thu, 13 Jul 2023 at 06:33, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Andrey,
> >
> > On Thu, Jul 13, 2023 at 12:12=E2=80=AFAM Andrey Konovalov <andreyknvl@g=
mail.com> wrote:
> > > On Wed, Jul 12, 2023 at 12:14=E2=80=AFPM Huacai Chen <chenhuacai@loon=
gson.cn> wrote:
> > > >
> > > > CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX hopes -fbuiltin for memset(=
)/
> > > > memcpy()/memmove() if instrumentation is needed. This is the defaul=
t
> > > > behavior but some archs pass -ffreestanding which implies -fno-buil=
tin,
> > > > 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 :=3D $(CFLAGS_KASAN_TEST)
> > > > +CFLAGS_REMOVE_kasan_test.o :=3D -ffreestanding
> > > >  CFLAGS_kasan_test_module.o :=3D $(CFLAGS_KASAN_TEST)
> > > > +CFLAGS_REMOVE_kasan_test_module.o :=3D -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@google.com
>
> -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.
Thank you for your advice, but we should keep -ffreestanding for
LoongArch, even if it may cause failing to detect bad accesses.
Because now the __builtin_memset() assumes hardware supports unaligned
access, which is not the case for Loongson-2K series. If removing
-ffreestanding, Loongson-2K gets a poor performance.

On the other hand, LoongArch is not the only architecture use
-ffreestanding, e.g., MIPS, X86_32, M68K and Xtensa also use, so the
tests should get fixed.

Huacai