[PATCH 0/2] Add __counted_by_ptr attribute to struct uid_gid_map

[PATCH 0/2] Add __counted_by_ptr attribute to struct uid_gid_map

[Bill Wendling]

Bill Wendling (2):
  userns: Add __counted_by_ptr attribute to struct uid_gid_map
  userns: Add KUnit test suite for uid_gid_map

 include/linux/user_namespace.h |  4 +-
 init/Kconfig                   | 10 ++++
 kernel/.kunitconfig            |  3 ++
 kernel/user_namespace.c        | 12 +++--
 kernel/user_namespace_kunit.c  | 87 ++++++++++++++++++++++++++++++++++
 5 files changed, 110 insertions(+), 6 deletions(-)
 create mode 100644 kernel/.kunitconfig
 create mode 100644 kernel/user_namespace_kunit.c

-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map

[Bill Wendling]

The compiler attribute __counted_by_ptr associates a pointer field of a
struct with a sibling field within the same struct that specifies the
element count of the allocated memory. This enables KASAN and fortified
bounds-checking to detect out-of-bounds accesses to the pointer field at
runtime.

We can add the __counted_by_ptr attribute to the 'forward' and 'reverse'
pointer fields of 'struct uid_gid_map', which are counted by
'nr_extents'. Since 'nr_extents' is defined in a sibling anonymous
struct inside an anonymous union, the nearest common non-anonymous
struct level is 'struct uid_gid_map' itself, which is supported by the
compiler.

However, doing so has runtime implications. In the original
implementation of insert_extent(), elements are written to
map->forward[map->nr_extents] before map->nr_extents is incremented:

	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
		dest = &map->extent[map->nr_extents];
	else
		dest = &map->forward[map->nr_extents];

	*dest = *extent;
	map->nr_extents++;

At the time of writing to 'map->forward[map->nr_extents]',
map->nr_extents is still 5, but we are accessing index 5 (which is the
6th element). Under __counted_by_ptr(nr_extents), the compiler and
KASAN expect the accessed index to be strictly less than
map->nr_extents. Therefore, accessing index 5 when the count is 5
triggers an out-of-bounds panic/trap at runtime.

To resolve this, insert_extent() is refactored to increment
map->nr_extents first, and then use map->nr_extents - 1 as the index:

	map->nr_extents++;
	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
		dest = &map->extent[map->nr_extents - 1];
	else
		dest = &map->forward[map->nr_extents - 1];

	*dest = *extent;

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: Christian Brauner <brauner@kernel.org>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Nicolas Schier <nsc@kernel.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Anand Moon <linux.amoon@gmail.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: codemender-patching+linux@google.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
---
 include/linux/user_namespace.h | 4 ++--
 kernel/user_namespace.c        | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index e38d9e60569f..2962256eddf7 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -29,8 +29,8 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
 			u32 nr_extents;
 		};
 		struct {
-			struct uid_gid_extent *forward;
-			struct uid_gid_extent *reverse;
+			struct uid_gid_extent *forward __counted_by_ptr(nr_extents);
+			struct uid_gid_extent *reverse __counted_by_ptr(nr_extents);
 		};
 	};
 };
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 0bed462e9b2a..7e5371d8f515 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -809,13 +809,13 @@ static int insert_extent(struct uid_gid_map *map, struct uid_gid_extent *extent)
 		map->reverse = NULL;
 	}
 
-	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
-		dest = &map->extent[map->nr_extents];
+	map->nr_extents++;
+	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
+		dest = &map->extent[map->nr_extents - 1];
 	else
-		dest = &map->forward[map->nr_extents];
+		dest = &map->forward[map->nr_extents - 1];
 
 	*dest = *extent;
-	map->nr_extents++;
 	return 0;
 }
 
-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

Add a KUnit test suite to verify the insertion and sorting of mappings
in struct uid_gid_map. This test suite validates both base extent
insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
which triggers the allocation of the forward and reverse pointers).

This is especially useful for verifying that the __counted_by_ptr
attribute added to 'forward' and 'reverse' pointers works correctly
without causing any runtime bounds-checking panics or traps.

Assisted-by: Gemini Next
Change-Id: If0c2c197a35cd7429cf0d2d6e3b33f0d9f0be66c
Signed-off-by: Bill Wendling <morbo@google.com>
---
Cc: Kees Cook <kees@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Nicolas Schier <nsc@kernel.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Anand Moon <linux.amoon@gmail.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: codemender-patching+linux@google.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
---
 init/Kconfig                  | 10 ++++
 kernel/.kunitconfig           |  3 ++
 kernel/user_namespace.c       |  4 ++
 kernel/user_namespace_kunit.c | 87 +++++++++++++++++++++++++++++++++++
 4 files changed, 104 insertions(+)
 create mode 100644 kernel/.kunitconfig
 create mode 100644 kernel/user_namespace_kunit.c

diff --git a/init/Kconfig b/init/Kconfig
index f63bf5e05e79..ba6a40b7315a 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1457,6 +1457,16 @@ config USER_NS
 
 	  If unsure, say N.
 
+config USER_NAMESPACE_KUNIT_TEST
+	tristate "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
+	depends on USER_NS && KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the KUnit test for user namespace uid/gid map insertion.
+	  It validates map insertion, limits, dynamic allocation of the
+	  extended extents array, and mapping sorting functions.
+	  If unsure, say N.
+
 config PID_NS
 	bool "PID Namespaces"
 	default y
diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
new file mode 100644
index 000000000000..7314dce05dc2
--- /dev/null
+++ b/kernel/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_USER_NS=y
+CONFIG_USER_NAMESPACE_KUNIT_TEST=y
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 7e5371d8f515..64c64e1028e8 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -1413,3 +1413,7 @@ static __init int user_namespaces_init(void)
 	return 0;
 }
 subsys_initcall(user_namespaces_init);
+
+#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
+#include "user_namespace_kunit.c"
+#endif
diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
new file mode 100644
index 000000000000..6d7662ef1916
--- /dev/null
+++ b/kernel/user_namespace_kunit.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for user namespace map insertion and sorting.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+
+static void test_user_ns_map_insert_base(struct kunit *test)
+{
+	struct uid_gid_map map;
+	struct uid_gid_extent extent;
+	int i, ret;
+
+	memset(&map, 0, sizeof(map));
+
+	/* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
+	for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
+		extent.first = i * 10;
+		extent.lower_first = i * 100;
+		extent.count = 5;
+
+		ret = insert_extent(&map, &extent);
+		KUNIT_EXPECT_EQ(test, ret, 0);
+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
+		KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
+		KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
+		KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
+	}
+}
+
+static void test_user_ns_map_insert_extended(struct kunit *test)
+{
+	struct uid_gid_map map;
+	struct uid_gid_extent extent;
+	int i, ret;
+
+	memset(&map, 0, sizeof(map));
+
+	/* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
+	for (i = 0; i < 10; i++) {
+		extent.first = i * 10;
+		extent.lower_first = i * 100;
+		extent.count = 5;
+
+		ret = insert_extent(&map, &extent);
+		KUNIT_EXPECT_EQ(test, ret, 0);
+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
+
+		if (i < UID_GID_MAP_MAX_BASE_EXTENTS) {
+			KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
+		} else {
+			KUNIT_EXPECT_NOT_ERR_OR_NULL(test, map.forward);
+			KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
+			KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
+			KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
+		}
+	}
+
+	/* Now sort the map to set up reverse mapping */
+	ret = sort_idmaps(&map);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, map.reverse);
+
+	/* Verify sorting is correct */
+	for (i = 0; i < map.nr_extents; i++) {
+		KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
+		KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
+	}
+
+	/* Clean up allocations to avoid leaks */
+	kfree(map.forward);
+	kfree(map.reverse);
+}
+
+static struct kunit_case user_ns_map_test_cases[] = {
+	KUNIT_CASE(test_user_ns_map_insert_base),
+	KUNIT_CASE(test_user_ns_map_insert_extended),
+	{}
+};
+
+static struct kunit_suite user_ns_map_test_suite = {
+	.name = "user_ns_map",
+	.test_cases = user_ns_map_test_cases,
+};
+
+kunit_test_suite(user_ns_map_test_suite);
-- 
2.55.0.860.g4b6b3295ed-goog

[PATCH 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

Add a KUnit test suite to verify the insertion and sorting of mappings
in struct uid_gid_map. This test suite validates both base extent
insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
which triggers the allocation of the forward and reverse pointers).

This is especially useful for verifying that the __counted_by_ptr
attribute added to 'forward' and 'reverse' pointers works correctly
without causing any runtime bounds-checking panics or traps.

Assisted-by: Gemini Next
Signed-off-by: Bill Wendling <morbo@google.com>
---
v2 - Remove Gerrit tag.
---
Cc: Kees Cook <kees@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Nicolas Schier <nsc@kernel.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Anand Moon <linux.amoon@gmail.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: codemender-patching+linux@google.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
---
 init/Kconfig                  | 10 ++++
 kernel/.kunitconfig           |  3 ++
 kernel/user_namespace.c       |  4 ++
 kernel/user_namespace_kunit.c | 87 +++++++++++++++++++++++++++++++++++
 4 files changed, 104 insertions(+)
 create mode 100644 kernel/.kunitconfig
 create mode 100644 kernel/user_namespace_kunit.c

diff --git a/init/Kconfig b/init/Kconfig
index f63bf5e05e79..ba6a40b7315a 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1457,6 +1457,16 @@ config USER_NS
 
 	  If unsure, say N.
 
+config USER_NAMESPACE_KUNIT_TEST
+	tristate "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
+	depends on USER_NS && KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the KUnit test for user namespace uid/gid map insertion.
+	  It validates map insertion, limits, dynamic allocation of the
+	  extended extents array, and mapping sorting functions.
+	  If unsure, say N.
+
 config PID_NS
 	bool "PID Namespaces"
 	default y
diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
new file mode 100644
index 000000000000..7314dce05dc2
--- /dev/null
+++ b/kernel/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_USER_NS=y
+CONFIG_USER_NAMESPACE_KUNIT_TEST=y
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 7e5371d8f515..64c64e1028e8 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -1413,3 +1413,7 @@ static __init int user_namespaces_init(void)
 	return 0;
 }
 subsys_initcall(user_namespaces_init);
+
+#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
+#include "user_namespace_kunit.c"
+#endif
diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
new file mode 100644
index 000000000000..6d7662ef1916
--- /dev/null
+++ b/kernel/user_namespace_kunit.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for user namespace map insertion and sorting.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+
+static void test_user_ns_map_insert_base(struct kunit *test)
+{
+	struct uid_gid_map map;
+	struct uid_gid_extent extent;
+	int i, ret;
+
+	memset(&map, 0, sizeof(map));
+
+	/* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
+	for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
+		extent.first = i * 10;
+		extent.lower_first = i * 100;
+		extent.count = 5;
+
+		ret = insert_extent(&map, &extent);
+		KUNIT_EXPECT_EQ(test, ret, 0);
+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
+		KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
+		KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
+		KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
+	}
+}
+
+static void test_user_ns_map_insert_extended(struct kunit *test)
+{
+	struct uid_gid_map map;
+	struct uid_gid_extent extent;
+	int i, ret;
+
+	memset(&map, 0, sizeof(map));
+
+	/* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
+	for (i = 0; i < 10; i++) {
+		extent.first = i * 10;
+		extent.lower_first = i * 100;
+		extent.count = 5;
+
+		ret = insert_extent(&map, &extent);
+		KUNIT_EXPECT_EQ(test, ret, 0);
+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
+
+		if (i < UID_GID_MAP_MAX_BASE_EXTENTS) {
+			KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
+		} else {
+			KUNIT_EXPECT_NOT_ERR_OR_NULL(test, map.forward);
+			KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
+			KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
+			KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
+		}
+	}
+
+	/* Now sort the map to set up reverse mapping */
+	ret = sort_idmaps(&map);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, map.reverse);
+
+	/* Verify sorting is correct */
+	for (i = 0; i < map.nr_extents; i++) {
+		KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
+		KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
+	}
+
+	/* Clean up allocations to avoid leaks */
+	kfree(map.forward);
+	kfree(map.reverse);
+}
+
+static struct kunit_case user_ns_map_test_cases[] = {
+	KUNIT_CASE(test_user_ns_map_insert_base),
+	KUNIT_CASE(test_user_ns_map_insert_extended),
+	{}
+};
+
+static struct kunit_suite user_ns_map_test_suite = {
+	.name = "user_ns_map",
+	.test_cases = user_ns_map_test_cases,
+};
+
+kunit_test_suite(user_ns_map_test_suite);
-- 
2.55.0.860.g4b6b3295ed-goog

Re: [PATCH 2/2] userns: Add KUnit test suite for uid_gid_map

[Bradley Morgan]

Hi Bill,

> +config USER_NAMESPACE_KUNIT_TEST
> + tristate "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
> + depends on USER_NS && KUNIT

The test is #include'd into user_namespace.c, which is builtin (USER_NS
is a bool), so =m here still compiles the suite into vmlinux. With
KUNIT=m that calls kunit symbols that live in a module, and the link
fails. Make it bool and depend on KUNIT=y, like EXEC_KUNIT_TEST:

 bool "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
 depends on USER_NS && KUNIT=y

> + /* Verify sorting is correct */
> + for (i = 0; i < map.nr_extents; i++) {
> +  KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> +  KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
> + }

This doesn't verify any sorting. Every extent was inserted with count
5, so the loop passes even if sort_idmaps() did nothing. Either assert
that forward is ordered by .first and reverse by .lower_first, or drop
the sorting claim from the changelog.

> + /* Clean up allocations to avoid leaks */
> + kfree(map.forward);
> + kfree(map.reverse);

Nice.

No tag, add me into V2, please?

Thanks!

Re: [PATCH 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map

[Bradley Morgan]

On 23 August 2026 13:51:47 BST, Bill Wendling <morbo@google.com> wrote:
>The compiler attribute __counted_by_ptr associates a pointer field of a
>struct with a sibling field within the same struct that specifies the
>element count of the allocated memory. This enables KASAN and fortified
>bounds-checking to detect out-of-bounds accesses to the pointer field at
>runtime.
>

Ack.

>We can add the __counted_by_ptr attribute to the 'forward' and 'reverse'
>pointer fields of 'struct uid_gid_map', which are counted by
>'nr_extents'. Since 'nr_extents' is defined in a sibling anonymous
>struct inside an anonymous union, the nearest common non-anonymous
>struct level is 'struct uid_gid_map' itself, which is supported by the
>compiler.

love it.


>However, doing so has runtime implications. In the original
>implementation of insert_extent(), elements are written to
>map->forward[map->nr_extents] before map->nr_extents is incremented:
>

Resounding ack.


>	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
>		dest = &map->extent[map->nr_extents];
>	else
>		dest = &map->forward[map->nr_extents];
>
>	*dest = *extent;
>	map->nr_extents++;
>
>At the time of writing to 'map->forward[map->nr_extents]',
>map->nr_extents is still 5, but we are accessing index 5 (which is the
>6th element). Under __counted_by_ptr(nr_extents), the compiler and
>KASAN expect the accessed index to be strictly less than
>map->nr_extents. Therefore, accessing index 5 when the count is 5
>triggers an out-of-bounds panic/trap at runtime.

oh!


>To resolve this, insert_extent() is refactored to increment
>map->nr_extents first, and then use map->nr_extents - 1 as the index:
>
>	map->nr_extents++;
>	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
>		dest = &map->extent[map->nr_extents - 1];
>	else
>		dest = &map->forward[map->nr_extents - 1];
>
>	*dest = *extent;
>
>Assisted-by: Gemini Next

Id like to wonder what the hell that model is Gemini 3.5 pro?, I gave it a
Google and saw nothing.

Btw. 

Reviewed-by: Bradley Morgan <include@grrlz.net>




>Signed-off-by: Bill Wendling <morbo@google.com>
>---
>Cc: Kees Cook <kees@kernel.org>
>Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
>Cc: Christian Brauner <brauner@kernel.org>
>Cc: Aleksa Sarai <cyphar>
@cyphar.com>
>Cc: Jan Kara <jack@suse.cz>
>Cc: Nathan Chancellor <nathan@kernel.org>
>Cc: Miguel Ojeda <ojeda@kernel.org>
>Cc: Thomas Gleixner <tglx@kernel.org>
>Cc: Nicolas Schier <nsc@kernel.org>
>Cc: Gary Guo <gary@garyguo.net>
>Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
>Cc: Alice Ryhl <aliceryhl@google.com>
>Cc: Douglas Anderson <dianders@chromium.org>
>Cc: Anand Moon <linux.amoon@gmail.com>
>Cc: Oleg Nesterov <oleg@redhat.com>
>Cc: codemender-patching+linux@google.com
>Cc: linux-kernel@vger.kernel.org
>Cc: linux-hardening@vger.kernel.org
>---
> include/linux/user_namespace.h | 4 ++--
> kernel/user_namespace.c        | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
>index e38d9e60569f..2962256eddf7 100644
>--- a/include/linux/user_namespace.h
>+++ b/include/linux/user_namespace.h
>@@ -29,8 +29,8 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
> 			u32 nr_extents;
> 		};
> 		struct {
>-			struct uid_gid_extent *forward;
>-			struct uid_gid_extent *reverse;
>+			struct uid_gid_extent *forward __counted_by_ptr(nr_extents);
>+			struct uid_gid_extent *reverse __counted_by_ptr(nr_extents);
> 		};
> 	};
> };
>diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
>index 0bed462e9b2a..7e5371d8f515 100644
>--- a/kernel/user_namespace.c
>+++ b/kernel/user_namespace.c
>@@ -809,13 +809,13 @@ static int insert_extent(struct uid_gid_map *map, struct uid_gid_extent *extent)
> 		map->reverse = NULL;
> 	}
> 
>-	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
>-		dest = &map->extent[map->nr_extents];
>+	map->nr_extents++;
>+	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
>+		dest = &map->extent[map->nr_extents - 1];
> 	else
>-		dest = &map->forward[map->nr_extents];
>+		dest = &map->forward[map->nr_extents - 1];
> 
> 	*dest = *extent;
>-	map->nr_extents++;
> 	return 0;
> }
> 
>

Thanks!

Re: [PATCH 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map

[Oleg Nesterov]

On 08/23, Bill Wendling wrote:
>
> The compiler attribute __counted_by_ptr associates a pointer field of a
> struct with a sibling field within the same struct that specifies the
> element count of the allocated memory. This enables KASAN and fortified
> bounds-checking to detect out-of-bounds accesses to the pointer field at
> runtime.
>
> We can add the __counted_by_ptr attribute to the 'forward' and 'reverse'
> pointer fields of 'struct uid_gid_map', which are counted by
> 'nr_extents'. Since 'nr_extents' is defined in a sibling anonymous
> struct inside an anonymous union, the nearest common non-anonymous
> struct level is 'struct uid_gid_map' itself, which is supported by the
> compiler.
>
> However, doing so has runtime implications. In the original
> implementation of insert_extent(), elements are written to
> map->forward[map->nr_extents] before map->nr_extents is incremented:
>
> 	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
> 		dest = &map->extent[map->nr_extents];
> 	else
> 		dest = &map->forward[map->nr_extents];
>
> 	*dest = *extent;
> 	map->nr_extents++;
>
> At the time of writing to 'map->forward[map->nr_extents]',
> map->nr_extents is still 5, but we are accessing index 5 (which is the
> 6th element). Under __counted_by_ptr(nr_extents), the compiler and
> KASAN expect the accessed index to be strictly less than
> map->nr_extents. Therefore, accessing index 5 when the count is 5
> triggers an out-of-bounds panic/trap at runtime.
>
> To resolve this, insert_extent() is refactored to increment
> map->nr_extents first, and then use map->nr_extents - 1 as the index:
>
> 	map->nr_extents++;
> 	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
> 		dest = &map->extent[map->nr_extents - 1];
> 	else
> 		dest = &map->forward[map->nr_extents - 1];
>
> 	*dest = *extent;

I leave this to you and other reviewers (add Alexey), you can safely
ignore my nit.

To me

	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
		dest = &map->extent;
	else
		dest = &map->forward;

	map->nr_extents++;
	dest[map->nr_extents - 1] = *extent;

looks a bit more clear, but this is minor/subjective.

Either way, I think this needs a short comment to explain why do we
need to increment ->nr_extents first, then subtract 1. IOW, to explain
why (say)

	dest[map->nr_extents++] = *extent;

would be wrong.

Oleg.

> 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: Christian Brauner <brauner@kernel.org>
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Thomas Gleixner <tglx@kernel.org>
> Cc: Nicolas Schier <nsc@kernel.org>
> Cc: Gary Guo <gary@garyguo.net>
> Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Anand Moon <linux.amoon@gmail.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: codemender-patching+linux@google.com
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-hardening@vger.kernel.org
> ---
>  include/linux/user_namespace.h | 4 ++--
>  kernel/user_namespace.c        | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index e38d9e60569f..2962256eddf7 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -29,8 +29,8 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
>  			u32 nr_extents;
>  		};
>  		struct {
> -			struct uid_gid_extent *forward;
> -			struct uid_gid_extent *reverse;
> +			struct uid_gid_extent *forward __counted_by_ptr(nr_extents);
> +			struct uid_gid_extent *reverse __counted_by_ptr(nr_extents);
>  		};
>  	};
>  };
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 0bed462e9b2a..7e5371d8f515 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -809,13 +809,13 @@ static int insert_extent(struct uid_gid_map *map, struct uid_gid_extent *extent)
>  		map->reverse = NULL;
>  	}
>  
> -	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
> -		dest = &map->extent[map->nr_extents];
> +	map->nr_extents++;
> +	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
> +		dest = &map->extent[map->nr_extents - 1];
>  	else
> -		dest = &map->forward[map->nr_extents];
> +		dest = &map->forward[map->nr_extents - 1];
>  
>  	*dest = *extent;
> -	map->nr_extents++;
>  	return 0;
>  }
>  
> -- 
> 2.55.0.860.g4b6b3295ed-goog
> 

Re: [PATCH 2/2] userns: Add KUnit test suite for uid_gid_map

[Thomas Weißschuh]

On Sun, Aug 23, 2026 at 12:51:48PM +0000, Bill Wendling wrote:
> Add a KUnit test suite to verify the insertion and sorting of mappings
> in struct uid_gid_map. This test suite validates both base extent
> insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
> which triggers the allocation of the forward and reverse pointers).
> 
> This is especially useful for verifying that the __counted_by_ptr
> attribute added to 'forward' and 'reverse' pointers works correctly
> without causing any runtime bounds-checking panics or traps.

AFAIU patch 1 is supposed to not change any behavior.
You could move the unit test to the front to make that clearer
and also validate it.

> Assisted-by: Gemini Next
> Change-Id: If0c2c197a35cd7429cf0d2d6e3b33f0d9f0be66c

Change-Id should not be used upstream.
See Documentation/dev-tools/checkpatch.rst.

> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> Cc: Kees Cook <kees@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Thomas Gleixner <tglx@kernel.org>
> Cc: Nicolas Schier <nsc@kernel.org>
> Cc: Gary Guo <gary@garyguo.net>
> Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Anand Moon <linux.amoon@gmail.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: codemender-patching+linux@google.com
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-hardening@vger.kernel.org
> ---
>  init/Kconfig                  | 10 ++++
>  kernel/.kunitconfig           |  3 ++
>  kernel/user_namespace.c       |  4 ++
>  kernel/user_namespace_kunit.c | 87 +++++++++++++++++++++++++++++++++++
>  4 files changed, 104 insertions(+)
>  create mode 100644 kernel/.kunitconfig
>  create mode 100644 kernel/user_namespace_kunit.c

(...)

> +static void test_user_ns_map_insert_extended(struct kunit *test)
> +{
> +	struct uid_gid_map map;
> +	struct uid_gid_extent extent;
> +	int i, ret;

(...)

> +	/* Now sort the map to set up reverse mapping */
> +	ret = sort_idmaps(&map);
> +	KUNIT_EXPECT_EQ(test, ret, 0);
> +	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, map.reverse);

KUNIT_EXPECT_*() will *not* abort the test when the assertion fails ...

> +
> +	/* Verify sorting is correct */
> +	for (i = 0; i < map.nr_extents; i++) {
> +		KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> +		KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);

... leading to a crash here if map.reverse is invalid.

To also abort the test on assertion failure use KUNIT_ASSERT_*().

> +	}
> +
> +	/* Clean up allocations to avoid leaks */

Pointless comment. This is true for every single call of kfree().

> +	kfree(map.forward);
> +	kfree(map.reverse);
> +}

(...)

Re: [PATCH 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map

[Gustavo A. R. Silva]

On 8/23/26 06:51, Bill Wendling wrote:
> The compiler attribute __counted_by_ptr associates a pointer field of a
> struct with a sibling field within the same struct that specifies the
> element count of the allocated memory. This enables KASAN and fortified
> bounds-checking to detect out-of-bounds accesses to the pointer field at
> runtime.
> 
> We can add the __counted_by_ptr attribute to the 'forward' and 'reverse'
> pointer fields of 'struct uid_gid_map', which are counted by
> 'nr_extents'. Since 'nr_extents' is defined in a sibling anonymous
> struct inside an anonymous union, the nearest common non-anonymous
> struct level is 'struct uid_gid_map' itself, which is supported by the
> compiler.
> 
> However, doing so has runtime implications. In the original
> implementation of insert_extent(), elements are written to
> map->forward[map->nr_extents] before map->nr_extents is incremented:
> 
> 	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
> 		dest = &map->extent[map->nr_extents];
> 	else
> 		dest = &map->forward[map->nr_extents];
> 
> 	*dest = *extent;
> 	map->nr_extents++;
> 
> At the time of writing to 'map->forward[map->nr_extents]',
> map->nr_extents is still 5, but we are accessing index 5 (which is the
> 6th element). Under __counted_by_ptr(nr_extents), the compiler and
> KASAN expect the accessed index to be strictly less than
> map->nr_extents. Therefore, accessing index 5 when the count is 5
> triggers an out-of-bounds panic/trap at runtime.
> 
> To resolve this, insert_extent() is refactored to increment
> map->nr_extents first, and then use map->nr_extents - 1 as the index:
> 
> 	map->nr_extents++;
> 	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
> 		dest = &map->extent[map->nr_extents - 1];
> 	else
> 		dest = &map->forward[map->nr_extents - 1];
> 
> 	*dest = *extent;
> 
> Assisted-by: Gemini Next
> Signed-off-by: Bill Wendling <morbo@google.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
-Gustavo

> ---
> Cc: Kees Cook <kees@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Thomas Gleixner <tglx@kernel.org>
> Cc: Nicolas Schier <nsc@kernel.org>
> Cc: Gary Guo <gary@garyguo.net>
> Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Anand Moon <linux.amoon@gmail.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: codemender-patching+linux@google.com
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-hardening@vger.kernel.org
> ---
>   include/linux/user_namespace.h | 4 ++--
>   kernel/user_namespace.c        | 8 ++++----
>   2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index e38d9e60569f..2962256eddf7 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -29,8 +29,8 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
>   			u32 nr_extents;
>   		};
>   		struct {
> -			struct uid_gid_extent *forward;
> -			struct uid_gid_extent *reverse;
> +			struct uid_gid_extent *forward __counted_by_ptr(nr_extents);
> +			struct uid_gid_extent *reverse __counted_by_ptr(nr_extents);
>   		};
>   	};
>   };
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 0bed462e9b2a..7e5371d8f515 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -809,13 +809,13 @@ static int insert_extent(struct uid_gid_map *map, struct uid_gid_extent *extent)
>   		map->reverse = NULL;
>   	}
>   
> -	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
> -		dest = &map->extent[map->nr_extents];
> +	map->nr_extents++;
> +	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
> +		dest = &map->extent[map->nr_extents - 1];
>   	else
> -		dest = &map->forward[map->nr_extents];
> +		dest = &map->forward[map->nr_extents - 1];
>   
>   	*dest = *extent;
> -	map->nr_extents++;
>   	return 0;
>   }
>   

Re: [PATCH 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

Hi Bradley,

On Sun, Aug 23, 2026 at 6:48 AM Bradley Morgan <include@grrlz.net> wrote:
>
> Hi Bill,
>
> > +config USER_NAMESPACE_KUNIT_TEST
> > + tristate "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
> > + depends on USER_NS && KUNIT
>
> The test is #include'd into user_namespace.c, which is builtin (USER_NS
> is a bool), so =m here still compiles the suite into vmlinux. With
> KUNIT=m that calls kunit symbols that live in a module, and the link
> fails. Make it bool and depend on KUNIT=y, like EXEC_KUNIT_TEST:
>
>  bool "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
>  depends on USER_NS && KUNIT=y
>
Okay.

> > + /* Verify sorting is correct */
> > + for (i = 0; i < map.nr_extents; i++) {
> > +  KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> > +  KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
> > + }
>
> This doesn't verify any sorting. Every extent was inserted with count
> 5, so the loop passes even if sort_idmaps() did nothing. Either assert
> that forward is ordered by .first and reverse by .lower_first, or drop
> the sorting claim from the changelog.
>
Good call. I added in checking for actual values.

> > + /* Clean up allocations to avoid leaks */
> > + kfree(map.forward);
> > + kfree(map.reverse);
>
> Nice.
>
> No tag, add me into V2, please?
>
Done!

-bw

Re: [PATCH 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

Hi Thomas,

On Sun, Aug 23, 2026 at 11:13 PM Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> On Sun, Aug 23, 2026 at 12:51:48PM +0000, Bill Wendling wrote:
> > Add a KUnit test suite to verify the insertion and sorting of mappings
> > in struct uid_gid_map. This test suite validates both base extent
> > insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
> > which triggers the allocation of the forward and reverse pointers).
> >
> > This is especially useful for verifying that the __counted_by_ptr
> > attribute added to 'forward' and 'reverse' pointers works correctly
> > without causing any runtime bounds-checking panics or traps.
>
> AFAIU patch 1 is supposed to not change any behavior.
> You could move the unit test to the front to make that clearer
> and also validate it.
>
Patch 1 generates code to assert if a memory address outside of the
bounds of a pointer is accessed. So it does change the behavior.
Should I still move the test case?

> > Assisted-by: Gemini Next
> > Change-Id: If0c2c197a35cd7429cf0d2d6e3b33f0d9f0be66c
>
> Change-Id should not be used upstream.
> See Documentation/dev-tools/checkpatch.rst.
>
I had removed it in v2. :-)

> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> > Cc: Kees Cook <kees@kernel.org>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Christian Brauner <brauner@kernel.org>
> > Cc: Aleksa Sarai <cyphar@cyphar.com>
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Cc: Thomas Gleixner <tglx@kernel.org>
> > Cc: Nicolas Schier <nsc@kernel.org>
> > Cc: Gary Guo <gary@garyguo.net>
> > Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
> > Cc: Alice Ryhl <aliceryhl@google.com>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Cc: Anand Moon <linux.amoon@gmail.com>
> > Cc: Oleg Nesterov <oleg@redhat.com>
> > Cc: codemender-patching+linux@google.com
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-hardening@vger.kernel.org
> > ---
> >  init/Kconfig                  | 10 ++++
> >  kernel/.kunitconfig           |  3 ++
> >  kernel/user_namespace.c       |  4 ++
> >  kernel/user_namespace_kunit.c | 87 +++++++++++++++++++++++++++++++++++
> >  4 files changed, 104 insertions(+)
> >  create mode 100644 kernel/.kunitconfig
> >  create mode 100644 kernel/user_namespace_kunit.c
>
> (...)
>
> > +static void test_user_ns_map_insert_extended(struct kunit *test)
> > +{
> > +     struct uid_gid_map map;
> > +     struct uid_gid_extent extent;
> > +     int i, ret;
>
> (...)
>
> > +     /* Now sort the map to set up reverse mapping */
> > +     ret = sort_idmaps(&map);
> > +     KUNIT_EXPECT_EQ(test, ret, 0);
> > +     KUNIT_EXPECT_NOT_ERR_OR_NULL(test, map.reverse);
>
> KUNIT_EXPECT_*() will *not* abort the test when the assertion fails ...
>
> > +
> > +     /* Verify sorting is correct */
> > +     for (i = 0; i < map.nr_extents; i++) {
> > +             KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> > +             KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
>
> ... leading to a crash here if map.reverse is invalid.
>
> To also abort the test on assertion failure use KUNIT_ASSERT_*().
>
Roger that! I changed them to KUNIT_ASSERT_*.

> > +     }
> > +
> > +     /* Clean up allocations to avoid leaks */
>
> Pointless comment. This is true for every single call of kfree().
>
Kiboshed.

Thanks!
-bw

[PATCH v3 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

Add a KUnit test suite to verify the insertion and sorting of mappings
in struct uid_gid_map. This test suite validates both base extent
insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
which triggers the allocation of the forward and reverse pointers).

This is especially useful for verifying that the __counted_by_ptr
attribute added to 'forward' and 'reverse' pointers works correctly
without causing any runtime bounds-checking panics or traps.

Assisted-by: Gemini Next
Signed-off-by: Bill Wendling <morbo@google.com>
---
v2 - Remove Gerrit tag.
v3 - s/KUNIT_EXPECT_NOT_ERR_OR_NULL/KUNIT_ASSERT_NOT_ERR_OR_NULL/
   - Fixed Kconfig tests.
---
Cc: Bradley Morgan <brads@mainlining.org>
Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Cc: Kees Cook <kees@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Nicolas Schier <nsc@kernel.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Anand Moon <linux.amoon@gmail.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: codemender-patching+linux@google.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
---
 init/Kconfig                  | 10 ++++
 kernel/.kunitconfig           |  3 ++
 kernel/user_namespace.c       |  4 ++
 kernel/user_namespace_kunit.c | 91 +++++++++++++++++++++++++++++++++++
 4 files changed, 108 insertions(+)
 create mode 100644 kernel/.kunitconfig
 create mode 100644 kernel/user_namespace_kunit.c

diff --git a/init/Kconfig b/init/Kconfig
index f63bf5e05e79..ba6a40b7315a 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1457,6 +1457,16 @@ config USER_NS
 
 	  If unsure, say N.
 
+config USER_NAMESPACE_KUNIT_TEST
+	bool "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
+	depends on USER_NS && KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the KUnit test for user namespace uid/gid map insertion.
+	  It validates map insertion, limits, dynamic allocation of the
+	  extended extents array, and mapping sorting functions.
+	  If unsure, say N.
+
 config PID_NS
 	bool "PID Namespaces"
 	default y
diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
new file mode 100644
index 000000000000..7314dce05dc2
--- /dev/null
+++ b/kernel/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_USER_NS=y
+CONFIG_USER_NAMESPACE_KUNIT_TEST=y
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 7e5371d8f515..64c64e1028e8 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -1413,3 +1413,7 @@ static __init int user_namespaces_init(void)
 	return 0;
 }
 subsys_initcall(user_namespaces_init);
+
+#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
+#include "user_namespace_kunit.c"
+#endif
diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
new file mode 100644
index 000000000000..d97dbac65e50
--- /dev/null
+++ b/kernel/user_namespace_kunit.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for user namespace map insertion and sorting.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+
+static void test_user_ns_map_insert_base(struct kunit *test)
+{
+	struct uid_gid_map map;
+	struct uid_gid_extent extent;
+	int i, ret;
+
+	memset(&map, 0, sizeof(map));
+
+	/* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
+	for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
+		extent.first = i * 10;
+		extent.lower_first = i * 100;
+		extent.count = 5;
+
+		ret = insert_extent(&map, &extent);
+		KUNIT_EXPECT_EQ(test, ret, 0);
+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
+		KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
+		KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
+		KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
+	}
+}
+
+static void test_user_ns_map_insert_extended(struct kunit *test)
+{
+	struct uid_gid_map map;
+	struct uid_gid_extent extent;
+	int i, ret;
+
+	memset(&map, 0, sizeof(map));
+
+	/* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
+	for (i = 0; i < 10; i++) {
+		extent.first = i * 10;
+		extent.lower_first = i * 100;
+		extent.count = 5;
+
+		ret = insert_extent(&map, &extent);
+		KUNIT_EXPECT_EQ(test, ret, 0);
+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
+
+		if (i < UID_GID_MAP_MAX_BASE_EXTENTS) {
+			KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
+		} else {
+			KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.forward);
+			KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
+			KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
+			KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
+		}
+	}
+
+	/* Now sort the map to set up reverse mapping */
+	ret = sort_idmaps(&map);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.reverse);
+
+	/* Verify sorting is correct */
+	for (i = 0; i < map.nr_extents; i++) {
+		KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
+		KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
+		KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
+
+		KUNIT_EXPECT_EQ(test, map.reverse[i].first, i * 10);
+		KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);
+		KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
+	}
+
+	kfree(map.forward);
+	kfree(map.reverse);
+}
+
+static struct kunit_case user_ns_map_test_cases[] = {
+	KUNIT_CASE(test_user_ns_map_insert_base),
+	KUNIT_CASE(test_user_ns_map_insert_extended),
+	{}
+};
+
+static struct kunit_suite user_ns_map_test_suite = {
+	.name = "user_ns_map",
+	.test_cases = user_ns_map_test_cases,
+};
+
+kunit_test_suite(user_ns_map_test_suite);
-- 
2.55.0.897.gb25b4bd76c-goog

Re: [PATCH v3 2/2] userns: Add KUnit test suite for uid_gid_map

[Bradley Morgan]

> + /* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
> + for (i = 0; i < 10; i++) {
> +   extent.first = i * 10;
> +   extent.lower_first = i * 100;

Both .first and .lower_first increase monotonically with i. So
forward[] is already sorted by .first and reverse[] is already sorted
by .lower_first before sort_idmaps() runs. The sort is a no op on
this input.

> + /* Verify sorting is correct */
> + for (i = 0; i < map.nr_extents; i++) {
> +   KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
> +   KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);

This passes even if sort_idmaps() did nothing. The test cannot tell
a working sort from a broken one.

To actually verify sorting, insert the extents out of order so that
sort_idmaps() has to rearrange them. For example:

    extent.first = (9 - i) * 10;
    extent.lower_first = (9 - i) * 100;

Then after sort_idmaps(), forward[0].first should be 0, forward[1]
should be 10, etc. That proves the sort moved things.

Nits:

> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.forward);

Redundant. insert_extent() just allocated it on the 6th call, and
the previous KUNIT_EXPECT_EQ(test, ret, 0) already confirmed success.
If you want to keep it, at least put it after the loop where it
protects the sort, not inside the insertion loop where it tests
something that just succeeded.

> + KUNIT_EXPECT_EQ(test, ret, 0);

Should be KUNIT_ASSERT_EQ. If insert_extent() fails the test
continues and the next assertions run on uninitialized data. Same
for the base test.

No tag yet, fix the test data and I'll tag v4.


--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

[PATCH v4 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map

[Bill Wendling]

The compiler attribute __counted_by_ptr associates a pointer field of a
struct with a sibling field within the same struct that specifies the
element count of the allocated memory. This enables KASAN and fortified
bounds-checking to detect out-of-bounds accesses to the pointer field at
runtime.

We can add the __counted_by_ptr attribute to the 'forward' and 'reverse'
pointer fields of 'struct uid_gid_map', which are counted by
'nr_extents'. Since 'nr_extents' is defined in a sibling anonymous
struct inside an anonymous union, the nearest common non-anonymous
struct level is 'struct uid_gid_map' itself, which is supported by the
compiler.

However, doing so has runtime implications. In the original
implementation of insert_extent(), elements are written to
map->forward[map->nr_extents] before map->nr_extents is incremented:

	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
		dest = &map->extent[map->nr_extents];
	else
		dest = &map->forward[map->nr_extents];

	*dest = *extent;
	map->nr_extents++;

At the time of writing to 'map->forward[map->nr_extents]',
map->nr_extents is still 5, but we are accessing index 5 (which is the
6th element). Under __counted_by_ptr(nr_extents), the compiler and
KASAN expect the accessed index to be strictly less than
map->nr_extents. Therefore, accessing index 5 when the count is 5
triggers an out-of-bounds panic/trap at runtime.

To resolve this, insert_extent() is refactored to increment
map->nr_extents first, and then use map->nr_extents - 1 as the index:

	map->nr_extents++;
	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
		dest = &map->extent[map->nr_extents - 1];
	else
		dest = &map->forward[map->nr_extents - 1];

	*dest = *extent;

Assisted-by: Gemini:3.1-pro-preview
Signed-off-by: Bill Wendling <morbo@google.com>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Reviewed-by: Bradley Morgan <brads@mainlining.org>
---
v2 - Remove Gerrit tag.
v4 - Added comment explaning the change. Corrected the "Assisted-by" tag.
---
Cc: Bradley Morgan <brads@mainlining.org>
Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Cc: Kees Cook <kees@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Nicolas Schier <nsc@kernel.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Anand Moon <linux.amoon@gmail.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: codemender-patching+linux@google.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
---
 include/linux/user_namespace.h |  4 ++--
 kernel/user_namespace.c        | 12 ++++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index e38d9e60569f..2962256eddf7 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -29,8 +29,8 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
 			u32 nr_extents;
 		};
 		struct {
-			struct uid_gid_extent *forward;
-			struct uid_gid_extent *reverse;
+			struct uid_gid_extent *forward __counted_by_ptr(nr_extents);
+			struct uid_gid_extent *reverse __counted_by_ptr(nr_extents);
 		};
 	};
 };
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 0bed462e9b2a..786dbf0506ca 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -809,13 +809,17 @@ static int insert_extent(struct uid_gid_map *map, struct uid_gid_extent *extent)
 		map->reverse = NULL;
 	}
 
-	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
-		dest = &map->extent[map->nr_extents];
+	/*
+	 * nr_extents must be updated before the extent and forward arrays are
+	 * accessed, otherwise KSAN will assert an out-of-bounds error.
+	 */
+	map->nr_extents++;
+	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
+		dest = &map->extent[map->nr_extents - 1];
 	else
-		dest = &map->forward[map->nr_extents];
+		dest = &map->forward[map->nr_extents - 1];
 
 	*dest = *extent;
-	map->nr_extents++;
 	return 0;
 }
 
-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

Add a KUnit test suite to verify the insertion and sorting of mappings
in struct uid_gid_map. This test suite validates both base extent
insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
which triggers the allocation of the forward and reverse pointers).

This is especially useful for verifying that the __counted_by_ptr
attribute added to 'forward' and 'reverse' pointers works correctly
without causing any runtime bounds-checking panics or traps.

Assisted-by: Gemini:3.1-pro-preview
Signed-off-by: Bill Wendling <morbo@google.com>
---
v2 - Remove Gerrit tag.
v3 - s/KUNIT_EXPECT_NOT_ERR_OR_NULL/KUNIT_ASSERT_NOT_ERR_OR_NULL/
   - Fixed Kconfig tests.
v4 - Actually test on unsorted data. Corrected the "Assisted-by" tag.
---
Cc: Bradley Morgan <brads@mainlining.org>
Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Cc: Kees Cook <kees@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Nicolas Schier <nsc@kernel.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Anand Moon <linux.amoon@gmail.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: codemender-patching+linux@google.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
---
 init/Kconfig                  | 10 ++++
 kernel/.kunitconfig           |  3 ++
 kernel/user_namespace.c       |  4 ++
 kernel/user_namespace_kunit.c | 92 +++++++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+)
 create mode 100644 kernel/.kunitconfig
 create mode 100644 kernel/user_namespace_kunit.c

diff --git a/init/Kconfig b/init/Kconfig
index f63bf5e05e79..d460344539a5 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1457,6 +1457,16 @@ config USER_NS
 
 	  If unsure, say N.
 
+config USER_NAMESPACE_KUNIT_TEST
+	bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
+	depends on KUNIT=y
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the KUnit test for user namespace uid/gid map insertion.
+	  It validates map insertion, limits, dynamic allocation of the
+	  extended extents array, and mapping sorting functions.
+	  If unsure, say N.
+
 config PID_NS
 	bool "PID Namespaces"
 	default y
diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
new file mode 100644
index 000000000000..7314dce05dc2
--- /dev/null
+++ b/kernel/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_USER_NS=y
+CONFIG_USER_NAMESPACE_KUNIT_TEST=y
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 786dbf0506ca..0e7373085af9 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -1417,3 +1417,7 @@ static __init int user_namespaces_init(void)
 	return 0;
 }
 subsys_initcall(user_namespaces_init);
+
+#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
+#include "user_namespace_kunit.c"
+#endif
diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
new file mode 100644
index 000000000000..88467361efdf
--- /dev/null
+++ b/kernel/user_namespace_kunit.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for user namespace map insertion and sorting.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+
+static void test_user_ns_map_insert_base(struct kunit *test)
+{
+	struct uid_gid_map map;
+	struct uid_gid_extent extent;
+	int i, ret;
+
+	memset(&map, 0, sizeof(map));
+
+	/* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
+	for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
+		extent.first = i * 10;
+		extent.lower_first = i * 100;
+		extent.count = 5;
+
+		ret = insert_extent(&map, &extent);
+		KUNIT_ASSERT_EQ(test, ret, 0);
+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
+		KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
+		KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
+		KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
+	}
+}
+
+static void test_user_ns_map_insert_extended(struct kunit *test)
+{
+	struct uid_gid_map map;
+	struct uid_gid_extent extent;
+	int i, ret;
+
+	memset(&map, 0, sizeof(map));
+
+	/* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
+	for (i = 0; i < 10; i++) {
+		int value = 9 - i;
+
+		extent.first = value * 10;
+		extent.lower_first = value * 100;
+		extent.count = 5;
+
+		ret = insert_extent(&map, &extent);
+		KUNIT_ASSERT_EQ(test, ret, 0);
+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
+
+		if (i < UID_GID_MAP_MAX_BASE_EXTENTS) {
+			KUNIT_EXPECT_EQ(test, map.extent[i].first, value * 10);
+		} else {
+			KUNIT_EXPECT_EQ(test, map.forward[i].first, value * 10);
+			KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, value  * 100);
+			KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
+		}
+	}
+
+	/* Now sort the map to set up reverse mapping */
+	ret = sort_idmaps(&map);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.reverse);
+
+	/* Verify sorting is correct */
+	for (i = 0; i < map.nr_extents; i++) {
+		KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
+		KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
+		KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
+
+		KUNIT_EXPECT_EQ(test, map.reverse[i].first, i * 10);
+		KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);
+		KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
+	}
+
+	kfree(map.forward);
+	kfree(map.reverse);
+}
+
+static struct kunit_case user_ns_map_test_cases[] = {
+	KUNIT_CASE(test_user_ns_map_insert_base),
+	KUNIT_CASE(test_user_ns_map_insert_extended),
+	{}
+};
+
+static struct kunit_suite user_ns_map_test_suite = {
+	.name = "user_ns_map",
+	.test_cases = user_ns_map_test_cases,
+};
+
+kunit_test_suite(user_ns_map_test_suite);
-- 
2.55.0.897.gb25b4bd76c-goog

Re: [PATCH v3 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

On Wed, Aug 26, 2026 at 2:21 PM Bradley Morgan <brads@mainlining.org> wrote:
>
> > + /* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
> > + for (i = 0; i < 10; i++) {
> > +   extent.first = i * 10;
> > +   extent.lower_first = i * 100;
>
> Both .first and .lower_first increase monotonically with i. So
> forward[] is already sorted by .first and reverse[] is already sorted
> by .lower_first before sort_idmaps() runs. The sort is a no op on
> this input.
>
> > + /* Verify sorting is correct */
> > + for (i = 0; i < map.nr_extents; i++) {
> > +   KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
> > +   KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);
>
> This passes even if sort_idmaps() did nothing. The test cannot tell
> a working sort from a broken one.
>
> To actually verify sorting, insert the extents out of order so that
> sort_idmaps() has to rearrange them. For example:
>
>     extent.first = (9 - i) * 10;
>     extent.lower_first = (9 - i) * 100;
>
> Then after sort_idmaps(), forward[0].first should be 0, forward[1]
> should be 10, etc. That proves the sort moved things.
>
> Nits:
>
> > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.forward);
>
> Redundant. insert_extent() just allocated it on the 6th call, and
> the previous KUNIT_EXPECT_EQ(test, ret, 0) already confirmed success.
> If you want to keep it, at least put it after the loop where it
> protects the sort, not inside the insertion loop where it tests
> something that just succeeded.
>
> > + KUNIT_EXPECT_EQ(test, ret, 0);
>
> Should be KUNIT_ASSERT_EQ. If insert_extent() fails the test
> continues and the next assertions run on uninitialized data. Same
> for the base test.
>
Billionth times the charm!

I hope I've addressed everyone's concerns. Obviously let me know if I
didn't. :-)

> No tag yet, fix the test data and I'll tag v4.

I added your "Reviewed-by" on "PATCH 1/2", but not this one.

-bw

>
> --- Thanks!
> https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

Re: [PATCH 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map

[Bill Wendling]

On Sun, Aug 23, 2026 at 7:52 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 08/23, Bill Wendling wrote:
> >
> > The compiler attribute __counted_by_ptr associates a pointer field of a
> > struct with a sibling field within the same struct that specifies the
> > element count of the allocated memory. This enables KASAN and fortified
> > bounds-checking to detect out-of-bounds accesses to the pointer field at
> > runtime.
> >
> > We can add the __counted_by_ptr attribute to the 'forward' and 'reverse'
> > pointer fields of 'struct uid_gid_map', which are counted by
> > 'nr_extents'. Since 'nr_extents' is defined in a sibling anonymous
> > struct inside an anonymous union, the nearest common non-anonymous
> > struct level is 'struct uid_gid_map' itself, which is supported by the
> > compiler.
> >
> > However, doing so has runtime implications. In the original
> > implementation of insert_extent(), elements are written to
> > map->forward[map->nr_extents] before map->nr_extents is incremented:
> >
> >       if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
> >               dest = &map->extent[map->nr_extents];
> >       else
> >               dest = &map->forward[map->nr_extents];
> >
> >       *dest = *extent;
> >       map->nr_extents++;
> >
> > At the time of writing to 'map->forward[map->nr_extents]',
> > map->nr_extents is still 5, but we are accessing index 5 (which is the
> > 6th element). Under __counted_by_ptr(nr_extents), the compiler and
> > KASAN expect the accessed index to be strictly less than
> > map->nr_extents. Therefore, accessing index 5 when the count is 5
> > triggers an out-of-bounds panic/trap at runtime.
> >
> > To resolve this, insert_extent() is refactored to increment
> > map->nr_extents first, and then use map->nr_extents - 1 as the index:
> >
> >       map->nr_extents++;
> >       if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
> >               dest = &map->extent[map->nr_extents - 1];
> >       else
> >               dest = &map->forward[map->nr_extents - 1];
> >
> >       *dest = *extent;
>
> I leave this to you and other reviewers (add Alexey), you can safely
> ignore my nit.
>
> To me
>
>         if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
>                 dest = &map->extent;
>         else
>                 dest = &map->forward;
>
>         map->nr_extents++;
>         dest[map->nr_extents - 1] = *extent;
>
> looks a bit more clear, but this is minor/subjective.
>
> Either way, I think this needs a short comment to explain why do we
> need to increment ->nr_extents first, then subtract 1. IOW, to explain
> why (say)
>
>         dest[map->nr_extents++] = *extent;
>
> would be wrong.
>
Hi Oleg,

I added a comment before the code in question explaining why it's
organized that way. I certainly don't hate your version, but for
simplicity of reviews I just left the code as is.

-bw

> Oleg.
>
> > 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: Christian Brauner <brauner@kernel.org>
> > Cc: Aleksa Sarai <cyphar@cyphar.com>
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Cc: Thomas Gleixner <tglx@kernel.org>
> > Cc: Nicolas Schier <nsc@kernel.org>
> > Cc: Gary Guo <gary@garyguo.net>
> > Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
> > Cc: Alice Ryhl <aliceryhl@google.com>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Cc: Anand Moon <linux.amoon@gmail.com>
> > Cc: Oleg Nesterov <oleg@redhat.com>
> > Cc: codemender-patching+linux@google.com
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-hardening@vger.kernel.org
> > ---
> >  include/linux/user_namespace.h | 4 ++--
> >  kernel/user_namespace.c        | 8 ++++----
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> > index e38d9e60569f..2962256eddf7 100644
> > --- a/include/linux/user_namespace.h
> > +++ b/include/linux/user_namespace.h
> > @@ -29,8 +29,8 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
> >                       u32 nr_extents;
> >               };
> >               struct {
> > -                     struct uid_gid_extent *forward;
> > -                     struct uid_gid_extent *reverse;
> > +                     struct uid_gid_extent *forward __counted_by_ptr(nr_extents);
> > +                     struct uid_gid_extent *reverse __counted_by_ptr(nr_extents);
> >               };
> >       };
> >  };
> > diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> > index 0bed462e9b2a..7e5371d8f515 100644
> > --- a/kernel/user_namespace.c
> > +++ b/kernel/user_namespace.c
> > @@ -809,13 +809,13 @@ static int insert_extent(struct uid_gid_map *map, struct uid_gid_extent *extent)
> >               map->reverse = NULL;
> >       }
> >
> > -     if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
> > -             dest = &map->extent[map->nr_extents];
> > +     map->nr_extents++;
> > +     if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
> > +             dest = &map->extent[map->nr_extents - 1];
> >       else
> > -             dest = &map->forward[map->nr_extents];
> > +             dest = &map->forward[map->nr_extents - 1];
> >
> >       *dest = *extent;
> > -     map->nr_extents++;
> >       return 0;
> >  }
> >
> > --
> > 2.55.0.860.g4b6b3295ed-goog
> >
>

Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Bradley Morgan]

On 26 August 2026 23:00:35 BST, Bill Wendling <morbo@google.com> wrote:
>Add a KUnit test suite to verify the insertion and sorting of mappings
>in struct uid_gid_map. This test suite validates both base extent
>insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
>which triggers the allocation of the forward and reverse pointers).
>
>This is especially useful for verifying that the __counted_by_ptr
>attribute added to 'forward' and 'reverse' pointers works correctly
>without causing any runtime bounds-checking panics or traps.
>
>Assisted-by: Gemini:3.1-pro-preview

Now for all the work s/you/your AI done, I've done the tests and they
passed, and they LGTM

Tested-by: Bradley Morgan <brads@mainlining.org>

Reviewed-by: Bradley Morgan <brads@mainlining.org>

Thanks for your patch mate, (pls slow down tho, we can review oh so quick)

>Signed-off-by: Bill Wendling <morbo@google.com>
>---
>v2 - Remove Gerrit tag.
>v3 - s/KUNIT_EXPECT_NOT_ERR_OR_NULL/KUNIT_ASSERT_NOT_ERR_OR_NULL/
>   - Fixed Kconfig tests.
>v4 - Actually test on unsorted data. Corrected the "Assisted-by" tag.
>---
>Cc: Bradley Morgan <brads@mainlining.org>
>Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>Cc: Kees Cook <kees@kernel.org>
>Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
>Cc: Christian Brauner <brauner@kernel.org>
>Cc: Aleksa Sarai <cyphar@cyphar.com>
>Cc: Jan Kara <jack@suse.cz>
>Cc: Nathan Chancellor <nathan@kernel.org>
>Cc: Miguel Ojeda <ojeda@kernel.org>
>Cc: Thomas Gleixner <tglx@kernel.org>
>Cc: Nicolas Schier <nsc@kernel.org>
>Cc: Gary Guo <gary@garyguo.net>
>Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
>Cc: Alice Ryhl <aliceryhl@google.com>
>Cc: Douglas Anderson <dianders@chromium.org>
>Cc: Anand Moon <linux.amoon@gmail.com>
>Cc: Oleg Nesterov <oleg@redhat.com>
>Cc: codemender-patching+linux@google.com
>Cc: linux-kernel@vger.kernel.org
>Cc: linux-hardening@vger.kernel.org
>---
> init/Kconfig                  | 10 ++++
> kernel/.kunitconfig           |  3 ++
> kernel/user_namespace.c       |  4 ++
> kernel/user_namespace_kunit.c | 92 +++++++++++++++++++++++++++++++++++
> 4 files changed, 109 insertions(+)
> create mode 100644 kernel/.kunitconfig
> create mode 100644 kernel/user_namespace_kunit.c
>
>diff --git a/init/Kconfig b/init/Kconfig
>index f63bf5e05e79..d460344539a5 100644
>--- a/init/Kconfig
>+++ b/init/Kconfig
>@@ -1457,6 +1457,16 @@ config USER_NS
> 
> 	  If unsure, say N.
> 
>+config USER_NAMESPACE_KUNIT_TEST
>+	bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
>+	depends on KUNIT=y
>+	default KUNIT_ALL_TESTS
>+	help
>+	  This builds the KUnit test for user namespace uid/gid map insertion.
>+	  It validates map insertion, limits, dynamic allocation of the
>+	  extended extents array, and mapping sorting functions.
>+	  If unsure, say N.
>+
> config PID_NS
> 	bool "PID Namespaces"
> 	default y
>diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
>new file mode 100644
>index 000000000000..7314dce05dc2
>--- /dev/null
>+++ b/kernel/.kunitconfig
>@@ -0,0 +1,3 @@
>+CONFIG_KUNIT=y
>+CONFIG_USER_NS=y
>+CONFIG_USER_NAMESPACE_KUNIT_TEST=y
>diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
>index 786dbf0506ca..0e7373085af9 100644
>--- a/kernel/user_namespace.c
>+++ b/kernel/user_namespace.c
>@@ -1417,3 +1417,7 @@ static __init int user_namespaces_init(void)
> 	return 0;
> }
> subsys_initcall(user_namespaces_init);
>+
>+#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
>+#include "user_namespace_kunit.c"
>+#endif
>diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
>new file mode 100644
>index 000000000000..88467361efdf
>--- /dev/null
>+++ b/kernel/user_namespace_kunit.c
>@@ -0,0 +1,92 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/*
>+ * KUnit test for user namespace map insertion and sorting.
>+ */
>+
>+#include <kunit/test.h>
>+#include <linux/user_namespace.h>
>+
>+static void test_user_ns_map_insert_base(struct kunit *test)
>+{
>+	struct uid_gid_map map;
>+	struct uid_gid_extent extent;
>+	int i, ret;
>+
>+	memset(&map, 0, sizeof(map));
>+
>+	/* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
>+	for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
>+		extent.first = i * 10;
>+		extent.lower_first = i * 100;
>+		extent.count = 5;
>+
>+		ret = insert_extent(&map, &extent);
>+		KUNIT_ASSERT_EQ(test, ret, 0);
>+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
>+		KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
>+		KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
>+		KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
>+	}
>+}
>+
>+static void test_user_ns_map_insert_extended(struct kunit *test)
>+{
>+	struct uid_gid_map map;
>+	struct uid_gid_extent extent;
>+	int i, ret;
>+
>+	memset(&map, 0, sizeof(map));
>+
>+	/* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
>+	for (i = 0; i < 10; i++) {
>+		int value = 9 - i;
>+
>+		extent.first = value * 10;
>+		extent.lower_first = value * 100;
>+		extent.count = 5;
>+
>+		ret = insert_extent(&map, &extent);
>+		KUNIT_ASSERT_EQ(test, ret, 0);
>+		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
>+
>+		if (i < UID_GID_MAP_MAX_BASE_EXTENTS) {
>+			KUNIT_EXPECT_EQ(test, map.extent[i].first, value * 10);
>+		} else {
>+			KUNIT_EXPECT_EQ(test, map.forward[i].first, value * 10);
>+			KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, value  * 100);
>+			KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
>+		}
>+	}
>+
>+	/* Now sort the map to set up reverse mapping */
>+	ret = sort_idmaps(&map);
>+	KUNIT_EXPECT_EQ(test, ret, 0);
>+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.reverse);
>+
>+	/* Verify sorting is correct */
>+	for (i = 0; i < map.nr_extents; i++) {
>+		KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
>+		KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
>+		KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
>+
>+		KUNIT_EXPECT_EQ(test, map.reverse[i].first, i * 10);
>+		KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);
>+		KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
>+	}
>+
>+	kfree(map.forward);
>+	kfree(map.reverse);
>+}
>+
>+static struct kunit_case user_ns_map_test_cases[] = {
>+	KUNIT_CASE(test_user_ns_map_insert_base),
>+	KUNIT_CASE(test_user_ns_map_insert_extended),
>+	{}
>+};
>+
>+static struct kunit_suite user_ns_map_test_suite = {
>+	.name = "user_ns_map",
>+	.test_cases = user_ns_map_test_cases,
>+};
>+
>+kunit_test_suite(user_ns_map_test_suite);
>

--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

Re: [PATCH 2/2] userns: Add KUnit test suite for uid_gid_map

[Thomas Weißschuh]

On Wed, Aug 26, 2026 at 01:40:03PM -0700, Bill Wendling wrote:
> On Sun, Aug 23, 2026 at 11:13 PM Thomas Weißschuh
> <thomas.weissschuh@linutronix.de> wrote:
> >
> > On Sun, Aug 23, 2026 at 12:51:48PM +0000, Bill Wendling wrote:
> > > Add a KUnit test suite to verify the insertion and sorting of mappings
> > > in struct uid_gid_map. This test suite validates both base extent
> > > insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
> > > which triggers the allocation of the forward and reverse pointers).
> > >
> > > This is especially useful for verifying that the __counted_by_ptr
> > > attribute added to 'forward' and 'reverse' pointers works correctly
> > > without causing any runtime bounds-checking panics or traps.
> >
> > AFAIU patch 1 is supposed to not change any behavior.
> > You could move the unit test to the front to make that clearer
> > and also validate it.
> >
> Patch 1 generates code to assert if a memory address outside of the
> bounds of a pointer is accessed. So it does change the behavior.

It should not change the behavior observed by a well-behaved caller.
And the test only simulates such a well-behaved caller.

> Should I still move the test case?

I think so. At least if you resend the series anyways.
Make sure to also run the test before your other patch is applied.

> 
> > > Assisted-by: Gemini Next
> > > Change-Id: If0c2c197a35cd7429cf0d2d6e3b33f0d9f0be66c
> >
> > Change-Id should not be used upstream.
> > See Documentation/dev-tools/checkpatch.rst.
> >
> I had removed it in v2. :-)

Thanks!

(...)

Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Thomas Weißschuh]

On Wed, Aug 26, 2026 at 10:00:35PM +0000, Bill Wendling wrote:
> Add a KUnit test suite to verify the insertion and sorting of mappings
> in struct uid_gid_map. This test suite validates both base extent
> insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
> which triggers the allocation of the forward and reverse pointers).
> 
> This is especially useful for verifying that the __counted_by_ptr
> attribute added to 'forward' and 'reverse' pointers works correctly
> without causing any runtime bounds-checking panics or traps.
> 
> Assisted-by: Gemini:3.1-pro-preview
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> v2 - Remove Gerrit tag.
> v3 - s/KUNIT_EXPECT_NOT_ERR_OR_NULL/KUNIT_ASSERT_NOT_ERR_OR_NULL/
>    - Fixed Kconfig tests.
> v4 - Actually test on unsorted data. Corrected the "Assisted-by" tag.
> ---
> Cc: Bradley Morgan <brads@mainlining.org>
> Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> Cc: Kees Cook <kees@kernel.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Thomas Gleixner <tglx@kernel.org>
> Cc: Nicolas Schier <nsc@kernel.org>
> Cc: Gary Guo <gary@garyguo.net>
> Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Anand Moon <linux.amoon@gmail.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: codemender-patching+linux@google.com
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-hardening@vger.kernel.org
> ---
>  init/Kconfig                  | 10 ++++
>  kernel/.kunitconfig           |  3 ++
>  kernel/user_namespace.c       |  4 ++
>  kernel/user_namespace_kunit.c | 92 +++++++++++++++++++++++++++++++++++
>  4 files changed, 109 insertions(+)
>  create mode 100644 kernel/.kunitconfig
>  create mode 100644 kernel/user_namespace_kunit.c
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index f63bf5e05e79..d460344539a5 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1457,6 +1457,16 @@ config USER_NS
>  
>  	  If unsure, say N.
>  
> +config USER_NAMESPACE_KUNIT_TEST
> +	bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
> +	depends on KUNIT=y

Urgh.

> +	default KUNIT_ALL_TESTS
> +	help
> +	  This builds the KUnit test for user namespace uid/gid map insertion.
> +	  It validates map insertion, limits, dynamic allocation of the
> +	  extended extents array, and mapping sorting functions.
> +	  If unsure, say N.
> +
>  config PID_NS
>  	bool "PID Namespaces"
>  	default y
> diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
> new file mode 100644
> index 000000000000..7314dce05dc2
> --- /dev/null
> +++ b/kernel/.kunitconfig
> @@ -0,0 +1,3 @@
> +CONFIG_KUNIT=y
> +CONFIG_USER_NS=y
> +CONFIG_USER_NAMESPACE_KUNIT_TEST=y
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 786dbf0506ca..0e7373085af9 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -1417,3 +1417,7 @@ static __init int user_namespaces_init(void)
>  	return 0;
>  }
>  subsys_initcall(user_namespaces_init);
> +
> +#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
> +#include "user_namespace_kunit.c"
> +#endif
> diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
> new file mode 100644
> index 000000000000..88467361efdf
> --- /dev/null
> +++ b/kernel/user_namespace_kunit.c
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for user namespace map insertion and sorting.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/user_namespace.h>
> +
> +static void test_user_ns_map_insert_base(struct kunit *test)
> +{
> +	struct uid_gid_map map;
> +	struct uid_gid_extent extent;
> +	int i, ret;
> +
> +	memset(&map, 0, sizeof(map));
> +
> +	/* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
> +	for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
> +		extent.first = i * 10;
> +		extent.lower_first = i * 100;
> +		extent.count = 5;
> +
> +		ret = insert_extent(&map, &extent);
> +		KUNIT_ASSERT_EQ(test, ret, 0);
> +		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
> +		KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
> +		KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
> +		KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
> +	}
> +}

The extended test below already tests everything the 'base' one does.
Do we need both?

> +
> +static void test_user_ns_map_insert_extended(struct kunit *test)
> +{
> +	struct uid_gid_map map;
> +	struct uid_gid_extent extent;
> +	int i, ret;
> +
> +	memset(&map, 0, sizeof(map));
> +
> +	/* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */

When UID_GID_MAP_MAX_BASE_EXTENTS is ever increased, this might not be true anymore.
Add an assertion or make the iteration count dynamic.

> +	for (i = 0; i < 10; i++) {
> +		int value = 9 - i;
> +
> +		extent.first = value * 10;
> +		extent.lower_first = value * 100;
> +		extent.count = 5;
> +
> +		ret = insert_extent(&map, &extent);
> +		KUNIT_ASSERT_EQ(test, ret, 0);
> +		KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
> +
> +		if (i < UID_GID_MAP_MAX_BASE_EXTENTS) {
> +			KUNIT_EXPECT_EQ(test, map.extent[i].first, value * 10);
> +		} else {
> +			KUNIT_EXPECT_EQ(test, map.forward[i].first, value * 10);
> +			KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, value  * 100);
> +			KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> +		}
> +	}
> +
> +	/* Now sort the map to set up reverse mapping */
> +	ret = sort_idmaps(&map);
> +	KUNIT_EXPECT_EQ(test, ret, 0);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.reverse);
> +
> +	/* Verify sorting is correct */
> +	for (i = 0; i < map.nr_extents; i++) {
> +		KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
> +		KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
> +		KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> +
> +		KUNIT_EXPECT_EQ(test, map.reverse[i].first, i * 10);
> +		KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);
> +		KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
> +	}

Isn't this the same as the original order?

> +
> +	kfree(map.forward);
> +	kfree(map.reverse);
> +}

(...)

Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

On Thu, Aug 27, 2026 at 6:37 AM Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> On Wed, Aug 26, 2026 at 10:00:35PM +0000, Bill Wendling wrote:
> > Add a KUnit test suite to verify the insertion and sorting of mappings
> > in struct uid_gid_map. This test suite validates both base extent
> > insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
> > which triggers the allocation of the forward and reverse pointers).
> >
> > This is especially useful for verifying that the __counted_by_ptr
> > attribute added to 'forward' and 'reverse' pointers works correctly
> > without causing any runtime bounds-checking panics or traps.
> >
> > Assisted-by: Gemini:3.1-pro-preview
> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> > v2 - Remove Gerrit tag.
> > v3 - s/KUNIT_EXPECT_NOT_ERR_OR_NULL/KUNIT_ASSERT_NOT_ERR_OR_NULL/
> >    - Fixed Kconfig tests.
> > v4 - Actually test on unsorted data. Corrected the "Assisted-by" tag.
> > ---
> > Cc: Bradley Morgan <brads@mainlining.org>
> > Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > Cc: Kees Cook <kees@kernel.org>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Christian Brauner <brauner@kernel.org>
> > Cc: Aleksa Sarai <cyphar@cyphar.com>
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Cc: Thomas Gleixner <tglx@kernel.org>
> > Cc: Nicolas Schier <nsc@kernel.org>
> > Cc: Gary Guo <gary@garyguo.net>
> > Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
> > Cc: Alice Ryhl <aliceryhl@google.com>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Cc: Anand Moon <linux.amoon@gmail.com>
> > Cc: Oleg Nesterov <oleg@redhat.com>
> > Cc: codemender-patching+linux@google.com
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-hardening@vger.kernel.org
> > ---
> >  init/Kconfig                  | 10 ++++
> >  kernel/.kunitconfig           |  3 ++
> >  kernel/user_namespace.c       |  4 ++
> >  kernel/user_namespace_kunit.c | 92 +++++++++++++++++++++++++++++++++++
> >  4 files changed, 109 insertions(+)
> >  create mode 100644 kernel/.kunitconfig
> >  create mode 100644 kernel/user_namespace_kunit.c
> >
> > diff --git a/init/Kconfig b/init/Kconfig
> > index f63bf5e05e79..d460344539a5 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1457,6 +1457,16 @@ config USER_NS
> >
> >         If unsure, say N.
> >
> > +config USER_NAMESPACE_KUNIT_TEST
> > +     bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
> > +     depends on KUNIT=y
>
> Urgh.
>
?? What's wrong? It's identical to the conditional for EXEC_KUNIT_TEST:

config EXEC_KUNIT_TEST
     bool "Build execve tests" if !KUNIT_ALL_TESTS
     depends on KUNIT=y
     default KUNIT_ALL_TESTS
     help
          This builds the exec KUnit tests, which tests boundary conditions
          of various aspects of the exec internals.

> > +     default KUNIT_ALL_TESTS
> > +     help
> > +       This builds the KUnit test for user namespace uid/gid map insertion.
> > +       It validates map insertion, limits, dynamic allocation of the
> > +       extended extents array, and mapping sorting functions.
> > +       If unsure, say N.
> > +
> >  config PID_NS
> >       bool "PID Namespaces"
> >       default y
> > diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
> > new file mode 100644
> > index 000000000000..7314dce05dc2
> > --- /dev/null
> > +++ b/kernel/.kunitconfig
> > @@ -0,0 +1,3 @@
> > +CONFIG_KUNIT=y
> > +CONFIG_USER_NS=y
> > +CONFIG_USER_NAMESPACE_KUNIT_TEST=y
> > diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> > index 786dbf0506ca..0e7373085af9 100644
> > --- a/kernel/user_namespace.c
> > +++ b/kernel/user_namespace.c
> > @@ -1417,3 +1417,7 @@ static __init int user_namespaces_init(void)
> >       return 0;
> >  }
> >  subsys_initcall(user_namespaces_init);
> > +
> > +#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
> > +#include "user_namespace_kunit.c"
> > +#endif
> > diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
> > new file mode 100644
> > index 000000000000..88467361efdf
> > --- /dev/null
> > +++ b/kernel/user_namespace_kunit.c
> > @@ -0,0 +1,92 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * KUnit test for user namespace map insertion and sorting.
> > + */
> > +
> > +#include <kunit/test.h>
> > +#include <linux/user_namespace.h>
> > +
> > +static void test_user_ns_map_insert_base(struct kunit *test)
> > +{
> > +     struct uid_gid_map map;
> > +     struct uid_gid_extent extent;
> > +     int i, ret;
> > +
> > +     memset(&map, 0, sizeof(map));
> > +
> > +     /* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
> > +     for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
> > +             extent.first = i * 10;
> > +             extent.lower_first = i * 100;
> > +             extent.count = 5;
> > +
> > +             ret = insert_extent(&map, &extent);
> > +             KUNIT_ASSERT_EQ(test, ret, 0);
> > +             KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
> > +             KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
> > +             KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
> > +             KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
> > +     }
> > +}
>
> The extended test below already tests everything the 'base' one does.
> Do we need both?
>
The one below tests the sorting algorithm.

> > +
> > +static void test_user_ns_map_insert_extended(struct kunit *test)
> > +{
> > +     struct uid_gid_map map;
> > +     struct uid_gid_extent extent;
> > +     int i, ret;
> > +
> > +     memset(&map, 0, sizeof(map));
> > +
> > +     /* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
>
> When UID_GID_MAP_MAX_BASE_EXTENTS is ever increased, this might not be true anymore.
> Add an assertion or make the iteration count dynamic.
>
Sure, I can make it something like "UID_GID_MAP_MAX_BASE_EXTENTS + 42"
or something. Let me know about the "Urgh" comment above first.

> > +     for (i = 0; i < 10; i++) {
> > +             int value = 9 - i;
> > +
> > +             extent.first = value * 10;
> > +             extent.lower_first = value * 100;
> > +             extent.count = 5;
> > +
> > +             ret = insert_extent(&map, &extent);
> > +             KUNIT_ASSERT_EQ(test, ret, 0);
> > +             KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
> > +
> > +             if (i < UID_GID_MAP_MAX_BASE_EXTENTS) {
> > +                     KUNIT_EXPECT_EQ(test, map.extent[i].first, value * 10);
> > +             } else {
> > +                     KUNIT_EXPECT_EQ(test, map.forward[i].first, value * 10);
> > +                     KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, value  * 100);
> > +                     KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> > +             }
> > +     }
> > +
> > +     /* Now sort the map to set up reverse mapping */
> > +     ret = sort_idmaps(&map);
> > +     KUNIT_EXPECT_EQ(test, ret, 0);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.reverse);
> > +
> > +     /* Verify sorting is correct */
> > +     for (i = 0; i < map.nr_extents; i++) {
> > +             KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
> > +             KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
> > +             KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> > +
> > +             KUNIT_EXPECT_EQ(test, map.reverse[i].first, i * 10);
> > +             KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);
> > +             KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
> > +     }
>
> Isn't this the same as the original order?
>
No. The original order uses "9 - i" for the base value, so it's not in
sorted order (though it's not exactly random either).

-bw

> > +
> > +     kfree(map.forward);
> > +     kfree(map.reverse);
> > +}
>
> (...)

Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Thomas Weißschuh]

Hi Bill,

On Thu, Aug 27, 2026 at 12:27:30PM -0700, Bill Wendling wrote:
> On Thu, Aug 27, 2026 at 6:37 AM Thomas Weißschuh
> <thomas.weissschuh@linutronix.de> wrote:

(...)

> > > +config USER_NAMESPACE_KUNIT_TEST
> > > +     bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
> > > +     depends on KUNIT=y
> >
> > Urgh.
> >
> ?? What's wrong? It's identical to the conditional for EXEC_KUNIT_TEST:

Sorry for this non-descript review comment.

> config EXEC_KUNIT_TEST
>      bool "Build execve tests" if !KUNIT_ALL_TESTS
>      depends on KUNIT=y
>      default KUNIT_ALL_TESTS
>      help
>           This builds the exec KUnit tests, which tests boundary conditions
>           of various aspects of the exec internals.

The problem is that KUNIT can be built as module, which would prevent this
test from being built. We have include/kunit/visibility.h to export certain
symbols only to tests and avoid this issue.
But I can see that some maintaines don't like this pattern, so maybe they can
chime in at some point.

> > > +     default KUNIT_ALL_TESTS
> > > +     help
> > > +       This builds the KUnit test for user namespace uid/gid map insertion.
> > > +       It validates map insertion, limits, dynamic allocation of the
> > > +       extended extents array, and mapping sorting functions.
> > > +       If unsure, say N.
> > > +
> > >  config PID_NS
> > >       bool "PID Namespaces"
> > >       default y
> > > diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
> > > new file mode 100644
> > > index 000000000000..7314dce05dc2
> > > --- /dev/null
> > > +++ b/kernel/.kunitconfig
> > > @@ -0,0 +1,3 @@
> > > +CONFIG_KUNIT=y
> > > +CONFIG_USER_NS=y
> > > +CONFIG_USER_NAMESPACE_KUNIT_TEST=y
> > > diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> > > index 786dbf0506ca..0e7373085af9 100644
> > > --- a/kernel/user_namespace.c
> > > +++ b/kernel/user_namespace.c
> > > @@ -1417,3 +1417,7 @@ static __init int user_namespaces_init(void)
> > >       return 0;
> > >  }
> > >  subsys_initcall(user_namespaces_init);
> > > +
> > > +#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
> > > +#include "user_namespace_kunit.c"
> > > +#endif
> > > diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
> > > new file mode 100644
> > > index 000000000000..88467361efdf
> > > --- /dev/null
> > > +++ b/kernel/user_namespace_kunit.c
> > > @@ -0,0 +1,92 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * KUnit test for user namespace map insertion and sorting.
> > > + */
> > > +
> > > +#include <kunit/test.h>
> > > +#include <linux/user_namespace.h>
> > > +
> > > +static void test_user_ns_map_insert_base(struct kunit *test)
> > > +{
> > > +     struct uid_gid_map map;
> > > +     struct uid_gid_extent extent;
> > > +     int i, ret;
> > > +
> > > +     memset(&map, 0, sizeof(map));
> > > +
> > > +     /* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
> > > +     for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
> > > +             extent.first = i * 10;
> > > +             extent.lower_first = i * 100;
> > > +             extent.count = 5;
> > > +
> > > +             ret = insert_extent(&map, &extent);
> > > +             KUNIT_ASSERT_EQ(test, ret, 0);
> > > +             KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
> > > +             KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
> > > +             KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
> > > +             KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
> > > +     }
> > > +}
> >
> > The extended test below already tests everything the 'base' one does.
> > Do we need both?
> >
> The one below tests the sorting algorithm.

It *also* tests the insertion, no?
(Especially if the conditional on UID_GID_MAP_MAX_BASE_EXTENTS is removed)

> > > +
> > > +static void test_user_ns_map_insert_extended(struct kunit *test)
> > > +{
> > > +     struct uid_gid_map map;
> > > +     struct uid_gid_extent extent;
> > > +     int i, ret;
> > > +
> > > +     memset(&map, 0, sizeof(map));
> > > +
> > > +     /* Insert more than UID_GID_MAP_MAX_BASE_EXTENTS (e.g., 10) elements */
> >
> > When UID_GID_MAP_MAX_BASE_EXTENTS is ever increased, this might not be true anymore.
> > Add an assertion or make the iteration count dynamic.
> >
> Sure, I can make it something like "UID_GID_MAP_MAX_BASE_EXTENTS + 42"
> or something.

That sounds good.

> > > +     for (i = 0; i < 10; i++) {
> > > +             int value = 9 - i;
> > > +
> > > +             extent.first = value * 10;
> > > +             extent.lower_first = value * 100;
> > > +             extent.count = 5;
> > > +
> > > +             ret = insert_extent(&map, &extent);
> > > +             KUNIT_ASSERT_EQ(test, ret, 0);
> > > +             KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
> > > +
> > > +             if (i < UID_GID_MAP_MAX_BASE_EXTENTS) {
> > > +                     KUNIT_EXPECT_EQ(test, map.extent[i].first, value * 10);
> > > +             } else {
> > > +                     KUNIT_EXPECT_EQ(test, map.forward[i].first, value * 10);
> > > +                     KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, value  * 100);
> > > +                     KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> > > +             }
> > > +     }
> > > +
> > > +     /* Now sort the map to set up reverse mapping */
> > > +     ret = sort_idmaps(&map);
> > > +     KUNIT_EXPECT_EQ(test, ret, 0);
> > > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.reverse);
> > > +
> > > +     /* Verify sorting is correct */
> > > +     for (i = 0; i < map.nr_extents; i++) {
> > > +             KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
> > > +             KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
> > > +             KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> > > +
> > > +             KUNIT_EXPECT_EQ(test, map.reverse[i].first, i * 10);
> > > +             KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);
> > > +             KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
> > > +     }
> >
> > Isn't this the same as the original order?
> >
> No. The original order uses "9 - i" for the base value, so it's not in
> sorted order (though it's not exactly random either).

Indeed, sorry for missing this.
If you send a new revision, maybe add a small comment.


Thomas

Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

Hi Thomas,

On Mon, Aug 31, 2026 at 2:22 AM Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> Hi Bill,
>
> On Thu, Aug 27, 2026 at 12:27:30PM -0700, Bill Wendling wrote:
> > On Thu, Aug 27, 2026 at 6:37 AM Thomas Weißschuh
> > <thomas.weissschuh@linutronix.de> wrote:
>
> (...)
>
> > > > +config USER_NAMESPACE_KUNIT_TEST
> > > > +     bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
> > > > +     depends on KUNIT=y
> > >
> > > Urgh.
> > >
> > ?? What's wrong? It's identical to the conditional for EXEC_KUNIT_TEST:
>
> Sorry for this non-descript review comment.
>
> > config EXEC_KUNIT_TEST
> >      bool "Build execve tests" if !KUNIT_ALL_TESTS
> >      depends on KUNIT=y
> >      default KUNIT_ALL_TESTS
> >      help
> >           This builds the exec KUnit tests, which tests boundary conditions
> >           of various aspects of the exec internals.
>
> The problem is that KUNIT can be built as module, which would prevent this
> test from being built. We have include/kunit/visibility.h to export certain
> symbols only to tests and avoid this issue.
> But I can see that some maintaines don't like this pattern, so maybe they can
> chime in at some point.

Bradley commented on this earlier (which is why I mentioned EXEC_KUNIT_TEST):

<comment>
The test is #include'd into user_namespace.c, which is builtin (USER_NS
is a bool), so =m here still compiles the suite into vmlinux. With
KUNIT=m that calls kunit symbols that live in a module, and the link
fails. Make it bool and depend on KUNIT=y, like EXEC_KUNIT_TEST:

 bool "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
 depends on USER_NS && KUNIT=y
</comment>

So there's a conflict and, because I'm not a KUnit guru, I'm not sure
which way is "best".

> > > > +     default KUNIT_ALL_TESTS
> > > > +     help
> > > > +       This builds the KUnit test for user namespace uid/gid map insertion.
> > > > +       It validates map insertion, limits, dynamic allocation of the
> > > > +       extended extents array, and mapping sorting functions.
> > > > +       If unsure, say N.
> > > > +
> > > >  config PID_NS
> > > >       bool "PID Namespaces"
> > > >       default y
> > > > diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
> > > > new file mode 100644
> > > > index 000000000000..7314dce05dc2
> > > > --- /dev/null
> > > > +++ b/kernel/.kunitconfig
> > > > @@ -0,0 +1,3 @@
> > > > +CONFIG_KUNIT=y
> > > > +CONFIG_USER_NS=y
> > > > +CONFIG_USER_NAMESPACE_KUNIT_TEST=y
> > > > diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> > > > index 786dbf0506ca..0e7373085af9 100644
> > > > --- a/kernel/user_namespace.c
> > > > +++ b/kernel/user_namespace.c
> > > > @@ -1417,3 +1417,7 @@ static __init int user_namespaces_init(void)
> > > >       return 0;
> > > >  }
> > > >  subsys_initcall(user_namespaces_init);
> > > > +
> > > > +#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
> > > > +#include "user_namespace_kunit.c"
> > > > +#endif
> > > > diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
> > > > new file mode 100644
> > > > index 000000000000..88467361efdf
> > > > --- /dev/null
> > > > +++ b/kernel/user_namespace_kunit.c
> > > > @@ -0,0 +1,92 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * KUnit test for user namespace map insertion and sorting.
> > > > + */
> > > > +
> > > > +#include <kunit/test.h>
> > > > +#include <linux/user_namespace.h>
> > > > +
> > > > +static void test_user_ns_map_insert_base(struct kunit *test)
> > > > +{
> > > > +     struct uid_gid_map map;
> > > > +     struct uid_gid_extent extent;
> > > > +     int i, ret;
> > > > +
> > > > +     memset(&map, 0, sizeof(map));
> > > > +
> > > > +     /* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
> > > > +     for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
> > > > +             extent.first = i * 10;
> > > > +             extent.lower_first = i * 100;
> > > > +             extent.count = 5;
> > > > +
> > > > +             ret = insert_extent(&map, &extent);
> > > > +             KUNIT_ASSERT_EQ(test, ret, 0);
> > > > +             KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
> > > > +             KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
> > > > +             KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
> > > > +             KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
> > > > +     }
> > > > +}
> > >
> > > The extended test below already tests everything the 'base' one does.
> > > Do we need both?
> > >
> > The one below tests the sorting algorithm.
>
> It *also* tests the insertion, no?
> (Especially if the conditional on UID_GID_MAP_MAX_BASE_EXTENTS is removed)
>
Correct. So there are three types of tests we should run here:

1. Insertions and accesses that don't go over the initial extents size.
2. Insertions and accesses that do go over the initial extents size.
3. Accesses outside of the number of entries.

Test (1) is a "smoke" test, where the struct is tested and no
sanitizer code is used. Test (2) makes sure that we can still go over
the UID_GID_MAP_MAX_BASE_EXTENTS size and the sanitizer won't
activate. Test (3) (which I'll add in my next upload) throws a
sanitizer exception.

> > > > +     for (i = 0; i < 10; i++) {
> > > > +             int value = 9 - i;
> > > > +
> > > > +             extent.first = value * 10;
> > > > +             extent.lower_first = value * 100;
> > > > +             extent.count = 5;
> > > > +
> > > > +             ret = insert_extent(&map, &extent);
> > > > +             KUNIT_ASSERT_EQ(test, ret, 0);
> > > > +             KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
> > > > +
> > > > +             if (i < UID_GID_MAP_MAX_BASE_EXTENTS) {
> > > > +                     KUNIT_EXPECT_EQ(test, map.extent[i].first, value * 10);
> > > > +             } else {
> > > > +                     KUNIT_EXPECT_EQ(test, map.forward[i].first, value * 10);
> > > > +                     KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, value  * 100);
> > > > +                     KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> > > > +             }
> > > > +     }
> > > > +
> > > > +     /* Now sort the map to set up reverse mapping */
> > > > +     ret = sort_idmaps(&map);
> > > > +     KUNIT_EXPECT_EQ(test, ret, 0);
> > > > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, map.reverse);
> > > > +
> > > > +     /* Verify sorting is correct */
> > > > +     for (i = 0; i < map.nr_extents; i++) {
> > > > +             KUNIT_EXPECT_EQ(test, map.forward[i].first, i * 10);
> > > > +             KUNIT_EXPECT_EQ(test, map.forward[i].lower_first, i * 100);
> > > > +             KUNIT_EXPECT_EQ(test, map.forward[i].count, 5);
> > > > +
> > > > +             KUNIT_EXPECT_EQ(test, map.reverse[i].first, i * 10);
> > > > +             KUNIT_EXPECT_EQ(test, map.reverse[i].lower_first, i * 100);
> > > > +             KUNIT_EXPECT_EQ(test, map.reverse[i].count, 5);
> > > > +     }
> > >
> > > Isn't this the same as the original order?
> > >
> > No. The original order uses "9 - i" for the base value, so it's not in
> > sorted order (though it's not exactly random either).
>
> Indeed, sorry for missing this.
> If you send a new revision, maybe add a small comment.
>
Ack.

-bw

Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Thomas Weißschuh]

On Thu, Sep 03, 2026 at 01:21:27PM -0700, Bill Wendling wrote:
> On Mon, Aug 31, 2026 at 2:22 AM Thomas Weißschuh
> <thomas.weissschuh@linutronix.de> wrote:
> > On Thu, Aug 27, 2026 at 12:27:30PM -0700, Bill Wendling wrote:
> > > On Thu, Aug 27, 2026 at 6:37 AM Thomas Weißschuh
> > > <thomas.weissschuh@linutronix.de> wrote:
> >
> > (...)
> >
> > > > > +config USER_NAMESPACE_KUNIT_TEST
> > > > > +     bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
> > > > > +     depends on KUNIT=y
> > > >
> > > > Urgh.
> > > >
> > > ?? What's wrong? It's identical to the conditional for EXEC_KUNIT_TEST:
> >
> > Sorry for this non-descript review comment.
> >
> > > config EXEC_KUNIT_TEST
> > >      bool "Build execve tests" if !KUNIT_ALL_TESTS
> > >      depends on KUNIT=y
> > >      default KUNIT_ALL_TESTS
> > >      help
> > >           This builds the exec KUnit tests, which tests boundary conditions
> > >           of various aspects of the exec internals.
> >
> > The problem is that KUNIT can be built as module, which would prevent this
> > test from being built. We have include/kunit/visibility.h to export certain
> > symbols only to tests and avoid this issue.
> > But I can see that some maintaines don't like this pattern, so maybe they can
> > chime in at some point.
> 
> Bradley commented on this earlier (which is why I mentioned EXEC_KUNIT_TEST):
> 
> <comment>
> The test is #include'd into user_namespace.c, which is builtin (USER_NS
> is a bool), so =m here still compiles the suite into vmlinux. With
> KUNIT=m that calls kunit symbols that live in a module, and the link
> fails. Make it bool and depend on KUNIT=y, like EXEC_KUNIT_TEST:
> 
>  bool "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
>  depends on USER_NS && KUNIT=y
> </comment>
> 
> So there's a conflict and, because I'm not a KUnit guru, I'm not sure
> which way is "best".

It's subjective. So as mentioned before, the preference of the maintainers
should go into it. The aproach I prefer requires a bit more setup boilerplate
but make the tests usable in more circumstances.

> > > > > +     default KUNIT_ALL_TESTS
> > > > > +     help
> > > > > +       This builds the KUnit test for user namespace uid/gid map insertion.
> > > > > +       It validates map insertion, limits, dynamic allocation of the
> > > > > +       extended extents array, and mapping sorting functions.
> > > > > +       If unsure, say N.
> > > > > +
> > > > >  config PID_NS
> > > > >       bool "PID Namespaces"
> > > > >       default y
> > > > > diff --git a/kernel/.kunitconfig b/kernel/.kunitconfig
> > > > > new file mode 100644
> > > > > index 000000000000..7314dce05dc2
> > > > > --- /dev/null
> > > > > +++ b/kernel/.kunitconfig
> > > > > @@ -0,0 +1,3 @@
> > > > > +CONFIG_KUNIT=y
> > > > > +CONFIG_USER_NS=y
> > > > > +CONFIG_USER_NAMESPACE_KUNIT_TEST=y
> > > > > diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> > > > > index 786dbf0506ca..0e7373085af9 100644
> > > > > --- a/kernel/user_namespace.c
> > > > > +++ b/kernel/user_namespace.c
> > > > > @@ -1417,3 +1417,7 @@ static __init int user_namespaces_init(void)
> > > > >       return 0;
> > > > >  }
> > > > >  subsys_initcall(user_namespaces_init);
> > > > > +
> > > > > +#if IS_ENABLED(CONFIG_USER_NAMESPACE_KUNIT_TEST)
> > > > > +#include "user_namespace_kunit.c"
> > > > > +#endif
> > > > > diff --git a/kernel/user_namespace_kunit.c b/kernel/user_namespace_kunit.c
> > > > > new file mode 100644
> > > > > index 000000000000..88467361efdf
> > > > > --- /dev/null
> > > > > +++ b/kernel/user_namespace_kunit.c
> > > > > @@ -0,0 +1,92 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * KUnit test for user namespace map insertion and sorting.
> > > > > + */
> > > > > +
> > > > > +#include <kunit/test.h>
> > > > > +#include <linux/user_namespace.h>
> > > > > +
> > > > > +static void test_user_ns_map_insert_base(struct kunit *test)
> > > > > +{
> > > > > +     struct uid_gid_map map;
> > > > > +     struct uid_gid_extent extent;
> > > > > +     int i, ret;
> > > > > +
> > > > > +     memset(&map, 0, sizeof(map));
> > > > > +
> > > > > +     /* Insert up to UID_GID_MAP_MAX_BASE_EXTENTS (5) elements */
> > > > > +     for (i = 0; i < UID_GID_MAP_MAX_BASE_EXTENTS; i++) {
> > > > > +             extent.first = i * 10;
> > > > > +             extent.lower_first = i * 100;
> > > > > +             extent.count = 5;
> > > > > +
> > > > > +             ret = insert_extent(&map, &extent);
> > > > > +             KUNIT_ASSERT_EQ(test, ret, 0);
> > > > > +             KUNIT_EXPECT_EQ(test, map.nr_extents, i + 1);
> > > > > +             KUNIT_EXPECT_EQ(test, map.extent[i].first, i * 10);
> > > > > +             KUNIT_EXPECT_EQ(test, map.extent[i].lower_first, i * 100);
> > > > > +             KUNIT_EXPECT_EQ(test, map.extent[i].count, 5);
> > > > > +     }
> > > > > +}
> > > >
> > > > The extended test below already tests everything the 'base' one does.
> > > > Do we need both?
> > > >
> > > The one below tests the sorting algorithm.
> >
> > It *also* tests the insertion, no?
> > (Especially if the conditional on UID_GID_MAP_MAX_BASE_EXTENTS is removed)
> >
> Correct. So there are three types of tests we should run here:
> 
> 1. Insertions and accesses that don't go over the initial extents size.
> 2. Insertions and accesses that do go over the initial extents size.
> 3. Accesses outside of the number of entries.
> 
> Test (1) is a "smoke" test, where the struct is tested and no
> sanitizer code is used.

A "smoke" test is useful when the more complete tests can not be run regularly.
But here both test cases will always run right after each other. Test (2) is
just as cheap as this one.

*Not* testing the overflow checking here sounds also weird. Test (2) will
excercise the same code, which is not using the checking, anyways.

> Test (2) makes sure that we can still go over
> the UID_GID_MAP_MAX_BASE_EXTENTS size and the sanitizer won't
> activate.

Nice.

> Test (3) (which I'll add in my next upload) throws a sanitizer exception.

What is the point of testing this specifically for user namespaces?
Normally we expect a used subsystem to work as advertised.
It is that used subsystem's responsibility to test that it does so.
If there is currently no test that validates __counted_by then it surely
should be created. But not here.


Thomas

Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Bill Wendling]

On Fri, Sep 4, 2026 at 1:02 AM Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> On Thu, Sep 03, 2026 at 01:21:27PM -0700, Bill Wendling wrote:
> > On Mon, Aug 31, 2026 at 2:22 AM Thomas Weißschuh
> > <thomas.weissschuh@linutronix.de> wrote:
> > > On Thu, Aug 27, 2026 at 12:27:30PM -0700, Bill Wendling wrote:
> > > > On Thu, Aug 27, 2026 at 6:37 AM Thomas Weißschuh
> > > > <thomas.weissschuh@linutronix.de> wrote:
> > >
> > > (...)
> > >
> > > > > > +config USER_NAMESPACE_KUNIT_TEST
> > > > > > +     bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
> > > > > > +     depends on KUNIT=y
> > > > >
> > > > > Urgh.
> > > > >
> > > > ?? What's wrong? It's identical to the conditional for EXEC_KUNIT_TEST:
> > >
> > > Sorry for this non-descript review comment.
> > >
> > > > config EXEC_KUNIT_TEST
> > > >      bool "Build execve tests" if !KUNIT_ALL_TESTS
> > > >      depends on KUNIT=y
> > > >      default KUNIT_ALL_TESTS
> > > >      help
> > > >           This builds the exec KUnit tests, which tests boundary conditions
> > > >           of various aspects of the exec internals.
> > >
> > > The problem is that KUNIT can be built as module, which would prevent this
> > > test from being built. We have include/kunit/visibility.h to export certain
> > > symbols only to tests and avoid this issue.
> > > But I can see that some maintaines don't like this pattern, so maybe they can
> > > chime in at some point.
> >
> > Bradley commented on this earlier (which is why I mentioned EXEC_KUNIT_TEST):
> >
> > <comment>
> > The test is #include'd into user_namespace.c, which is builtin (USER_NS
> > is a bool), so =m here still compiles the suite into vmlinux. With
> > KUNIT=m that calls kunit symbols that live in a module, and the link
> > fails. Make it bool and depend on KUNIT=y, like EXEC_KUNIT_TEST:
> >
> >  bool "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
> >  depends on USER_NS && KUNIT=y
> > </comment>
> >
> > So there's a conflict and, because I'm not a KUnit guru, I'm not sure
> > which way is "best".
>
> It's subjective. So as mentioned before, the preference of the maintainers
> should go into it. The aproach I prefer requires a bit more setup boilerplate
> but make the tests usable in more circumstances.
>
Because user_namespace.c is always built-in (USER_NS is a bool),
compiling the test into vmlinux causes linker failures if
CONFIG_KUNIT=m. Using the "visibility.h" version also strips static
from insert_extent() and sort_idmaps() or exporting internal user
namespace functions into the kernel symbol table, which isn't ideal.

> > > > > The extended test below already tests everything the 'base' one does.
> > > > > Do we need both?
> > > > >
> > > > The one below tests the sorting algorithm.
> > >
> > > It *also* tests the insertion, no?
> > > (Especially if the conditional on UID_GID_MAP_MAX_BASE_EXTENTS is removed)
> > >
> > Correct. So there are three types of tests we should run here:
> >
> > 1. Insertions and accesses that don't go over the initial extents size.
> > 2. Insertions and accesses that do go over the initial extents size.
> > 3. Accesses outside of the number of entries.
> >
> > Test (1) is a "smoke" test, where the struct is tested and no
> > sanitizer code is used.
>
> A "smoke" test is useful when the more complete tests can not be run regularly.
> But here both test cases will always run right after each other. Test (2) is
> just as cheap as this one.
>
> *Not* testing the overflow checking here sounds also weird. Test (2) will
> excercise the same code, which is not using the checking, anyways.
>
I suppose this depends on one's view of how testing should be done. I
prefer to have small test cases which directly test a specific
feature. Other test cases would still run the same code, but they
focus on features. So yes, the same code is ran many times during
testing, but that's fine, because testing is meant to test one feature
at a time. The benefit of this approach is if a test case fails, it's
easier to determine what feature is responsible.

> > Test (2) makes sure that we can still go over
> > the UID_GID_MAP_MAX_BASE_EXTENTS size and the sanitizer won't
> > activate.
>
> Nice.
>
> > Test (3) (which I'll add in my next upload) throws a sanitizer exception.
>
> What is the point of testing this specifically for user namespaces?
> Normally we expect a used subsystem to work as advertised.
> It is that used subsystem's responsibility to test that it does so.
> If there is currently no test that validates __counted_by then it surely
> should be created. But not here.
>
This would directly test that the attribute on the struct field is
caught by UBSAN. I'm not sure how we could more directly test it
otherwise...

-bw

Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Kees Cook]

On Fri, Sep 04, 2026 at 01:37:00PM -0700, Bill Wendling wrote:
> On Fri, Sep 4, 2026 at 1:02 AM Thomas Weißschuh
> <thomas.weissschuh@linutronix.de> wrote:
> >
> > On Thu, Sep 03, 2026 at 01:21:27PM -0700, Bill Wendling wrote:
> > > On Mon, Aug 31, 2026 at 2:22 AM Thomas Weißschuh
> > > <thomas.weissschuh@linutronix.de> wrote:
> > > > On Thu, Aug 27, 2026 at 12:27:30PM -0700, Bill Wendling wrote:
> > > > > On Thu, Aug 27, 2026 at 6:37 AM Thomas Weißschuh
> > > > > <thomas.weissschuh@linutronix.de> wrote:
> > > >
> > > > (...)
> > > >
> > > > > > > +config USER_NAMESPACE_KUNIT_TEST
> > > > > > > +     bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
> > > > > > > +     depends on KUNIT=y
> > > > > >
> > > > > > Urgh.
> > > > > >
> > > > > ?? What's wrong? It's identical to the conditional for EXEC_KUNIT_TEST:
> > > >
> > > > Sorry for this non-descript review comment.
> > > >
> > > > > config EXEC_KUNIT_TEST
> > > > >      bool "Build execve tests" if !KUNIT_ALL_TESTS
> > > > >      depends on KUNIT=y
> > > > >      default KUNIT_ALL_TESTS
> > > > >      help
> > > > >           This builds the exec KUnit tests, which tests boundary conditions
> > > > >           of various aspects of the exec internals.
> > > >
> > > > The problem is that KUNIT can be built as module, which would prevent this
> > > > test from being built. We have include/kunit/visibility.h to export certain
> > > > symbols only to tests and avoid this issue.
> > > > But I can see that some maintaines don't like this pattern, so maybe they can
> > > > chime in at some point.
> > >
> > > Bradley commented on this earlier (which is why I mentioned EXEC_KUNIT_TEST):
> > >
> > > <comment>
> > > The test is #include'd into user_namespace.c, which is builtin (USER_NS
> > > is a bool), so =m here still compiles the suite into vmlinux. With
> > > KUNIT=m that calls kunit symbols that live in a module, and the link
> > > fails. Make it bool and depend on KUNIT=y, like EXEC_KUNIT_TEST:
> > >
> > >  bool "KUnit test for user namespace map insertion" if !KUNIT_ALL_TESTS
> > >  depends on USER_NS && KUNIT=y
> > > </comment>
> > >
> > > So there's a conflict and, because I'm not a KUnit guru, I'm not sure
> > > which way is "best".
> >
> > It's subjective. So as mentioned before, the preference of the maintainers
> > should go into it. The aproach I prefer requires a bit more setup boilerplate
> > but make the tests usable in more circumstances.
> >
> Because user_namespace.c is always built-in (USER_NS is a bool),
> compiling the test into vmlinux causes linker failures if
> CONFIG_KUNIT=m. Using the "visibility.h" version also strips static
> from insert_extent() and sort_idmaps() or exporting internal user
> namespace functions into the kernel symbol table, which isn't ideal.

The export namespacing should limit it to just KUnit, so that should be
okay, I would think? My guide for this in the past has been "did the
kernel get less fast/safe because of Kunit exports, etc?" I don't
_think_ that's the case here.

So, I guess it's down to a matter of taste by USER_NS maintainers? I've
move Christian to "To:" from "Cc:", as I think he is the defacto USER_NS
maintainer? I don't know, though, there is no MAINTAINERS entry for it. 

> > > Test (3) (which I'll add in my next upload) throws a sanitizer exception.
> >
> > What is the point of testing this specifically for user namespaces?
> > Normally we expect a used subsystem to work as advertised.
> > It is that used subsystem's responsibility to test that it does so.
> > If there is currently no test that validates __counted_by then it surely
> > should be created. But not here.
> >
> This would directly test that the attribute on the struct field is
> caught by UBSAN. I'm not sure how we could more directly test it
> otherwise...

It is tricky to catch the traps sanely with KUnit. All the Oops tests
I've written for it are in LKDTM, and only __bdos is tested in
lib/tests/overflow_kunit.c. I was playing with catching Oops in KUnit
for testing KCFI recently, but haven't come back to finishing it safely:
https://lore.kernel.org/lkml/20260618210946.it.538-kees@kernel.org/

-Kees

-- 
Kees Cook

Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map

[Kees Cook]

On Wed, Aug 26, 2026 at 10:00:35PM +0000, Bill Wendling wrote:
> Add a KUnit test suite to verify the insertion and sorting of mappings
> in struct uid_gid_map. This test suite validates both base extent
> insertion (<= 5 mappings) and extended extent insertion (> 5 mappings,
> which triggers the allocation of the forward and reverse pointers).
> 
> This is especially useful for verifying that the __counted_by_ptr
> attribute added to 'forward' and 'reverse' pointers works correctly
> without causing any runtime bounds-checking panics or traps.
> 
> Assisted-by: Gemini:3.1-pro-preview
> Signed-off-by: Bill Wendling <morbo@google.com>

Various style nit-picks as dictated by
Documentation/dev-tools/kunit/style.rst

(We may need to teach Sashiko about KUnit style?)

> kernel/user_namespace_kunit.c | 92 +++++++++++++++++++++++++++++++++++

Like below's naming, this should be named after the suite, and in the
tests/ subdir (even when #included), so it should be:
kernel/tests/user_ns_map_kunit.c

> +config USER_NAMESPACE_KUNIT_TEST

The suite and Kconfig should match, so this should be
USER_NS_MAP_KUNIT_TEST

> +	bool "Test user namespace map insertion" if !KUNIT_ALL_TESTS
> +	depends on KUNIT=y
> +	default KUNIT_ALL_TESTS

I think you need a "depends on USER_NS" ?

> +static struct kunit_suite user_ns_map_test_suite = {
> +	.name = "user_ns_map",
> +	.test_cases = user_ns_map_test_cases,
> +};


-Kees

-- 
Kees Cook