[PATCH 1/2] selftests: add a kselftest for SLUB debugging functionality

[PATCH 1/2] selftests: add a kselftest for SLUB debugging functionality

[glittao]

From: Oliver Glitta <glittao@gmail.com>

SLUB has resiliency_test() function which is hidden behind #ifdef
SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
runs it. Kselftest should 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_TEST_SLUB in Kconfig.

Add parameter to function validate_slab_cache() to return
number of errors in cache.

Signed-off-by: Oliver Glitta <glittao@gmail.com>
---
 lib/Kconfig.debug                    |   4 +
 lib/Makefile                         |   1 +
 lib/test_slub.c                      | 125 +++++++++++++++++++++++++++
 mm/slab.h                            |   1 +
 mm/slub.c                            |  34 +++++---
 tools/testing/selftests/lib/Makefile |   2 +-
 tools/testing/selftests/lib/config   |   1 +
 tools/testing/selftests/lib/slub.sh  |   3 +
 8 files changed, 159 insertions(+), 12 deletions(-)
 create mode 100644 lib/test_slub.c
 create mode 100755 tools/testing/selftests/lib/slub.sh

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2779c29d9981..2d56092abbc4 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2123,6 +2123,10 @@ config TEST_KSTRTOX
 config TEST_PRINTF
 	tristate "Test printf() family of functions at runtime"
 
+config TEST_SLUB
+	tristate "Test SLUB cache errors at runtime"
+	depends on SLUB_DEBUG
+
 config TEST_BITMAP
 	tristate "Test bitmap_*() family of functions at runtime"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index b5307d3eec1a..b6603803b1c4 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
 obj-$(CONFIG_TEST_PRINTF) += test_printf.o
+obj-$(CONFIG_TEST_SLUB) += test_slub.o
 obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
 obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o
 obj-$(CONFIG_TEST_UUID) += test_uuid.o
diff --git a/lib/test_slub.c b/lib/test_slub.c
new file mode 100644
index 000000000000..0075d9b44251
--- /dev/null
+++ b/lib/test_slub.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test cases for slub facility.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include "../mm/slab.h"
+
+#include "../tools/testing/selftests/kselftest_module.h"
+
+
+KSTM_MODULE_GLOBALS();
+
+
+static void __init validate_result(struct kmem_cache *s, int expected_errors)
+{
+	int errors = 0;
+
+	validate_slab_cache(s, &errors);
+	KSTM_CHECK_ZERO(errors - expected_errors);
+}
+
+static void __init test_clobber_zone(void)
+{
+	struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_alloc", 64, 0,
+				SLAB_RED_ZONE, NULL);
+	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
+
+	p[64] = 0x12;
+	pr_err("1. kmem_cache: Clobber Redzone 0x12->0x%p\n", p + 64);
+
+	validate_result(s, 1);
+	kmem_cache_free(s, p);
+	kmem_cache_destroy(s);
+}
+
+static void __init test_next_pointer(void)
+{
+	struct kmem_cache *s = kmem_cache_create("TestSlub_next_ptr_free", 64, 0,
+				SLAB_RED_ZONE, NULL);
+	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
+
+	kmem_cache_free(s, p);
+	p[s->offset] = 0x12;
+	pr_err("1. kmem_cache: Clobber next pointer 0x34 -> -0x%p\n", p);
+
+	validate_result(s, 1);
+	kmem_cache_destroy(s);
+}
+
+static void __init test_first_word(void)
+{
+	struct kmem_cache *s = kmem_cache_create("TestSlub_1th_word_free", 64, 0,
+				SLAB_POISON, NULL);
+	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
+
+	kmem_cache_free(s, p);
+	*p = 0x78;
+	pr_err("2. kmem_cache: Clobber first word 0x78->0x%p\n", p);
+
+	validate_result(s, 1);
+	kmem_cache_destroy(s);
+}
+
+static void __init test_clobber_50th_byte(void)
+{
+	struct kmem_cache *s = kmem_cache_create("TestSlub_50th_word_free", 64, 0,
+				SLAB_POISON, NULL);
+	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
+
+	kmem_cache_free(s, p);
+	p[50] = 0x9a;
+	pr_err("3. kmem_cache: Clobber 50th byte 0x9a->0x%p\n", p);
+
+	validate_result(s, 1);
+	kmem_cache_destroy(s);
+}
+
+static void __init test_clobber_redzone_free(void)
+{
+	struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_free", 64, 0,
+				SLAB_RED_ZONE, NULL);
+	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
+
+	kmem_cache_free(s, p);
+	p[64] = 0xab;
+	pr_err("4. kmem_cache: Clobber redzone 0xab->0x%p\n", p);
+
+	validate_result(s, 1);
+	kmem_cache_destroy(s);
+}
+
+static void __init resiliency_test(void)
+{
+
+	BUILD_BUG_ON(KMALLOC_MIN_SIZE > 16 || KMALLOC_SHIFT_HIGH < 10);
+
+	pr_err("SLUB resiliency testing\n");
+	pr_err("-----------------------\n");
+	pr_err("A. Corruption after allocation\n");
+
+	test_clobber_zone();
+
+	pr_err("\nB. Corruption after free\n");
+
+	test_next_pointer();
+	test_first_word();
+	test_clobber_50th_byte();
+	test_clobber_redzone_free();
+}
+
+
+static void __init selftest(void)
+{
+	resiliency_test();
+}
+
+
+KSTM_MODULE_LOADERS(test_slub);
+MODULE_LICENSE("GPL");
diff --git a/mm/slab.h b/mm/slab.h
index 076582f58f68..5fc18d506b3b 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -215,6 +215,7 @@ DECLARE_STATIC_KEY_TRUE(slub_debug_enabled);
 DECLARE_STATIC_KEY_FALSE(slub_debug_enabled);
 #endif
 extern void print_tracking(struct kmem_cache *s, void *object);
+long validate_slab_cache(struct kmem_cache *s, int *errors);
 #else
 static inline void print_tracking(struct kmem_cache *s, void *object)
 {
diff --git a/mm/slub.c b/mm/slub.c
index e26c274b4657..c00e2b263e03 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4612,7 +4612,8 @@ static int count_total(struct page *page)
 #endif
 
 #ifdef CONFIG_SLUB_DEBUG
-static void validate_slab(struct kmem_cache *s, struct page *page)
+static void validate_slab(struct kmem_cache *s, struct page *page,
+						int *errors)
 {
 	void *p;
 	void *addr = page_address(page);
@@ -4620,8 +4621,10 @@ static void validate_slab(struct kmem_cache *s, struct page *page)
 
 	slab_lock(page);
 
-	if (!check_slab(s, page) || !on_freelist(s, page, NULL))
+	if (!check_slab(s, page) || !on_freelist(s, page, NULL)) {
+		*errors += 1;
 		goto unlock;
+	}
 
 	/* Now we know that a valid freelist exists */
 	map = get_map(s, page);
@@ -4629,8 +4632,10 @@ static void validate_slab(struct kmem_cache *s, struct page *page)
 		u8 val = test_bit(__obj_to_index(s, addr, p), map) ?
 			 SLUB_RED_INACTIVE : SLUB_RED_ACTIVE;
 
-		if (!check_object(s, page, p, val))
+		if (!check_object(s, page, p, val)) {
+			*errors += 1;
 			break;
+		}
 	}
 	put_map(map);
 unlock:
@@ -4638,7 +4643,7 @@ static void validate_slab(struct kmem_cache *s, struct page *page)
 }
 
 static int validate_slab_node(struct kmem_cache *s,
-		struct kmem_cache_node *n)
+		struct kmem_cache_node *n, int *errors)
 {
 	unsigned long count = 0;
 	struct page *page;
@@ -4647,30 +4652,34 @@ static int validate_slab_node(struct kmem_cache *s,
 	spin_lock_irqsave(&n->list_lock, flags);
 
 	list_for_each_entry(page, &n->partial, slab_list) {
-		validate_slab(s, page);
+		validate_slab(s, page, errors);
 		count++;
 	}
-	if (count != n->nr_partial)
+	if (count != n->nr_partial) {
 		pr_err("SLUB %s: %ld partial slabs counted but counter=%ld\n",
 		       s->name, count, n->nr_partial);
+		*errors += 1;
+	}
 
 	if (!(s->flags & SLAB_STORE_USER))
 		goto out;
 
 	list_for_each_entry(page, &n->full, slab_list) {
-		validate_slab(s, page);
+		validate_slab(s, page, errors);
 		count++;
 	}
-	if (count != atomic_long_read(&n->nr_slabs))
+	if (count != atomic_long_read(&n->nr_slabs)) {
 		pr_err("SLUB: %s %ld slabs counted but counter=%ld\n",
 		       s->name, count, atomic_long_read(&n->nr_slabs));
+		*errors += 1;
+	}
 
 out:
 	spin_unlock_irqrestore(&n->list_lock, flags);
 	return count;
 }
 
-static long validate_slab_cache(struct kmem_cache *s)
+long validate_slab_cache(struct kmem_cache *s, int *errors)
 {
 	int node;
 	unsigned long count = 0;
@@ -4678,10 +4687,12 @@ static long validate_slab_cache(struct kmem_cache *s)
 
 	flush_all(s);
 	for_each_kmem_cache_node(s, node, n)
-		count += validate_slab_node(s, n);
+		count += validate_slab_node(s, n, errors);
 
 	return count;
 }
+EXPORT_SYMBOL(validate_slab_cache);
+
 /*
  * Generate lists of code addresses where slabcache objects are allocated
  * and freed.
@@ -5336,9 +5347,10 @@ static ssize_t validate_store(struct kmem_cache *s,
 			const char *buf, size_t length)
 {
 	int ret = -EINVAL;
+	int errors = 0;
 
 	if (buf[0] == '1') {
-		ret = validate_slab_cache(s);
+		ret = validate_slab_cache(s, &errors);
 		if (ret >= 0)
 			ret = length;
 	}
diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile
index a105f094676e..f168313b7949 100644
--- a/tools/testing/selftests/lib/Makefile
+++ b/tools/testing/selftests/lib/Makefile
@@ -4,6 +4,6 @@
 # No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
 all:
 
-TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh strscpy.sh
+TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh strscpy.sh slub.sh
 
 include ../lib.mk
diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config
index b80ee3f6e265..4190863032e7 100644
--- a/tools/testing/selftests/lib/config
+++ b/tools/testing/selftests/lib/config
@@ -3,3 +3,4 @@ CONFIG_TEST_BITMAP=m
 CONFIG_PRIME_NUMBERS=m
 CONFIG_TEST_STRSCPY=m
 CONFIG_TEST_BITOPS=m
+CONFIG_TEST_SLUB=m
\ No newline at end of file
diff --git a/tools/testing/selftests/lib/slub.sh b/tools/testing/selftests/lib/slub.sh
new file mode 100755
index 000000000000..8b5757702910
--- /dev/null
+++ b/tools/testing/selftests/lib/slub.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0+
+$(dirname $0)/../kselftest/module.sh "slub" test_slub
-- 
2.17.1

[PATCH 2/2] slub: remove resiliency_test() function

[glittao]

From: Oliver Glitta <glittao@gmail.com>

Function resiliency_test() is hidden behind #ifdef
SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
runs it.

This function is replaced with kselftest for SLUB added
by the previous patch "selftests: add a kselftest for SLUB
debugging functionality".

Signed-off-by: Oliver Glitta <glittao@gmail.com>
---
 mm/slub.c | 64 -------------------------------------------------------
 1 file changed, 64 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index c00e2b263e03..bfeb9ecd56b4 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -153,9 +153,6 @@ static inline bool kmem_cache_has_cpu_partial(struct kmem_cache *s)
  * - Variable sizing of the per node arrays
  */
 
-/* Enable to test recovery from slab corruption on boot */
-#undef SLUB_RESILIENCY_TEST
-
 /* Enable to log cmpxchg failures */
 #undef SLUB_DEBUG_CMPXCHG
 
@@ -4910,66 +4907,6 @@ static int list_locations(struct kmem_cache *s, char *buf,
 }
 #endif	/* CONFIG_SLUB_DEBUG */
 
-#ifdef SLUB_RESILIENCY_TEST
-static void __init resiliency_test(void)
-{
-	u8 *p;
-	int type = KMALLOC_NORMAL;
-
-	BUILD_BUG_ON(KMALLOC_MIN_SIZE > 16 || KMALLOC_SHIFT_HIGH < 10);
-
-	pr_err("SLUB resiliency testing\n");
-	pr_err("-----------------------\n");
-	pr_err("A. Corruption after allocation\n");
-
-	p = kzalloc(16, GFP_KERNEL);
-	p[16] = 0x12;
-	pr_err("\n1. kmalloc-16: Clobber Redzone/next pointer 0x12->0x%p\n\n",
-	       p + 16);
-
-	validate_slab_cache(kmalloc_caches[type][4]);
-
-	/* Hmmm... The next two are dangerous */
-	p = kzalloc(32, GFP_KERNEL);
-	p[32 + sizeof(void *)] = 0x34;
-	pr_err("\n2. kmalloc-32: Clobber next pointer/next slab 0x34 -> -0x%p\n",
-	       p);
-	pr_err("If allocated object is overwritten then not detectable\n\n");
-
-	validate_slab_cache(kmalloc_caches[type][5]);
-	p = kzalloc(64, GFP_KERNEL);
-	p += 64 + (get_cycles() & 0xff) * sizeof(void *);
-	*p = 0x56;
-	pr_err("\n3. kmalloc-64: corrupting random byte 0x56->0x%p\n",
-	       p);
-	pr_err("If allocated object is overwritten then not detectable\n\n");
-	validate_slab_cache(kmalloc_caches[type][6]);
-
-	pr_err("\nB. Corruption after free\n");
-	p = kzalloc(128, GFP_KERNEL);
-	kfree(p);
-	*p = 0x78;
-	pr_err("1. kmalloc-128: Clobber first word 0x78->0x%p\n\n", p);
-	validate_slab_cache(kmalloc_caches[type][7]);
-
-	p = kzalloc(256, GFP_KERNEL);
-	kfree(p);
-	p[50] = 0x9a;
-	pr_err("\n2. kmalloc-256: Clobber 50th byte 0x9a->0x%p\n\n", p);
-	validate_slab_cache(kmalloc_caches[type][8]);
-
-	p = kzalloc(512, GFP_KERNEL);
-	kfree(p);
-	p[512] = 0xab;
-	pr_err("\n3. kmalloc-512: Clobber redzone 0xab->0x%p\n\n", p);
-	validate_slab_cache(kmalloc_caches[type][9]);
-}
-#else
-#ifdef CONFIG_SYSFS
-static void resiliency_test(void) {};
-#endif
-#endif	/* SLUB_RESILIENCY_TEST */
-
 #ifdef CONFIG_SYSFS
 enum slab_stat_type {
 	SL_ALL,			/* All slabs */
@@ -5819,7 +5756,6 @@ static int __init slab_sysfs_init(void)
 	}
 
 	mutex_unlock(&slab_mutex);
-	resiliency_test();
 	return 0;
 }
 
-- 
2.17.1

Re: [PATCH 2/2] slub: remove resiliency_test() function

[Vlastimil Babka]

On 3/16/21 1:41 PM, glittao@gmail.com wrote:
> From: Oliver Glitta <glittao@gmail.com>
> 
> Function resiliency_test() is hidden behind #ifdef
> SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
> runs it.
> 
> This function is replaced with kselftest for SLUB added
> by the previous patch "selftests: add a kselftest for SLUB
> debugging functionality".
> 
> Signed-off-by: Oliver Glitta <glittao@gmail.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Re: [PATCH 2/2] slub: remove resiliency_test() function

[David Rientjes]

On Tue, 16 Mar 2021, glittao@gmail.com wrote:

> From: Oliver Glitta <glittao@gmail.com>
> 
> Function resiliency_test() is hidden behind #ifdef
> SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
> runs it.
> 
> This function is replaced with kselftest for SLUB added
> by the previous patch "selftests: add a kselftest for SLUB
> debugging functionality".
> 
> Signed-off-by: Oliver Glitta <glittao@gmail.com>

Very nice!

Acked-by: David Rientjes <rientjes@google.com>

Re: [PATCH 1/2] selftests: add a kselftest for SLUB debugging functionality

[Vlastimil Babka]

On 3/16/21 1:41 PM, glittao@gmail.com wrote:
> From: Oliver Glitta <glittao@gmail.com>
> 
> SLUB has resiliency_test() function which is hidden behind #ifdef
> SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
> runs it. Kselftest should 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_TEST_SLUB in Kconfig.
> 
> Add parameter to function validate_slab_cache() to return
> number of errors in cache.
> 
> Signed-off-by: Oliver Glitta <glittao@gmail.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Disclaimer: this is done as part of Oliver's university project that I'm advising.

Re: [PATCH 1/2] selftests: add a kselftest for SLUB debugging functionality

[David Rientjes]

On Tue, 16 Mar 2021, glittao@gmail.com wrote:

> From: Oliver Glitta <glittao@gmail.com>
> 
> SLUB has resiliency_test() function which is hidden behind #ifdef
> SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
> runs it. Kselftest should 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_TEST_SLUB in Kconfig.
> 
> Add parameter to function validate_slab_cache() to return
> number of errors in cache.
> 
> Signed-off-by: Oliver Glitta <glittao@gmail.com>

Acked-by: David Rientjes <rientjes@google.com>

Re: [PATCH 1/2] selftests: add a kselftest for SLUB debugging functionality

[Marco Elver]

On Tue, Mar 16, 2021 at 01:41PM +0100, glittao@gmail.com wrote:
> From: Oliver Glitta <glittao@gmail.com>
> 
> SLUB has resiliency_test() function which is hidden behind #ifdef
> SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
> runs it. Kselftest should 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_TEST_SLUB in Kconfig.
> 
> Add parameter to function validate_slab_cache() to return
> number of errors in cache.
> 
> Signed-off-by: Oliver Glitta <glittao@gmail.com>

No objection per-se, but have you considered a KUnit-based test instead?

There is no user space portion required to run this test, and a pure
in-kernel KUnit test would be cleaner. Various boiler-plate below,
including pr_err()s, the kselftest script etc. would simply not be
necessary.

This is only a suggestion, but just want to make sure you've considered
the option and weighed its pros/cons.

Thanks,
-- Marco

> ---
>  lib/Kconfig.debug                    |   4 +
>  lib/Makefile                         |   1 +
>  lib/test_slub.c                      | 125 +++++++++++++++++++++++++++
>  mm/slab.h                            |   1 +
>  mm/slub.c                            |  34 +++++---
>  tools/testing/selftests/lib/Makefile |   2 +-
>  tools/testing/selftests/lib/config   |   1 +
>  tools/testing/selftests/lib/slub.sh  |   3 +
>  8 files changed, 159 insertions(+), 12 deletions(-)
>  create mode 100644 lib/test_slub.c
>  create mode 100755 tools/testing/selftests/lib/slub.sh
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 2779c29d9981..2d56092abbc4 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2123,6 +2123,10 @@ config TEST_KSTRTOX
>  config TEST_PRINTF
>  	tristate "Test printf() family of functions at runtime"
>  
> +config TEST_SLUB
> +	tristate "Test SLUB cache errors at runtime"
> +	depends on SLUB_DEBUG
> +
>  config TEST_BITMAP
>  	tristate "Test bitmap_*() family of functions at runtime"
>  	help
> diff --git a/lib/Makefile b/lib/Makefile
> index b5307d3eec1a..b6603803b1c4 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
>  obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
>  obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
>  obj-$(CONFIG_TEST_PRINTF) += test_printf.o
> +obj-$(CONFIG_TEST_SLUB) += test_slub.o
>  obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
>  obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o
>  obj-$(CONFIG_TEST_UUID) += test_uuid.o
> diff --git a/lib/test_slub.c b/lib/test_slub.c
> new file mode 100644
> index 000000000000..0075d9b44251
> --- /dev/null
> +++ b/lib/test_slub.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Test cases for slub facility.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include "../mm/slab.h"
> +
> +#include "../tools/testing/selftests/kselftest_module.h"
> +
> +
> +KSTM_MODULE_GLOBALS();
> +
> +
> +static void __init validate_result(struct kmem_cache *s, int expected_errors)
> +{
> +	int errors = 0;
> +
> +	validate_slab_cache(s, &errors);
> +	KSTM_CHECK_ZERO(errors - expected_errors);
> +}
> +
> +static void __init test_clobber_zone(void)
> +{
> +	struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_alloc", 64, 0,
> +				SLAB_RED_ZONE, NULL);
> +	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
> +
> +	p[64] = 0x12;
> +	pr_err("1. kmem_cache: Clobber Redzone 0x12->0x%p\n", p + 64);
> +
> +	validate_result(s, 1);
> +	kmem_cache_free(s, p);
> +	kmem_cache_destroy(s);
> +}
> +
> +static void __init test_next_pointer(void)
> +{
> +	struct kmem_cache *s = kmem_cache_create("TestSlub_next_ptr_free", 64, 0,
> +				SLAB_RED_ZONE, NULL);
> +	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
> +
> +	kmem_cache_free(s, p);
> +	p[s->offset] = 0x12;
> +	pr_err("1. kmem_cache: Clobber next pointer 0x34 -> -0x%p\n", p);
> +
> +	validate_result(s, 1);
> +	kmem_cache_destroy(s);
> +}
> +
> +static void __init test_first_word(void)
> +{
> +	struct kmem_cache *s = kmem_cache_create("TestSlub_1th_word_free", 64, 0,
> +				SLAB_POISON, NULL);
> +	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
> +
> +	kmem_cache_free(s, p);
> +	*p = 0x78;
> +	pr_err("2. kmem_cache: Clobber first word 0x78->0x%p\n", p);
> +
> +	validate_result(s, 1);
> +	kmem_cache_destroy(s);
> +}
> +
> +static void __init test_clobber_50th_byte(void)
> +{
> +	struct kmem_cache *s = kmem_cache_create("TestSlub_50th_word_free", 64, 0,
> +				SLAB_POISON, NULL);
> +	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
> +
> +	kmem_cache_free(s, p);
> +	p[50] = 0x9a;
> +	pr_err("3. kmem_cache: Clobber 50th byte 0x9a->0x%p\n", p);
> +
> +	validate_result(s, 1);
> +	kmem_cache_destroy(s);
> +}
> +
> +static void __init test_clobber_redzone_free(void)
> +{
> +	struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_free", 64, 0,
> +				SLAB_RED_ZONE, NULL);
> +	u8 *p = kmem_cache_alloc(s, GFP_KERNEL);
> +
> +	kmem_cache_free(s, p);
> +	p[64] = 0xab;
> +	pr_err("4. kmem_cache: Clobber redzone 0xab->0x%p\n", p);
> +
> +	validate_result(s, 1);
> +	kmem_cache_destroy(s);
> +}
> +
> +static void __init resiliency_test(void)
> +{
> +
> +	BUILD_BUG_ON(KMALLOC_MIN_SIZE > 16 || KMALLOC_SHIFT_HIGH < 10);
> +
> +	pr_err("SLUB resiliency testing\n");
> +	pr_err("-----------------------\n");
> +	pr_err("A. Corruption after allocation\n");
> +
> +	test_clobber_zone();
> +
> +	pr_err("\nB. Corruption after free\n");
> +
> +	test_next_pointer();
> +	test_first_word();
> +	test_clobber_50th_byte();
> +	test_clobber_redzone_free();
> +}
> +
> +
> +static void __init selftest(void)
> +{
> +	resiliency_test();
> +}
> +
> +
> +KSTM_MODULE_LOADERS(test_slub);
> +MODULE_LICENSE("GPL");
> diff --git a/mm/slab.h b/mm/slab.h
> index 076582f58f68..5fc18d506b3b 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -215,6 +215,7 @@ DECLARE_STATIC_KEY_TRUE(slub_debug_enabled);
>  DECLARE_STATIC_KEY_FALSE(slub_debug_enabled);
>  #endif
>  extern void print_tracking(struct kmem_cache *s, void *object);
> +long validate_slab_cache(struct kmem_cache *s, int *errors);
>  #else
>  static inline void print_tracking(struct kmem_cache *s, void *object)
>  {
> diff --git a/mm/slub.c b/mm/slub.c
> index e26c274b4657..c00e2b263e03 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4612,7 +4612,8 @@ static int count_total(struct page *page)
>  #endif
>  
>  #ifdef CONFIG_SLUB_DEBUG
> -static void validate_slab(struct kmem_cache *s, struct page *page)
> +static void validate_slab(struct kmem_cache *s, struct page *page,
> +						int *errors)
>  {
>  	void *p;
>  	void *addr = page_address(page);
> @@ -4620,8 +4621,10 @@ static void validate_slab(struct kmem_cache *s, struct page *page)
>  
>  	slab_lock(page);
>  
> -	if (!check_slab(s, page) || !on_freelist(s, page, NULL))
> +	if (!check_slab(s, page) || !on_freelist(s, page, NULL)) {
> +		*errors += 1;
>  		goto unlock;
> +	}
>  
>  	/* Now we know that a valid freelist exists */
>  	map = get_map(s, page);
> @@ -4629,8 +4632,10 @@ static void validate_slab(struct kmem_cache *s, struct page *page)
>  		u8 val = test_bit(__obj_to_index(s, addr, p), map) ?
>  			 SLUB_RED_INACTIVE : SLUB_RED_ACTIVE;
>  
> -		if (!check_object(s, page, p, val))
> +		if (!check_object(s, page, p, val)) {
> +			*errors += 1;
>  			break;
> +		}
>  	}
>  	put_map(map);
>  unlock:
> @@ -4638,7 +4643,7 @@ static void validate_slab(struct kmem_cache *s, struct page *page)
>  }
>  
>  static int validate_slab_node(struct kmem_cache *s,
> -		struct kmem_cache_node *n)
> +		struct kmem_cache_node *n, int *errors)
>  {
>  	unsigned long count = 0;
>  	struct page *page;
> @@ -4647,30 +4652,34 @@ static int validate_slab_node(struct kmem_cache *s,
>  	spin_lock_irqsave(&n->list_lock, flags);
>  
>  	list_for_each_entry(page, &n->partial, slab_list) {
> -		validate_slab(s, page);
> +		validate_slab(s, page, errors);
>  		count++;
>  	}
> -	if (count != n->nr_partial)
> +	if (count != n->nr_partial) {
>  		pr_err("SLUB %s: %ld partial slabs counted but counter=%ld\n",
>  		       s->name, count, n->nr_partial);
> +		*errors += 1;
> +	}
>  
>  	if (!(s->flags & SLAB_STORE_USER))
>  		goto out;
>  
>  	list_for_each_entry(page, &n->full, slab_list) {
> -		validate_slab(s, page);
> +		validate_slab(s, page, errors);
>  		count++;
>  	}
> -	if (count != atomic_long_read(&n->nr_slabs))
> +	if (count != atomic_long_read(&n->nr_slabs)) {
>  		pr_err("SLUB: %s %ld slabs counted but counter=%ld\n",
>  		       s->name, count, atomic_long_read(&n->nr_slabs));
> +		*errors += 1;
> +	}
>  
>  out:
>  	spin_unlock_irqrestore(&n->list_lock, flags);
>  	return count;
>  }
>  
> -static long validate_slab_cache(struct kmem_cache *s)
> +long validate_slab_cache(struct kmem_cache *s, int *errors)
>  {
>  	int node;
>  	unsigned long count = 0;
> @@ -4678,10 +4687,12 @@ static long validate_slab_cache(struct kmem_cache *s)
>  
>  	flush_all(s);
>  	for_each_kmem_cache_node(s, node, n)
> -		count += validate_slab_node(s, n);
> +		count += validate_slab_node(s, n, errors);
>  
>  	return count;
>  }
> +EXPORT_SYMBOL(validate_slab_cache);
> +
>  /*
>   * Generate lists of code addresses where slabcache objects are allocated
>   * and freed.
> @@ -5336,9 +5347,10 @@ static ssize_t validate_store(struct kmem_cache *s,
>  			const char *buf, size_t length)
>  {
>  	int ret = -EINVAL;
> +	int errors = 0;
>  
>  	if (buf[0] == '1') {
> -		ret = validate_slab_cache(s);
> +		ret = validate_slab_cache(s, &errors);
>  		if (ret >= 0)
>  			ret = length;
>  	}
> diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile
> index a105f094676e..f168313b7949 100644
> --- a/tools/testing/selftests/lib/Makefile
> +++ b/tools/testing/selftests/lib/Makefile
> @@ -4,6 +4,6 @@
>  # No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
>  all:
>  
> -TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh strscpy.sh
> +TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh strscpy.sh slub.sh
>  
>  include ../lib.mk
> diff --git a/tools/testing/selftests/lib/config b/tools/testing/selftests/lib/config
> index b80ee3f6e265..4190863032e7 100644
> --- a/tools/testing/selftests/lib/config
> +++ b/tools/testing/selftests/lib/config
> @@ -3,3 +3,4 @@ CONFIG_TEST_BITMAP=m
>  CONFIG_PRIME_NUMBERS=m
>  CONFIG_TEST_STRSCPY=m
>  CONFIG_TEST_BITOPS=m
> +CONFIG_TEST_SLUB=m
> \ No newline at end of file
> diff --git a/tools/testing/selftests/lib/slub.sh b/tools/testing/selftests/lib/slub.sh
> new file mode 100755
> index 000000000000..8b5757702910
> --- /dev/null
> +++ b/tools/testing/selftests/lib/slub.sh
> @@ -0,0 +1,3 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0+
> +$(dirname $0)/../kselftest/module.sh "slub" test_slub

Re: [PATCH 1/2] selftests: add a kselftest for SLUB debugging functionality

[Vlastimil Babka]

On 3/18/21 12:47 PM, Marco Elver wrote:
> On Tue, Mar 16, 2021 at 01:41PM +0100, glittao@gmail.com wrote:
>> From: Oliver Glitta <glittao@gmail.com>
>> 
>> SLUB has resiliency_test() function which is hidden behind #ifdef
>> SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
>> runs it. Kselftest should 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_TEST_SLUB in Kconfig.
>> 
>> Add parameter to function validate_slab_cache() to return
>> number of errors in cache.
>> 
>> Signed-off-by: Oliver Glitta <glittao@gmail.com>
> 
> No objection per-se, but have you considered a KUnit-based test instead?

To be honest, we didn't realize about that option.

> There is no user space portion required to run this test, and a pure
> in-kernel KUnit test would be cleaner. Various boiler-plate below,
> including pr_err()s, the kselftest script etc. would simply not be
> necessary.
> 
> This is only a suggestion, but just want to make sure you've considered
> the option and weighed its pros/cons.

Thanks for the suggestion. But I hope we would expand the tests later to e.g.
check the contents of various SLUB related sysfs files or even write to them,
and for that goal kselftest seems to be a better starting place?

Vlastimil

Re: [PATCH 1/2] selftests: add a kselftest for SLUB debugging functionality

[Marco Elver]

On Fri, 19 Mar 2021 at 11:46, Vlastimil Babka <vbabka@suse.cz> wrote:
> On 3/18/21 12:47 PM, Marco Elver wrote:
> > On Tue, Mar 16, 2021 at 01:41PM +0100, glittao@gmail.com wrote:
> >> From: Oliver Glitta <glittao@gmail.com>
> >>
> >> SLUB has resiliency_test() function which is hidden behind #ifdef
> >> SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody
> >> runs it. Kselftest should 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_TEST_SLUB in Kconfig.
> >>
> >> Add parameter to function validate_slab_cache() to return
> >> number of errors in cache.
> >>
> >> Signed-off-by: Oliver Glitta <glittao@gmail.com>
> >
> > No objection per-se, but have you considered a KUnit-based test instead?
>
> To be honest, we didn't realize about that option.
>
> > There is no user space portion required to run this test, and a pure
> > in-kernel KUnit test would be cleaner. Various boiler-plate below,
> > including pr_err()s, the kselftest script etc. would simply not be
> > necessary.
> >
> > This is only a suggestion, but just want to make sure you've considered
> > the option and weighed its pros/cons.
>
> Thanks for the suggestion. But I hope we would expand the tests later to e.g.
> check the contents of various SLUB related sysfs files or even write to them,
> and for that goal kselftest seems to be a better starting place?

Not sure, but I would probably go about it this way:

A. Anything that is purely in-kernel and doesn't require a user space
component should be a KUnit test.

B. For any test that requires a user space component, it'd be a kselftest.

And I think the best design here would also clearly separate those 2
types of tests, and I wouldn't lump tests of type A into modules that
are also used for B. That way, running tests of type A also is a bit
easier, and if somebody wants to just quickly run those it's e.g. very
quick to do so with kunit-tool.

Thanks,
-- Marco