Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
MIME-Version: 1.0
References: <20231018153147.167393-1-hamza.mahfooz@amd.com>
 <CANpmjNPZ0Eii3ZTrVqEL2Ez0Jv23y-emLBCLSZ==xmH--4E65g@mail.gmail.com> <c7a80dc5-d18f-4a7d-915b-1803cc3e33ca@amd.com>
In-Reply-To: <c7a80dc5-d18f-4a7d-915b-1803cc3e33ca@amd.com>
From:   Marco Elver <elver@google.com>
Date:   Wed, 18 Oct 2023 18:51:31 +0200
Message-ID: <CANpmjNN7yvEjvTDHzzEqvN2iKvxjvOjpsz_ugSjwh4VBKDNH6g@mail.gmail.com>
Subject: Re: [PATCH] lib: Kconfig: disable dynamic sanitizers for test builds
To:     Hamza Mahfooz <hamza.mahfooz@amd.com>
Cc:     linux-kernel@vger.kernel.org,
        Rodrigo Siqueira <rodrigo.siqueira@amd.com>,
        Harry Wentland <harry.wentland@amd.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>,
        Alexander Potapenko <glider@google.com>,
        Andrey Konovalov <andreyknvl@gmail.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Tom Rix <trix@redhat.com>, kasan-dev@googlegroups.com,
        llvm@lists.linux.dev, Arnd Bergmann <arnd@arndb.de>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 18 Oct 2023 at 18:43, Hamza Mahfooz <hamza.mahfooz@amd.com> wrote:
>
> On 10/18/23 12:22, Marco Elver wrote:
> > On Wed, 18 Oct 2023 at 17:32, 'Hamza Mahfooz' via kasan-dev
> > <kasan-dev@googlegroups.com> wrote:
> >>
> >> kasan, kcsan and kmsan all have the tendency to blow up the stack
> >> and there isn't a lot of value in having them enabled for test builds,
> >> since they are intended to be useful for runtime debugging. So, disable
> >> them for test builds.
> >>
> >> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
> >> ---
> >>   lib/Kconfig.kasan | 1 +
> >>   lib/Kconfig.kcsan | 1 +
> >>   lib/Kconfig.kmsan | 1 +
> >>   3 files changed, 3 insertions(+)
> >
> > Do you have links to discussions that motivate this change? This has
> > been discussed in the past. One recommendation is to adjust the
>
> Sure, you can checkout:
> https://lore.kernel.org/amd-gfx/CADnq5_OyO9CHqahFvdnx7-8s9654udgdfhUntyxfjae+iHey0Q@mail.gmail.com/T/#m5d227dc1ef07b1f4953312287dce4568666c5e09

I would add this as a Link context to the patch.

> > build/test scripts to exclude some combination of configs if they are
> > causing issues. Or we increase CONFIG_FRAME_WARN if one of them is
> > enabled (KMSAN sets it to 0, 32-bit KASAN increases it a bit).
> >
> > That being said, we're aware of KASAN having had more issues and there
> > are some suboptions that have been disabled because of that (like
> > KASAN_STACK). I'm not sure if Clang's KASAN instrumentation has had
> > some recent improvements (we did investigate it, but I can't recall
> > what the outcome was [1]) - maybe try a more recent compiler? However,
> > KCSAN and KMSAN shouldn't have any issues (if KMSAN is enabled,
>
> This patch was initially motivated by KCSAN (i.e. I am able to get it to
> blow up the stack with a minimal .config). I don't mind dropping the
> other ones since I only included them because Nathan implied that they
> could cause similar issues.

!COMPILE_TEST is not the solution. Clearly from the link you provided
build testing is helpful in catching early issues, so that these tools
remain usable for everyone. But we know they use a little more stack,
and the warnings need to be adjusted accordingly.

My suggestion is to just increase FRAME_WARN for KCSAN, or set it to 0
(like for KMSAN). My guess is that first trying to increase it is the
safer option.

> > FRAME_WARN is 0). And having build tests with them enabled isn't
> > useless at all: we're making sure that these tools (even though only
> > for debugging), still work. We _want_ them to work during random build
> > testing!
> >
> > Please share the concrete problem you're having, because this change
> > will make things worse for everyone in the long run.
> >
> > [1] https://github.com/llvm/llvm-project/issues/38157
> >
> >> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> >> index fdca89c05745..fbd85c4872c0 100644
> >> --- a/lib/Kconfig.kasan
> >> +++ b/lib/Kconfig.kasan
> >> @@ -38,6 +38,7 @@ menuconfig KASAN
> >>                      CC_HAS_WORKING_NOSANITIZE_ADDRESS) || \
> >>                     HAVE_ARCH_KASAN_HW_TAGS
> >>          depends on (SLUB && SYSFS && !SLUB_TINY) || (SLAB && !DEBUG_SLAB)
> >> +       depends on !COMPILE_TEST
> >>          select STACKDEPOT_ALWAYS_INIT
> >>          help
> >>            Enables KASAN (Kernel Address Sanitizer) - a dynamic memory safety
> >
> > This also disables KASAN_HW_TAGS, which is actually enabled in
> > production kernels and does not use any compiler instrumentation.
> >
> >> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> >> index 609ddfc73de5..7bcefdbfb46f 100644
> >> --- a/lib/Kconfig.kcsan
> >> +++ b/lib/Kconfig.kcsan
> >> @@ -14,6 +14,7 @@ menuconfig KCSAN
> >>          bool "KCSAN: dynamic data race detector"
> >>          depends on HAVE_ARCH_KCSAN && HAVE_KCSAN_COMPILER
> >>          depends on DEBUG_KERNEL && !KASAN
> >> +       depends on !COMPILE_TEST
> >>          select CONSTRUCTORS
> >>          select STACKTRACE
> >>          help
> >> diff --git a/lib/Kconfig.kmsan b/lib/Kconfig.kmsan
> >> index ef2c8f256c57..eb05c885d3fd 100644
> >> --- a/lib/Kconfig.kmsan
> >> +++ b/lib/Kconfig.kmsan
> >> @@ -13,6 +13,7 @@ config KMSAN
> >>          depends on HAVE_ARCH_KMSAN && HAVE_KMSAN_COMPILER
> >>          depends on SLUB && DEBUG_KERNEL && !KASAN && !KCSAN
> >>          depends on !PREEMPT_RT
> >> +       depends on !COMPILE_TEST
> >
> > KMSAN already selects FRAME_WARN of 0 and should not cause you any
> > issues during build testing.
> >
> > Nack.
> --
> Hamza
>