On 3/16/22 15:38, Hyeonggon Yoo wrote: > Testcase test_next_pointer in slub_kunit fails when SLAB_RED_ZONE flag > is globally set. This is because on_freelist() cuts corrupted freelist > chain and does not update cut objects' redzone to SLUB_RED_ACTIVE. > > When the test validates a slab that whose freelist is cut, it expects > redzone of objects unreachable by freelist is set to SLUB_RED_ACTIVE. > And it reports "Left Redzone overritten" error because the expectation > failed. > > This patch makes slub_kunit expect two more errors for reporting and > fixing red overwritten error when SLAB_RED_ZONE flag is set. > > The test passes on slub_debug and slub_debug=Z after this patch. Hmm I think it's not optimal strategy for unit tests to adapt like this to external influence. It seems rather fragile. The test cases should be designed to test a specific condition and that's it. So maybe we could e.g. introduce a new SLAB_ flag passed to kmem_cache_create that tells it to ignore any globally specified slub debug flags? > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > --- > lib/slub_kunit.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c > index 8662dc6cb509..7cf1fb5a7fde 100644 > --- a/lib/slub_kunit.c > +++ b/lib/slub_kunit.c > @@ -45,21 +45,36 @@ static void test_next_pointer(struct kunit *test) > * Expecting three errors. > * One for the corrupted freechain and the other one for the wrong > * count of objects in use. The third error is fixing broken cache. > + * > + * When flag SLUB_RED_ZONE is set, we expect two more errors for reporting > + * and fixing overwritten redzone error. This two errors are detected > + * because SLUB cuts corrupted freelist in on_freelist(), but does not > + * update its redzone to SLUB_RED_ACTIVE. > */ > validate_slab_cache(s); > - KUNIT_EXPECT_EQ(test, 3, slab_errors); > + > + if (s->flags & SLAB_RED_ZONE) > + KUNIT_EXPECT_EQ(test, 5, slab_errors); > + else > + KUNIT_EXPECT_EQ(test, 3, slab_errors); > > /* > * Try to repair corrupted freepointer. > * Still expecting two errors. The first for the wrong count > * of objects in use. > * The second error is for fixing broken cache. > + * > + * When SLUB_RED_ZONE flag is set, we expect two more errors > + * for same reason as above. > */ > *ptr_addr = tmp; > slab_errors = 0; > > validate_slab_cache(s); > - KUNIT_EXPECT_EQ(test, 2, slab_errors); > + if (s->flags & SLAB_RED_ZONE) > + KUNIT_EXPECT_EQ(test, 4, slab_errors); > + else > + KUNIT_EXPECT_EQ(test, 2, slab_errors); > > /* > * Previous validation repaired the count of objects in use.
