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>] [day] [month] [year] [list]
Message-Id: <20250304173835.work.970-kees@kernel.org>
Date: Tue,  4 Mar 2025 09:38:40 -0800
From: Kees Cook <kees@...nel.org>
To: Nathan Chancellor <nathan@...nel.org>
Cc: Kees Cook <kees@...nel.org>,
	Thomas Weißschuh <linux@...ssschuh.net>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>,
	linux-hardening@...r.kernel.org,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>,
	Christian Schrefl <chrisi.schrefl@...il.com>,
	linux-kernel@...r.kernel.org,
	llvm@...ts.linux.dev
Subject: [PATCH] kunit/overflow: Fix DEFINE_FLEX tests for counted_by

Unfortunately, __builtin_dynamic_object_size() does not take into account
flexible array sizes, even when they are sized by __counted_by. As a
result, the size tests for the flexible arrays need to be separated to
get an accurate check of the compiler's behavior. While at it, fully test
sizeof, __struct_size (bdos(..., 0)), and __member_size (bdos(..., 1)).

I still think this is a compiler design issue, but there's not much to
be done about it currently beyond adjusting these tests. GCC and Clang
agree on this behavior at least. :)

Reported-by: "Thomas Weißschuh" <linux@...ssschuh.net>
Closes: https://lore.kernel.org/lkml/e1a1531d-6968-4ae8-a3b5-5ea0547ec4b3@t-8ch.de/
Fixes: 9dd5134c6158 ("kunit/overflow: Adjust for __counted_by with DEFINE_RAW_FLEX()")
Signed-off-by: Kees Cook <kees@...nel.org>
---
Cc: Nathan Chancellor <nathan@...nel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc: linux-hardening@...r.kernel.org
---
 lib/overflow_kunit.c | 38 ++++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c
index 5222c6393f11..894691b4411a 100644
--- a/lib/overflow_kunit.c
+++ b/lib/overflow_kunit.c
@@ -1185,22 +1185,40 @@ struct bar {
 
 static void DEFINE_FLEX_test(struct kunit *test)
 {
-	/* Using _RAW_ on a __counted_by struct will initialize "counter" to zero */
-	DEFINE_RAW_FLEX(struct foo, two_but_zero, array, 2);
-#ifdef CONFIG_CC_HAS_COUNTED_BY
-	int expected_raw_size = sizeof(struct foo);
-#else
-	int expected_raw_size = sizeof(struct foo) + 2 * sizeof(s16);
-#endif
-	/* Without annotation, it will always be on-stack size. */
 	DEFINE_RAW_FLEX(struct bar, two, array, 2);
 	DEFINE_FLEX(struct foo, eight, array, counter, 8);
 	DEFINE_FLEX(struct foo, empty, array, counter, 0);
+	/* Using _RAW_ on a __counted_by struct will initialize "counter" to zero */
+	DEFINE_RAW_FLEX(struct foo, two_but_zero, array, 2);
+	int array_size_override = 0;
 
-	KUNIT_EXPECT_EQ(test, __struct_size(two_but_zero), expected_raw_size);
+	KUNIT_EXPECT_EQ(test, sizeof(*two), sizeof(struct bar));
 	KUNIT_EXPECT_EQ(test, __struct_size(two), sizeof(struct bar) + 2 * sizeof(s16));
-	KUNIT_EXPECT_EQ(test, __struct_size(eight), 24);
+	KUNIT_EXPECT_EQ(test, __member_size(two), sizeof(struct bar) + 2 * sizeof(s16));
+	KUNIT_EXPECT_EQ(test, __struct_size(two->array), 2 * sizeof(s16));
+	KUNIT_EXPECT_EQ(test, __member_size(two->array), 2 * sizeof(s16));
+
+	KUNIT_EXPECT_EQ(test, sizeof(*eight), sizeof(struct foo));
+	KUNIT_EXPECT_EQ(test, __struct_size(eight), sizeof(struct foo) + 8 * sizeof(s16));
+	KUNIT_EXPECT_EQ(test, __member_size(eight), sizeof(struct foo) + 8 * sizeof(s16));
+	KUNIT_EXPECT_EQ(test, __struct_size(eight->array), 8 * sizeof(s16));
+	KUNIT_EXPECT_EQ(test, __member_size(eight->array), 8 * sizeof(s16));
+
+	KUNIT_EXPECT_EQ(test, sizeof(*empty), sizeof(struct foo));
 	KUNIT_EXPECT_EQ(test, __struct_size(empty), sizeof(struct foo));
+	KUNIT_EXPECT_EQ(test, __member_size(empty), sizeof(struct foo));
+	KUNIT_EXPECT_EQ(test, __struct_size(empty->array), 0);
+	KUNIT_EXPECT_EQ(test, __member_size(empty->array), 0);
+
+	/* If __counted_by is not being used, array size will have the on-stack size. */
+	if (!IS_ENABLED(CONFIG_CC_HAS_COUNTED_BY))
+		array_size_override = 2 * sizeof(s16);
+
+	KUNIT_EXPECT_EQ(test, sizeof(*two_but_zero), sizeof(struct foo));
+	KUNIT_EXPECT_EQ(test, __struct_size(two_but_zero), sizeof(struct foo) + 2 * sizeof(s16));
+	KUNIT_EXPECT_EQ(test, __member_size(two_but_zero), sizeof(struct foo) + 2 * sizeof(s16));
+	KUNIT_EXPECT_EQ(test, __struct_size(two_but_zero->array), array_size_override);
+	KUNIT_EXPECT_EQ(test, __member_size(two_but_zero->array), array_size_override);
 }
 
 static struct kunit_case overflow_test_cases[] = {
-- 
2.34.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ