[PATCH master 1/2] range: fix corner cases when exclusive end is zero

[PATCH master 1/2] range: fix corner cases when exclusive end is zero

[Ahmad Fatoum]

Unlike inclusive ranges, exclusive ranges can be empty and
region_overlap_end_exclusive() goes beyond that and checks that ranges
are neither empty nor does the end precede the start.

This is problematic for a range that stretches to the maximum value of a
type. Exclusive end in that case would be 0 and 0 comes before all
non-zero numbers, so the region_overlap_end_exclusive() would always
return false.

Fix this by normalizing end to be inclusive first thing in the function.
And for extra safety, enforce that the function may only be called with
arguments that are all of the same unsigned type.

This resolves a MMU hang on an STM32MP1 board with 1G RAM that had
memory stretching from 0xc00000000 to 0xffffffff inclusive as
remap_range_end_sans_text() would not detect the text area overlap and
then all of RAM, including where barebox is running from would be mapped
non-executable.

Fixes: 768fdb36f30e ("partition: define new region_overlap_end_exclusive helper")
Reported-by: Sohaib Mohamed <sohaib.amhmd@gmail.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 include/range.h | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/include/range.h b/include/range.h
index bb41dc78ac43..eb75486d9add 100644
--- a/include/range.h
+++ b/include/range.h
@@ -3,6 +3,8 @@
 #define _RANGE_H__
 
 #include <linux/types.h>
+#include <linux/compiler.h>
+#include <linux/build_bug.h>
 
 /**
  * region_overlap_end_inclusive - check whether a pair of [start, end] ranges overlap
@@ -29,17 +31,26 @@ static inline bool region_overlap_end_inclusive(u64 starta, u64 enda,
  * @enda:   end of the first range (exclusive)
  * @startb: start of the second range
  * @endb:   end of the second range (exclusive)
+ *
+ * NOTE: end of zero is always interpreted to mean including the maximum
+ * value of the type.
  */
-static inline bool region_overlap_end_exclusive(u64 starta, u64 enda,
-						u64 startb, u64 endb)
-{
-	/* Empty ranges don't overlap */
-	if (starta >= enda || startb >= endb)
-		return false;
-
-	return region_overlap_end_inclusive(starta, enda - 1,
-					    startb, endb - 1);
-}
+#define region_overlap_end_exclusive(starta, enda, startb, endb)	\
+({									\
+	u64 __starta = (starta), __enda = (enda) - 1;			\
+	u64 __startb = (startb), __endb = (endb) - 1;			\
+									\
+	static_assert(__same_type(starta, enda));			\
+	static_assert(__same_type(enda, startb));			\
+	static_assert(__same_type(startb, endb));			\
+	static_assert(((typeof(endb))-1) > 0);				\
+									\
+	/* Empty ranges don't overlap */				\
+	(__starta <= __enda && __startb <= __endb)			\
+		 ? region_overlap_end_inclusive(__starta, __enda,	\
+						__startb, __endb)	\
+		 : false;						\
+})
 
 
 /**
-- 
2.47.3

[PATCH master 2/2] test: self: add range.h test

[Ahmad Fatoum]

Due to the subtle bug in range.h breaking the MMU code, it's probably a
good idea to have some tests that exercise the corner cases of the
range.h utility functions.

Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 test/self/Kconfig  |   6 +++
 test/self/Makefile |   1 +
 test/self/range.c  | 105 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+)
 create mode 100644 test/self/range.c

diff --git a/test/self/Kconfig b/test/self/Kconfig
index fa2911fa649a..f46253afc034 100644
--- a/test/self/Kconfig
+++ b/test/self/Kconfig
@@ -28,6 +28,7 @@ config SELFTEST_AUTORUN
 
 config SELFTEST_ENABLE_ALL
 	bool "Enable all self-tests"
+	select SELFTEST_RANGE
 	select SELFTEST_PRINTF
 	select SELFTEST_MALLOC
 	select SELFTEST_PROGRESS_NOTIFIER
@@ -50,6 +51,11 @@ config SELFTEST_ENABLE_ALL
 	help
 	  Selects all self-tests compatible with current configuration
 
+config SELFTEST_RANGE
+	bool "range.h selftest"
+	help
+	  Tests range functions
+
 config SELFTEST_MALLOC
 	bool "malloc() selftest"
 	help
diff --git a/test/self/Makefile b/test/self/Makefile
index 3d74bf9e98ad..124607b375d8 100644
--- a/test/self/Makefile
+++ b/test/self/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_SELFTEST) += core.o
+obj-$(CONFIG_SELFTEST_RANGE) += range.o
 obj-$(CONFIG_SELFTEST_MALLOC) += malloc.o
 obj-$(CONFIG_SELFTEST_PRINTF) += printf.o
 CFLAGS_printf.o += -Wno-format-security -Wno-format
diff --git a/test/self/range.c b/test/self/range.c
new file mode 100644
index 000000000000..82b19239eac8
--- /dev/null
+++ b/test/self/range.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) "range: " fmt
+
+#include <common.h>
+#include <bselftest.h>
+#include <linux/types.h>
+#include <linux/sizes.h>
+#include <stdio.h>
+#include <range.h>
+
+BSELFTEST_GLOBALS();
+
+static void __expect_bool(const char *msg, bool got, bool want)
+{
+	total_tests++;
+
+	if (got != want) {
+		failed_tests++;
+		pr_err("FAIL: %s — got %d want %d\n", msg, got, want);
+	}
+}
+
+#define expect_bool(got, want, type, a, b, c, d) do {		\
+	char _buf[256];						\
+	snprintf(_buf, sizeof(_buf), "%d: " type		\
+		 " [%#llx, %#llx] vs [%#llx, %#llx]", __LINE__,	\
+		 (u64)(a), (u64)(b), (u64)(c), (u64)(d));	\
+	__expect_bool(_buf, got, want);				\
+} while (0)
+
+#define TEST_INCL(a, b, c, d, expect)				\
+	expect_bool(region_overlap_end_inclusive(a, b, c, d),	\
+		    expect, "incl", a, b, c, d);
+
+#define TEST_EXCL(a, b, c, d, expect)				\
+	expect_bool(region_overlap_end_exclusive(a, b, c, d),	\
+		    expect, "excl", a, b, c, d);
+
+#define TEST_SIZE(a, lena, b, lenb, expect) \
+	expect_bool(region_overlap_size(a, lena, b, lenb),	\
+		    expect, "size", a, lena, b, lenb)
+
+static void test_region_overlap(void)
+{
+	TEST_INCL(0u, 5u, 5u, 10u, true);
+	TEST_INCL(0u, 4u, 5u, 10u, false);
+	TEST_INCL(10u, 20u, 12u, 15u, true);
+	TEST_INCL(20u, 10u, 12u, 15u, false);
+
+	TEST_EXCL(0ull, 5ull, 5ull, 10ull, false);
+	TEST_EXCL(0u, 6u, 5u, 10u, true);
+	TEST_EXCL(5u, 5u, 5u, 10u, false);
+	TEST_EXCL(5u, 10u, 7u, 7u, false);
+	TEST_EXCL(3u, 8u, 3u, 8u, true);
+	TEST_EXCL(0u, 0u, 0u, 1u, true);
+	TEST_EXCL(0xFFFFFF00, 0xFFFFFF10,
+		  0xFFFFF000, 0xFFFFFFF0, true);
+	TEST_EXCL(0u, 1u, 1u, 1u, false);
+
+	TEST_SIZE(0u, 0u, 0u, 10u, false);
+	TEST_SIZE(0u, 10u, 5u, 0u, false);
+	TEST_SIZE(0u, 5u, 5u, 5u, false);
+	TEST_SIZE(0u, 6u, 5u, 5u, true);
+	TEST_SIZE(0u, U64_MAX, 123u, 1u, true);
+
+	TEST_EXCL(0u, 0u, 0u, 0u, true);
+	TEST_EXCL(0ull, 0ull, 0ull, 0ull, true);
+
+
+	TEST_EXCL(0xF0000000u, 0u,
+		  (u32)(SZ_4G + SZ_1G), (u32)(SZ_4G + SZ_2G), false);
+	TEST_EXCL(0xF0000000ul, 0ul,
+		  (ulong)(SZ_4G + SZ_1G), (ulong)(SZ_4G + SZ_2G),
+		  sizeof(ulong) == sizeof(u64));
+	TEST_EXCL(0xF0000000ull, 0ull,
+		  SZ_4G + SZ_1G, SZ_4G + SZ_2G, true);
+
+	TEST_EXCL(1u, 1u, 1u, 1u, false);
+
+	TEST_EXCL(0xFFFFFFFEu, 0u, 0u, 1u, false);
+
+	TEST_EXCL(U32_MAX / 2, U32_MAX + 1,
+		  U32_MAX / 2 + 0x10, U32_MAX / 2 + 0x100, true);
+
+	TEST_SIZE(1u, U64_MAX, 1u, 1u, true);
+	TEST_SIZE(1u, U64_MAX, 10u, 10u, true);
+
+	TEST_SIZE(0xFFFFFFFFFFFFFFE0ull, 0x20ull,
+		  0xFFFFFFFFFFFFFFFFull, 0x0ull, false);
+
+	TEST_SIZE(0xFFFFFFFFFFFFFFE0ull, 0x20ull,
+		  0xFFFFFFFFFFFFFFFFull, 0x0ull, false);
+
+	TEST_SIZE(0xFFFFFFFFFFFFFFE0ull, 0x20ull,
+		  0xFFFFFFFFFFFFFFFCull, 0x1ull, true);
+
+	TEST_SIZE(0xFFFFFFFFFFFFFFE0ull, 0x20ull,
+		  0xFFFFFFFFFFFFFFFEull, 0x1ull, true);
+
+	TEST_SIZE(0xFFFFFFFFFFFFFFE0ull, 0x20ull,
+		  0ull, 10ull, false);
+}
+
+bselftest(core, test_region_overlap);
-- 
2.47.3

[PATCH] fixup! range: fix corner cases when exclusive end is zero

[Ahmad Fatoum]

While there is no risk of miscomputation here, let's avoid possible
compile errors by putting parenthesis around macro arguments as usual.

Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 include/range.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/range.h b/include/range.h
index eb75486d9add..b5dc5cdb4d39 100644
--- a/include/range.h
+++ b/include/range.h
@@ -40,9 +40,9 @@ static inline bool region_overlap_end_inclusive(u64 starta, u64 enda,
 	u64 __starta = (starta), __enda = (enda) - 1;			\
 	u64 __startb = (startb), __endb = (endb) - 1;			\
 									\
-	static_assert(__same_type(starta, enda));			\
-	static_assert(__same_type(enda, startb));			\
-	static_assert(__same_type(startb, endb));			\
+	static_assert(__same_type((starta), (enda)));			\
+	static_assert(__same_type((enda), (startb)));			\
+	static_assert(__same_type((startb), (endb)));			\
 	static_assert(((typeof(endb))-1) > 0);				\
 									\
 	/* Empty ranges don't overlap */				\
-- 
2.47.3