linux-kernel - Re: [PATCH v4 2/3] mm/slub, kunit: add a KUnit test for SLUB debugging functionality

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]

Message-ID: <CANpmjNNGGmhe+GxeP+MYgt1F-RXoC7K-3Sh0h4gOnwpE=RuNXQ@mail.gmail.com>
Date:   Thu, 15 Apr 2021 13:01:21 +0200
From:   Marco Elver <elver@...gle.com>
To:     Vlastimil Babka <vbabka@...e.cz>
Cc:     Oliver Glitta <glittao@...il.com>,
        Brendan Higgins <brendanhiggins@...gle.com>,
        Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        KUnit Development <kunit-dev@...glegroups.com>,
        Linux Memory Management List <linux-mm@...ck.org>,
        Daniel Latypov <dlatypov@...gle.com>
Subject: Re: [PATCH v4 2/3] mm/slub, kunit: add a KUnit test for SLUB
 debugging functionality

On Thu, 15 Apr 2021 at 12:38, Vlastimil Babka <vbabka@...e.cz> wrote:
>
> On 4/15/21 12:10 PM, Oliver Glitta wrote:
> > ut 13. 4. 2021 o 15:54 Marco Elver <elver@...gle.com> napísal(a):
> >>
> >> On Tue, 13 Apr 2021 at 12:07, <glittao@...il.com> wrote:
> >> > From: Oliver Glitta <glittao@...il.com>
> >> >
> >> > SLUB has resiliency_test() function which is hidden behind #ifdef
> >> > SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
> >> > runs it. KUnit should be a proper replacement for it.
> >> >
> >> > Try changing byte in redzone after allocation and changing
> >> > pointer to next free node, first byte, 50th byte and redzone
> >> > byte. Check if validation finds errors.
> >> >
> >> > There are several differences from the original resiliency test:
> >> > Tests create own caches with known state instead of corrupting
> >> > shared kmalloc caches.
> >> >
> >> > The corruption of freepointer uses correct offset, the original
> >> > resiliency test got broken with freepointer changes.
> >> >
> >> > Scratch changing random byte test, because it does not have
> >> > meaning in this form where we need deterministic results.
> >> >
> >> > Add new option CONFIG_SLUB_KUNIT_TEST in Kconfig.
> >> > Because the test deliberatly modifies non-allocated objects, it depends on
> >> > !KASAN which would have otherwise prevented that.
> >>
> >> Hmm, did the test fail with KASAN? Is it possible to skip the tests
> >> and still run a subset of tests with KASAN? It'd be nice if we could
> >> run some of these tests with KASAN as well.
> >>
> >> > Use kunit_resource to count errors in cache and silence bug reports.
> >> > Count error whenever slab_bug() or slab_fix() is called or when
> >> > the count of pages is wrong.
> >> >
> >> > Signed-off-by: Oliver Glitta <glittao@...il.com>
> >>
> >> Reviewed-by: Marco Elver <elver@...gle.com>
> >>
> >
> > Thank you.
> >
> >> Thanks, this all looks good to me. But perhaps do test what works with
> >> KASAN, to see if you need the !KASAN constraint for all cases.
> >
> > I tried to run tests with KASAN functionality disabled with function
> > kasan_disable_current() and three of the tests failed with wrong
> > errors counts.
> > So I add the !KASAN constraint for all tests, because the merge window
> > is coming, we want to know if this version is stable and without other
> > mistakes.
> > We will take a closer look at that in the follow-up patch.
>
> Agreed. In this context, KASAN is essentially a different implementation of the
> same checks that SLUB_DEBUG offers (and also does other checks) and we excercise
> these SLUB_DEBUG checks by deliberately causing the corruption that they detect
> - so instead, KASAN detects it, as it should. I assume that once somebody opts
> for a full KASAN kernel build, they don't need the SLUB_DEBUG functionality at
> that point, as KASAN is more extensive (On the other hand SLUB_DEBUG kernels can
> be (and are) shipped as production distro kernels where specific targetted
> debugging can be enabled to help find bugs in production with minimal disruption).
> So trying to make both cooperate can work only to some extent and for now we've
> chosen the safer way.

Sounds reasonable. In any case, I'm fine with this version to land and
my Reviewed-by above remains valid. :-)

Thanks,
-- Marco