linux-hardening - [PATCH 3/3] printk: kunit: Fix __counted_by() in struct prbtest

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250702095157.110916-4-pmladek@suse.com>
Date: Wed,  2 Jul 2025 11:51:57 +0200
From: Petr Mladek <pmladek@...e.com>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>,
	John Ogness <john.ogness@...utronix.de>,
	Dan Carpenter <dan.carpenter@...aro.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Kees Cook <kees@...nel.org>,
	"Gustavo A . R . Silva" <gustavoars@...nel.org>,
	David Gow <davidgow@...gle.com>,
	Arnd Bergmann <arnd@...nel.org>,
	Arnd Bergmann <arnd@...db.de>,
	linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org,
	Petr Mladek <pmladek@...e.com>
Subject: [PATCH 3/3] printk: kunit: Fix __counted_by() in struct prbtest_rbdata

__counted_by() has to point to a variable which defines the size
of the related array. The code must never access the array
beyond this limit.

struct prbtest_rbdata currently stores the length of the string.
And the code access the array beyond the limit when writing
or reading the trailing '\0'.

Store the size of the string, including the trailing '\0' if
we wanted to keep __counted_by().

Consistently use "_size" suffix when the trailing '\0' is counted.
Note that MAX_RBDATA_TEXT_SIZE was originally used to limit
the text length.

When touching the code, make sure that @text_size produced by
get_random_u32_inclusive() stays within the limits.

Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
Closes: https://lore.kernel.org/r/eaea66b9-266a-46e7-980d-33f40ad4b215@sabinyo.mountain
Suggested-by: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Signed-off-by: Petr Mladek <pmladek@...e.com>
---
 kernel/printk/printk_ringbuffer_kunit_test.c | 47 +++++++++++---------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
index 0c3030fde8c2..088fe4d8c9b6 100644
--- a/kernel/printk/printk_ringbuffer_kunit_test.c
+++ b/kernel/printk/printk_ringbuffer_kunit_test.c
@@ -52,13 +52,12 @@ module_param(runtime_ms, ulong, 0400);
 
 /* test data structure */
 struct prbtest_rbdata {
-	unsigned int len;
-	char text[] __counted_by(len);
+	unsigned int size;
+	char text[] __counted_by(size);
 };
 
-#define MAX_RBDATA_TEXT_SIZE 0x7f
-/* +1 for terminator. */
-#define MAX_PRB_RECORD_SIZE (sizeof(struct prbtest_rbdata) + MAX_RBDATA_TEXT_SIZE + 1)
+#define MAX_RBDATA_TEXT_SIZE 0x80
+#define MAX_PRB_RECORD_SIZE (sizeof(struct prbtest_rbdata) + MAX_RBDATA_TEXT_SIZE)
 
 struct prbtest_data {
 	struct kunit *test;
@@ -74,25 +73,29 @@ struct prbtest_thread_data {
 
 static void prbtest_fail_record(struct kunit *test, const struct prbtest_rbdata *dat, u64 seq)
 {
-	KUNIT_FAIL(test, "BAD RECORD: seq=%llu len=%u text=%.*s\n",
-		   seq, dat->len,
-		   dat->len <= MAX_RBDATA_TEXT_SIZE ? dat->len : -1,
-		   dat->len <= MAX_RBDATA_TEXT_SIZE ? dat->text : "<invalid>");
+	unsigned int len;
+
+	len = dat->size - 1;
+
+	KUNIT_FAIL(test, "BAD RECORD: seq=%llu size=%u text=%.*s\n",
+		   seq, dat->size,
+		   len < MAX_RBDATA_TEXT_SIZE ? len : -1,
+		   len < MAX_RBDATA_TEXT_SIZE ? dat->text : "<invalid>");
 }
 
 static bool prbtest_check_data(const struct prbtest_rbdata *dat)
 {
 	unsigned int len;
 
-	/* Sane length? */
-	if (dat->len < 1 || dat->len > MAX_RBDATA_TEXT_SIZE)
+	/* Sane size? At least one character + trailing '\0' */
+	if (dat->size < 2 || dat->size > MAX_RBDATA_TEXT_SIZE)
 		return false;
 
-	if (dat->text[dat->len] != '\0')
+	len = dat->size - 1;
+	if (dat->text[len] != '\0')
 		return false;
 
 	/* String repeats with the same character? */
-	len = dat->len;
 	while (len--) {
 		if (dat->text[len] != dat->text[0])
 			return false;
@@ -114,10 +117,14 @@ static int prbtest_writer(void *data)
 	kunit_info(tr->test_data->test, "start thread %03lu (writer)\n", tr->num);
 
 	for (;;) {
-		/* ensure at least 1 character */
-		text_size = get_random_u32_inclusive(1, MAX_RBDATA_TEXT_SIZE);
-		/* +1 for terminator. */
-		record_size = sizeof(struct prbtest_rbdata) + text_size + 1;
+		/* ensure at least 1 character + trailing '\0' */
+		text_size = get_random_u32_inclusive(2, MAX_RBDATA_TEXT_SIZE);
+		if (WARN_ON_ONCE(text_size < 2))
+			text_size = 2;
+		if (WARN_ON_ONCE(text_size > MAX_RBDATA_TEXT_SIZE))
+			text_size = MAX_RBDATA_TEXT_SIZE;
+
+		record_size = sizeof(struct prbtest_rbdata) + text_size;
 		WARN_ON_ONCE(record_size > MAX_PRB_RECORD_SIZE);
 
 		/* specify the text sizes for reservation */
@@ -140,9 +147,9 @@ static int prbtest_writer(void *data)
 			r.info->text_len = record_size;
 
 			dat = (struct prbtest_rbdata *)r.text_buf;
-			dat->len = text_size;
-			memset(dat->text, text_id, text_size);
-			dat->text[text_size] = 0;
+			dat->size = text_size;
+			memset(dat->text, text_id, text_size - 1);
+			dat->text[text_size - 1] = '\0';
 
 			prb_commit(&e);
 
-- 
2.50.0