* Re: [PATCH 2/2] lib/tests: Add KUnit test for struct stack_trace __counted_by_ptr attribute
2026-08-23 12:35 ` [PATCH 2/2] lib/tests: Add KUnit test for struct stack_trace __counted_by_ptr attribute Bill Wendling
@ 2026-08-23 15:11 ` Kuan-Wei Chiu
2026-08-24 6:41 ` Thomas Weißschuh
2026-08-25 5:07 ` David Gow
2 siblings, 0 replies; 6+ messages in thread
From: Kuan-Wei Chiu @ 2026-08-23 15:11 UTC (permalink / raw)
To: Bill Wendling
Cc: linux-kernel, Kees Cook, Gustavo A. R. Silva, Andrew Morton,
Brendan Higgins, David Gow, Rae Moar, Ryota Sakamoto,
Pasha Tatashin, Dmitry Antipov, Petr Mladek, Kir Chou,
codemender-patching+linux, linux-hardening, linux-kselftest,
kunit-dev
Hi Bill,
On Sun, Aug 23, 2026 at 12:35:33PM +0000, Bill Wendling wrote:
> Add a custom KUnit test suite 'stacktrace_counted_by' to verify that the
> __counted_by_ptr annotation on the 'entries' field of 'struct stack_trace'
> behaves correctly.
>
> The test verifies that 'max_entries' correctly limits and validates access
> to 'entries' when CONFIG_ARCH_STACKWALK is not defined. If it is defined,
> the test is cleanly skipped at runtime to prevent compile-time or runtime
> failures due to 'struct stack_trace' being undefined on modern
> architectures.
>
> Assisted-by: Gemini Next
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> Cc: Kees Cook <kees@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Brendan Higgins <brendan.higgins@linux.dev>
> Cc: David Gow <david@davidgow.net>
> Cc: Rae Moar <raemoar63@gmail.com>
> Cc: Ryota Sakamoto <sakamo.ryota@gmail.com>
> Cc: Kuan-Wei Chiu <visitorckw@gmail.com>
> Cc: Pasha Tatashin <pasha.tatashin@soleen.com>
> Cc: Dmitry Antipov <dmantipov@yandex.ru>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Kir Chou <note351@hotmail.com>
> Cc: codemender-patching+linux@google.com
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-hardening@vger.kernel.org
> Cc: linux-kselftest@vger.kernel.org
> Cc: kunit-dev@googlegroups.com
> Cc: linux-hardening@vger.kernel.org
> ---
> lib/Kconfig.debug | 10 +++++++
> lib/kunit/.kunitconfig | 1 +
> lib/tests/Makefile | 1 +
> lib/tests/stacktrace_kunit.c | 51 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 63 insertions(+)
> create mode 100644 lib/tests/stacktrace_kunit.c
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index e97bdf3a42a8..51a6ac1a2461 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2716,6 +2716,16 @@ config BITOPS_KUNIT
>
> If unsure, say N.
>
> +config STACKTRACE_KUNIT_TEST
> + tristate "KUnit test for stacktrace counted_by attribute" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + This option enables the KUnit test for verifying the __counted_by_ptr
> + attribute on struct stack_trace.
> +
> + If unsure, say N.
> +
> config BITFIELD_KUNIT
> tristate "KUnit test bitfield functions at runtime" if !KUNIT_ALL_TESTS
> depends on KUNIT
> diff --git a/lib/kunit/.kunitconfig b/lib/kunit/.kunitconfig
> index 9235b7d42d38..b3761b41459e 100644
> --- a/lib/kunit/.kunitconfig
> +++ b/lib/kunit/.kunitconfig
> @@ -1,3 +1,4 @@
> CONFIG_KUNIT=y
> CONFIG_KUNIT_TEST=y
> CONFIG_KUNIT_EXAMPLE_TEST=y
> +CONFIG_STACKTRACE_KUNIT_TEST=y
> diff --git a/lib/tests/Makefile b/lib/tests/Makefile
> index 4ead57602eac..40875e729fc8 100644
> --- a/lib/tests/Makefile
> +++ b/lib/tests/Makefile
> @@ -6,6 +6,7 @@
> CFLAGS_bitfield_kunit.o := $(DISABLE_STRUCTLEAK_PLUGIN)
> obj-$(CONFIG_BASE64_KUNIT) += base64_kunit.o
> obj-$(CONFIG_BITOPS_KUNIT) += bitops_kunit.o
> +obj-$(CONFIG_STACKTRACE_KUNIT_TEST) += stacktrace_kunit.o
> obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
> obj-$(CONFIG_BITS_TEST) += test_bits.o
> obj-$(CONFIG_SHDI3_KUNIT_TEST) += shdi3_kunit.o
> diff --git a/lib/tests/stacktrace_kunit.c b/lib/tests/stacktrace_kunit.c
> new file mode 100644
> index 000000000000..7ec48edf84fe
> --- /dev/null
> +++ b/lib/tests/stacktrace_kunit.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for struct stack_trace counted_by attribute.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/stacktrace.h>
> +
> +#ifndef CONFIG_ARCH_STACKWALK
> +static void test_stack_trace_counted_by(struct kunit *test)
> +{
> + unsigned long entries_buf[4];
> + struct stack_trace trace = {
> + .entries = entries_buf,
> + .max_entries = 4,
> + };
> +
> + KUNIT_EXPECT_EQ(test, trace.max_entries, 4U);
> + KUNIT_EXPECT_PTR_EQ(test, trace.entries, (unsigned long *)entries_buf);
> +
> + /* Write to the allocated elements to verify access */
> + trace.entries[0] = 0xdeadbeef;
> + trace.entries[1] = 0xbeefcafe;
> + trace.entries[2] = 0xcafebabe;
> + trace.entries[3] = 0x12345678;
This only does in bounds array writes.
To test __counted_by_ptr, I thought we were supposed to intentionally
trigger an out of bounds access and see if it actually catches the
error?
Regards,
Kuan-Wei
> +
> + KUNIT_EXPECT_EQ(test, trace.entries[0], 0xdeadbeefUL);
> + KUNIT_EXPECT_EQ(test, trace.entries[1], 0xbeefcafeUL);
> + KUNIT_EXPECT_EQ(test, trace.entries[2], 0xcafebabeUL);
> + KUNIT_EXPECT_EQ(test, trace.entries[3], 0x12345678UL);
> +}
> +#else
> +static void test_stack_trace_counted_by(struct kunit *test)
> +{
> + kunit_skip(test, "CONFIG_ARCH_STACKWALK is enabled, struct stack_trace is not defined");
> +}
> +#endif
> +
> +static struct kunit_case stacktrace_test_cases[] = {
> + KUNIT_CASE(test_stack_trace_counted_by),
> + {}
> +};
> +
> +static struct kunit_suite stacktrace_test_suite = {
> + .name = "stacktrace_counted_by",
> + .test_cases = stacktrace_test_cases,
> +};
> +
> +kunit_test_suite(stacktrace_test_suite);
> +
> +MODULE_LICENSE("GPL");
> --
> 2.55.0.860.g4b6b3295ed-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] lib/tests: Add KUnit test for struct stack_trace __counted_by_ptr attribute
2026-08-23 12:35 ` [PATCH 2/2] lib/tests: Add KUnit test for struct stack_trace __counted_by_ptr attribute Bill Wendling
2026-08-23 15:11 ` Kuan-Wei Chiu
@ 2026-08-24 6:41 ` Thomas Weißschuh
2026-08-25 5:07 ` David Gow
2 siblings, 0 replies; 6+ messages in thread
From: Thomas Weißschuh @ 2026-08-24 6:41 UTC (permalink / raw)
To: Bill Wendling
Cc: linux-kernel, Kees Cook, Gustavo A. R. Silva, Andrew Morton,
Brendan Higgins, David Gow, Rae Moar, Ryota Sakamoto,
Kuan-Wei Chiu, Pasha Tatashin, Dmitry Antipov, Petr Mladek,
Kir Chou, codemender-patching+linux, linux-hardening,
linux-kselftest, kunit-dev
On Sun, Aug 23, 2026 at 12:35:33PM +0000, Bill Wendling wrote:
> Add a custom KUnit test suite 'stacktrace_counted_by' to verify that the
> __counted_by_ptr annotation on the 'entries' field of 'struct stack_trace'
> behaves correctly.
The implementation of __counted_by_ptr should be tested by a dedicated unittest
for that feature. And I am fairly sure that already exists. It should not be
tested in random subsystems.
> The test verifies that 'max_entries' correctly limits and validates access
> to 'entries' when CONFIG_ARCH_STACKWALK is not defined. If it is defined,
> the test is cleanly skipped at runtime to prevent compile-time or runtime
> failures due to 'struct stack_trace' being undefined on modern
> architectures.
>
> Assisted-by: Gemini Next
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> Cc: Kees Cook <kees@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Brendan Higgins <brendan.higgins@linux.dev>
> Cc: David Gow <david@davidgow.net>
> Cc: Rae Moar <raemoar63@gmail.com>
> Cc: Ryota Sakamoto <sakamo.ryota@gmail.com>
> Cc: Kuan-Wei Chiu <visitorckw@gmail.com>
> Cc: Pasha Tatashin <pasha.tatashin@soleen.com>
> Cc: Dmitry Antipov <dmantipov@yandex.ru>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Kir Chou <note351@hotmail.com>
> Cc: codemender-patching+linux@google.com
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-hardening@vger.kernel.org
> Cc: linux-kselftest@vger.kernel.org
> Cc: kunit-dev@googlegroups.com
> Cc: linux-hardening@vger.kernel.org
> ---
> lib/Kconfig.debug | 10 +++++++
> lib/kunit/.kunitconfig | 1 +
> lib/tests/Makefile | 1 +
> lib/tests/stacktrace_kunit.c | 51 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 63 insertions(+)
> create mode 100644 lib/tests/stacktrace_kunit.c
(...)
> diff --git a/lib/tests/stacktrace_kunit.c b/lib/tests/stacktrace_kunit.c
> new file mode 100644
> index 000000000000..7ec48edf84fe
> --- /dev/null
> +++ b/lib/tests/stacktrace_kunit.c
> @@ -0,0 +1,51 @@
(...)
> +}
> +#else
> +static void test_stack_trace_counted_by(struct kunit *test)
> +{
> + kunit_skip(test, "CONFIG_ARCH_STACKWALK is enabled, struct stack_trace is not defined");
If the test requires a specific kconfig symbol, then depend on it in kconfig.
Putting all of the code behind a single, giant ifdef is pointless.
> +}
> +#endif
(...)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] lib/tests: Add KUnit test for struct stack_trace __counted_by_ptr attribute
2026-08-23 12:35 ` [PATCH 2/2] lib/tests: Add KUnit test for struct stack_trace __counted_by_ptr attribute Bill Wendling
2026-08-23 15:11 ` Kuan-Wei Chiu
2026-08-24 6:41 ` Thomas Weißschuh
@ 2026-08-25 5:07 ` David Gow
2 siblings, 0 replies; 6+ messages in thread
From: David Gow @ 2026-08-25 5:07 UTC (permalink / raw)
To: Bill Wendling, linux-kernel
Cc: Kees Cook, Gustavo A. R. Silva, Andrew Morton, Brendan Higgins,
Rae Moar, Ryota Sakamoto, Kuan-Wei Chiu, Pasha Tatashin,
Dmitry Antipov, Petr Mladek, Kir Chou, codemender-patching+linux,
linux-hardening, linux-kselftest, kunit-dev
Le 23/08/2026 à 20:35, Bill Wendling a écrit :
> Add a custom KUnit test suite 'stacktrace_counted_by' to verify that the
> __counted_by_ptr annotation on the 'entries' field of 'struct stack_trace'
> behaves correctly.
>
> The test verifies that 'max_entries' correctly limits and validates access
> to 'entries' when CONFIG_ARCH_STACKWALK is not defined. If it is defined,
> the test is cleanly skipped at runtime to prevent compile-time or runtime
> failures due to 'struct stack_trace' being undefined on modern
> architectures.
>
> Assisted-by: Gemini Next
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> Cc: Kees Cook <kees@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Brendan Higgins <brendan.higgins@linux.dev>
> Cc: David Gow <david@davidgow.net>
> Cc: Rae Moar <raemoar63@gmail.com>
> Cc: Ryota Sakamoto <sakamo.ryota@gmail.com>
> Cc: Kuan-Wei Chiu <visitorckw@gmail.com>
> Cc: Pasha Tatashin <pasha.tatashin@soleen.com>
> Cc: Dmitry Antipov <dmantipov@yandex.ru>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Kir Chou <note351@hotmail.com>
> Cc: codemender-patching+linux@google.com
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-hardening@vger.kernel.org
> Cc: linux-kselftest@vger.kernel.org
> Cc: kunit-dev@googlegroups.com
> Cc: linux-hardening@vger.kernel.org
> ---
I'm a bit confused: what is this actually testing? You're creating an
array of size 4 and writing 4 entries to it, after setting max_entries
correctly? Removing the __counted_by_ptr attribute doesn't change
anything here. Is there any circumstance where this should fail? The
only case I can think of is if __counted_by_ptr is either totally
broken, or someone randomly points it to a different value. Neither of
which seem likely.
Even if this did something more exciting (like try to access elements
beyond the end of the array), I'm not sure a KUnit test is the optimal
place for it: ultimately we're looking for a compiler warning / error,
not a runtime one, and even then, one which probably isn't of much value
given it's unlikely __counted_by_ptr will spontaneously break.
Added to that, there are a bunch of stylistic issues with the test below
anyway. These mostly seem like LLM-isms, but probably should've been
caught before this went to the list.
Unless there's something I'm missing (and, if so, it's probably worth
writing a description which better explains the "why" here), I think we
can probably drop this test entirely and stick to just patch 1.
-- David
> lib/Kconfig.debug | 10 +++++++
> lib/kunit/.kunitconfig | 1 +
> lib/tests/Makefile | 1 +
> lib/tests/stacktrace_kunit.c | 51 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 63 insertions(+)
> create mode 100644 lib/tests/stacktrace_kunit.c
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index e97bdf3a42a8..51a6ac1a2461 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2716,6 +2716,16 @@ config BITOPS_KUNIT
>
> If unsure, say N.
>
> +config STACKTRACE_KUNIT_TEST
> + tristate "KUnit test for stacktrace counted_by attribute" if !KUNIT_ALL_TESTS
> + depends on KUNIT
This should also just depend on CONFIG_ARCH_STACKWALK, rather than doing
the whole skip test thing.
> + default KUNIT_ALL_TESTS
> + help
> + This option enables the KUnit test for verifying the __counted_by_ptr
> + attribute on struct stack_trace.
> +
> + If unsure, say N.
> +
> config BITFIELD_KUNIT
> tristate "KUnit test bitfield functions at runtime" if !KUNIT_ALL_TESTS
> depends on KUNIT
> diff --git a/lib/kunit/.kunitconfig b/lib/kunit/.kunitconfig
> index 9235b7d42d38..b3761b41459e 100644
> --- a/lib/kunit/.kunitconfig
> +++ b/lib/kunit/.kunitconfig
> @@ -1,3 +1,4 @@
> CONFIG_KUNIT=y
> CONFIG_KUNIT_TEST=y
> CONFIG_KUNIT_EXAMPLE_TEST=y
> +CONFIG_STACKTRACE_KUNIT_TEST=y
Please don't add this here. This is not a test of KUnit itself, which is
what the lib/kunit/.kunitconfig file is meant to configure.
For general KUnit test build, CONFIG_KUNIT_ALL_TESTS will enable it.
> diff --git a/lib/tests/Makefile b/lib/tests/Makefile
> index 4ead57602eac..40875e729fc8 100644
> --- a/lib/tests/Makefile
> +++ b/lib/tests/Makefile
> @@ -6,6 +6,7 @@
> CFLAGS_bitfield_kunit.o := $(DISABLE_STRUCTLEAK_PLUGIN)
> obj-$(CONFIG_BASE64_KUNIT) += base64_kunit.o
> obj-$(CONFIG_BITOPS_KUNIT) += bitops_kunit.o
> +obj-$(CONFIG_STACKTRACE_KUNIT_TEST) += stacktrace_kunit.o
> obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
> obj-$(CONFIG_BITS_TEST) += test_bits.o
> obj-$(CONFIG_SHDI3_KUNIT_TEST) += shdi3_kunit.o
> diff --git a/lib/tests/stacktrace_kunit.c b/lib/tests/stacktrace_kunit.c
> new file mode 100644
> index 000000000000..7ec48edf84fe
> --- /dev/null
> +++ b/lib/tests/stacktrace_kunit.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for struct stack_trace counted_by attribute.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/stacktrace.h>
> +
> +#ifndef CONFIG_ARCH_STACKWALK
> +static void test_stack_trace_counted_by(struct kunit *test)
> +{
> + unsigned long entries_buf[4];
> + struct stack_trace trace = {
> + .entries = entries_buf,
> + .max_entries = 4,
> + };
> +
> + KUNIT_EXPECT_EQ(test, trace.max_entries, 4U);
> + KUNIT_EXPECT_PTR_EQ(test, trace.entries, (unsigned long *)entries_buf);
> +
> + /* Write to the allocated elements to verify access */
> + trace.entries[0] = 0xdeadbeef;
> + trace.entries[1] = 0xbeefcafe;
> + trace.entries[2] = 0xcafebabe;
> + trace.entries[3] = 0x12345678;
> +
> + KUNIT_EXPECT_EQ(test, trace.entries[0], 0xdeadbeefUL);
> + KUNIT_EXPECT_EQ(test, trace.entries[1], 0xbeefcafeUL);
> + KUNIT_EXPECT_EQ(test, trace.entries[2], 0xcafebabeUL);
> + KUNIT_EXPECT_EQ(test, trace.entries[3], 0x12345678UL);
> +}
> +#else
> +static void test_stack_trace_counted_by(struct kunit *test)
> +{
> + kunit_skip(test, "CONFIG_ARCH_STACKWALK is enabled, struct stack_trace is not defined");
> +}
> +#endif
> +
> +static struct kunit_case stacktrace_test_cases[] = {
> + KUNIT_CASE(test_stack_trace_counted_by),
> + {}
> +};
> +
> +static struct kunit_suite stacktrace_test_suite = {
> + .name = "stacktrace_counted_by",
> + .test_cases = stacktrace_test_cases,
> +};
Is this suite intended to cover more stacktrace tests than just the
counted_by one at some point? If so, it'd be better to name it
"stacktrace". If not, these structs should be named
"stacktrace_counted_by_test_suite", etc.
> +
> +kunit_test_suite(stacktrace_test_suite);
> +
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 6+ messages in thread