linux-hardening - Re: [PATCH v2 02/10] fortify: Allow KUnit test to build without FORTIFY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 3 Jul 2023 12:47:07 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     linux-hardening@...r.kernel.org, Andy Shevchenko <andy@...nel.org>,
        Cezary Rojewski <cezary.rojewski@...el.com>,
        Puyou Lu <puyou.lu@...il.com>, Mark Brown <broonie@...nel.org>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Brendan Higgins <brendan.higgins@...ux.dev>,
        David Gow <davidgow@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Alexander Potapenko <glider@...gle.com>,
        Zhaoyang Huang <zhaoyang.huang@...soc.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Miguel Ojeda <ojeda@...nel.org>,
        Alexander Lobakin <aleksander.lobakin@...el.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Liam Howlett <liam.howlett@...cle.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Dan Williams <dan.j.williams@...el.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Yury Norov <yury.norov@...il.com>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Sander Vanheule <sander@...nheule.net>,
        Eric Biggers <ebiggers@...gle.com>,
        "Masami Hiramatsu (Google)" <mhiramat@...nel.org>,
        Andrey Konovalov <andreyknvl@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Daniel Latypov <dlatypov@...gle.com>,
        José Expósito <jose.exposito89@...il.com>,
        linux-kernel@...r.kernel.org, kunit-dev@...glegroups.com
Subject: Re: [PATCH v2 02/10] fortify: Allow KUnit test to build without
 FORTIFY

On Sun, Jul 02, 2023 at 05:07:05PM +0200, Geert Uytterhoeven wrote:
> Hi Kees,
> 
> On Fri, Apr 7, 2023 at 9:27 PM Kees Cook <keescook@...omium.org> wrote:
> > In order for CI systems to notice all the skipped tests related to
> > CONFIG_FORTIFY_SOURCE, allow the FORTIFY_SOURCE KUnit tests to build
> > with or without CONFIG_FORTIFY_SOURCE.
> >
> > Signed-off-by: Kees Cook <keescook@...omium.org>
> 
> Thanks for your patch, which is now commit a9dc8d0442294b42
> ("fortify: Allow KUnit test to build without FORTIFY") upstream.
> 
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -2614,7 +2614,7 @@ config STACKINIT_KUNIT_TEST
> >
> >  config FORTIFY_KUNIT_TEST
> >         tristate "Test fortified str*() and mem*() function internals at runtime" if !KUNIT_ALL_TESTS
> > -       depends on KUNIT && FORTIFY_SOURCE
> > +       depends on KUNIT
> 
> All other tests depend on the functionality they test.
> Which makes sense, as you only want to test the functionality that is
> available in the kernel you want to run.

Yeah, that is true for KUnit.

> 
> >         default KUNIT_ALL_TESTS
> >         help
> >           Builds unit tests for checking internals of FORTIFY_SOURCE as used
> > diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
> > index c8c33cbaae9e..524132f33cf0 100644
> > --- a/lib/fortify_kunit.c
> > +++ b/lib/fortify_kunit.c
> 
> > @@ -307,6 +312,14 @@ DEFINE_ALLOC_SIZE_TEST_PAIR(kvmalloc)
> >  } while (0)
> >  DEFINE_ALLOC_SIZE_TEST_PAIR(devm_kmalloc)
> >
> > +static int fortify_test_init(struct kunit *test)
> > +{
> > +       if (!IS_ENABLED(CONFIG_FORTIFY_SOURCE))
> > +               kunit_skip(test, "Not built with CONFIG_FORTIFY_SOURCE=y");
> 
> I was greeted by this message, which wasn't that helpful, as
> CONFIG_FORTIFY_SOURCE depends on CONFIG_ARCH_HAS_FORTIFY_SOURCE,
> which is not available yet on all architectures.
> 
> So I think the proper thing to do is to revert this patch.
> Thanks!

I created this patch so that I could add CONFIG_FORTIFY_SOURCE support
to UML, but you have a good point about other archs. I'll prepare a
revert.

-- 
Kees Cook