| 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
| ||
|
Message-ID: <CABVgOSkf+v6vGvUCQ6eo3_RXZvmzRpLgA1PmpQ-Bd90iD4yzZA@mail.gmail.com>
Date: Sat, 26 Jul 2025 17:08:19 +0800
From: David Gow <davidgow@...gle.com>
To: Kees Cook <kees@...nel.org>
Cc: Rae Moar <rmoar@...gle.com>, Tamir Duberstein <tamird@...il.com>,
Eric Biggers <ebiggers@...nel.org>, "Steven Rostedt (Google)" <rostedt@...dmis.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Petr Mladek <pmladek@...e.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>, Luis Chamberlain <mcgrof@...nel.org>,
"Paul E. McKenney" <paulmck@...nel.org>, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH] seq_buf: Introduce KUnit tests
On Thu, 17 Jul 2025 at 16:52, Kees Cook <kees@...nel.org> wrote:
>
> Add KUnit tests for the seq_buf API to ensure its correctness and
> prevent future regressions.
>
> The tests cover the following functions:
> - seq_buf_init()
> - DECLARE_SEQ_BUF()
> - seq_buf_clear()
> - seq_buf_puts()
> - seq_buf_putc()
> - seq_buf_printf()
> - seq_buf_get_buf()
> - seq_buf_commit()
>
> $ tools/testing/kunit/kunit.py run seq_buf
> =================== seq_buf (9 subtests) ===================
> [PASSED] seq_buf_init_test
> [PASSED] seq_buf_declare_test
> [PASSED] seq_buf_clear_test
> [PASSED] seq_buf_puts_test
> [PASSED] seq_buf_puts_overflow_test
> [PASSED] seq_buf_putc_test
> [PASSED] seq_buf_printf_test
> [PASSED] seq_buf_printf_overflow_test
> [PASSED] seq_buf_get_buf_commit_test
> ===================== [PASSED] seq_buf =====================
>
> Signed-off-by: Kees Cook <kees@...nel.org>
> ---
> I used an LLM to produce this; it did pretty well, but I had to help it
> get the Kconfig and make targets in the right places, and I tweaked some
> of the edge cases and added a bit more (perhaps redundant) state checking.
>
This patch looks good to me. A couple of very minor notes below, but
nothing I think is a big issue.
I don't have much to say on the LLM use -- it definitely looks better
than the last time I tried it. If something like Sasha's suggestion
for how to tag LLM contributions[1] goes through, I do think noting
what was used could be a useful thing to have.
Reviewed-by: David Gow <davidgow@...gle.com>
-- David
[1] https://lore.kernel.org/all/20250725175358.1989323-1-sashal@kernel.org/
> Cc: David Gow <davidgow@...gle.com>
> Cc: Rae Moar <rmoar@...gle.com>
> Cc: Tamir Duberstein <tamird@...il.com>
> Cc: Eric Biggers <ebiggers@...nel.org>
> Cc: "Steven Rostedt (Google)" <rostedt@...dmis.org>
> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Cc: Petr Mladek <pmladek@...e.com>
> Cc: "Matthew Wilcox (Oracle)" <willy@...radead.org>
> ---
> lib/Kconfig.debug | 9 ++
> lib/tests/Makefile | 1 +
> lib/tests/seq_buf_kunit.c | 205 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 215 insertions(+)
> create mode 100644 lib/tests/seq_buf_kunit.c
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index cf05bf1df983..048efc3183d5 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2470,6 +2470,15 @@ config SCANF_KUNIT_TEST
>
> If unsure, say N.
>
> +config SEQ_BUF_KUNIT_TEST
> + tristate "KUnit test for seq_buf" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + This builds unit tests for the seq_buf library.
> +
> + If unsure, say N.
> +
FYI: Checkpatch complains a bit that this is too short a description.
It doesn't bother me much, though:
WARNING: please write a help paragraph that fully describes the config
symbol with at least 4 lines
> config STRING_KUNIT_TEST
> tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS
> depends on KUNIT
> diff --git a/lib/tests/Makefile b/lib/tests/Makefile
> index 84b15c986b8c..fa6d728a8b5b 100644
> --- a/lib/tests/Makefile
> +++ b/lib/tests/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
> obj-$(CONFIG_PRINTF_KUNIT_TEST) += printf_kunit.o
> obj-$(CONFIG_RANDSTRUCT_KUNIT_TEST) += randstruct_kunit.o
> obj-$(CONFIG_SCANF_KUNIT_TEST) += scanf_kunit.o
> +obj-$(CONFIG_SEQ_BUF_KUNIT_TEST) += seq_buf_kunit.o
> obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
> obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
> obj-$(CONFIG_TEST_SORT) += test_sort.o
> diff --git a/lib/tests/seq_buf_kunit.c b/lib/tests/seq_buf_kunit.c
> new file mode 100644
> index 000000000000..74648dbda13f
> --- /dev/null
> +++ b/lib/tests/seq_buf_kunit.c
> @@ -0,0 +1,205 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit tests for the seq_buf API
> + *
> + * Copyright (C) 2025, Google LLC.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/seq_buf.h>
> +
> +static void seq_buf_init_test(struct kunit *test)
> +{
> + char buf[32];
> + struct seq_buf s;
> +
> + seq_buf_init(&s, buf, sizeof(buf));
> +
> + KUNIT_EXPECT_EQ(test, s.size, 32);
> + KUNIT_EXPECT_EQ(test, s.len, 0);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_EQ(test, seq_buf_buffer_left(&s), 32);
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 0);
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "");
> +}
> +
> +static void seq_buf_declare_test(struct kunit *test)
> +{
> + DECLARE_SEQ_BUF(s, 24);
> +
> + KUNIT_EXPECT_EQ(test, s.size, 24);
> + KUNIT_EXPECT_EQ(test, s.len, 0);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_EQ(test, seq_buf_buffer_left(&s), 24);
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 0);
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "");
> +}
> +
> +static void seq_buf_clear_test(struct kunit *test)
> +{
> + DECLARE_SEQ_BUF(s, 128);
> +
> + seq_buf_puts(&s, "hello");
> + KUNIT_EXPECT_EQ(test, s.len, 5);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
My gut feeling seeing this was that we should test that
seq_buf_clear() also clears the overflow flag, though the separate
overflow tests below all do seem to test that, so it's probably
redundant.
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello");
> +
> + seq_buf_clear(&s);
> +
> + KUNIT_EXPECT_EQ(test, s.len, 0);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "");
> +}
> +
> +static void seq_buf_puts_test(struct kunit *test)
> +{
> + DECLARE_SEQ_BUF(s, 16);
> +
> + seq_buf_puts(&s, "hello");
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 5);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello");
> +
> + seq_buf_puts(&s, " world");
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 11);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world");
> +}
> +
> +static void seq_buf_puts_overflow_test(struct kunit *test)
> +{
> + DECLARE_SEQ_BUF(s, 10);
> +
> + seq_buf_puts(&s, "123456789");
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 9);
> +
> + seq_buf_puts(&s, "0");
> + KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 10);
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "123456789");
> +
> + seq_buf_clear(&s);
> + KUNIT_EXPECT_EQ(test, s.len, 0);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "");
> +}
> +
> +static void seq_buf_putc_test(struct kunit *test)
> +{
> + DECLARE_SEQ_BUF(s, 4);
> +
> + seq_buf_putc(&s, 'a');
> + seq_buf_putc(&s, 'b');
> + seq_buf_putc(&s, 'c');
> +
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 3);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "abc");
> +
> + seq_buf_putc(&s, 'd');
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 4);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "abc");
> +
> + seq_buf_putc(&s, 'e');
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 4);
> + KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "abc");
> +
> + seq_buf_clear(&s);
> + KUNIT_EXPECT_EQ(test, s.len, 0);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "");
> +}
> +
> +static void seq_buf_printf_test(struct kunit *test)
> +{
> + DECLARE_SEQ_BUF(s, 32);
> +
> + seq_buf_printf(&s, "hello %s", "world");
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 11);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world");
> +
> + seq_buf_printf(&s, " %d", 123);
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 15);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world 123");
> +}
> +
> +static void seq_buf_printf_overflow_test(struct kunit *test)
> +{
> + DECLARE_SEQ_BUF(s, 16);
> +
> + seq_buf_printf(&s, "%lu", 1234567890UL);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 10);
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "1234567890");
> +
> + seq_buf_printf(&s, "%s", "abcdefghij");
> + KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 16);
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "1234567890abcde");
> +
> + seq_buf_clear(&s);
> + KUNIT_EXPECT_EQ(test, s.len, 0);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "");
> +}
> +
> +static void seq_buf_get_buf_commit_test(struct kunit *test)
> +{
> + DECLARE_SEQ_BUF(s, 16);
> + char *buf;
> + size_t len;
> +
> + len = seq_buf_get_buf(&s, &buf);
> + KUNIT_EXPECT_EQ(test, len, 16);
> + KUNIT_EXPECT_PTR_NE(test, buf, NULL);
> +
> + memcpy(buf, "hello", 5);
> + seq_buf_commit(&s, 5);
> +
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 5);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello");
> +
> + len = seq_buf_get_buf(&s, &buf);
> + KUNIT_EXPECT_EQ(test, len, 11);
> + KUNIT_EXPECT_PTR_NE(test, buf, NULL);
> +
> + memcpy(buf, " worlds!", 8);
> + seq_buf_commit(&s, 6);
> +
> + KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 11);
> + KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
> + KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world");
> +
> + len = seq_buf_get_buf(&s, &buf);
> + KUNIT_EXPECT_EQ(test, len, 5);
> + KUNIT_EXPECT_PTR_NE(test, buf, NULL);
> +
> + seq_buf_commit(&s, -1);
> + KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
> +}
> +
> +static struct kunit_case seq_buf_test_cases[] = {
> + KUNIT_CASE(seq_buf_init_test),
> + KUNIT_CASE(seq_buf_declare_test),
> + KUNIT_CASE(seq_buf_clear_test),
> + KUNIT_CASE(seq_buf_puts_test),
> + KUNIT_CASE(seq_buf_puts_overflow_test),
> + KUNIT_CASE(seq_buf_putc_test),
> + KUNIT_CASE(seq_buf_printf_test),
> + KUNIT_CASE(seq_buf_printf_overflow_test),
> + KUNIT_CASE(seq_buf_get_buf_commit_test),
> + {}
> +};
> +
> +static struct kunit_suite seq_buf_test_suite = {
> + .name = "seq_buf",
> + .test_cases = seq_buf_test_cases,
> +};
> +
> +kunit_test_suite(seq_buf_test_suite);
> --
> 2.34.1
>
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5281 bytes)
Powered by blists - more mailing lists