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

[Kees Cook]

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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> 
> 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