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@xxxxxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxx>
> > Cc: Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>
> > Cc: Kees Cook <kees@xxxxxxxxxx>
> > Cc: "Gustavo A. R. Silva" <gustavoars@xxxxxxxxxx>
> > Cc: Christian Brauner <brauner@xxxxxxxxxx>
> > Cc: Aleksa Sarai <cyphar@xxxxxxxxxx>
> > Cc: Jan Kara <jack@xxxxxxx>
> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx>
> > Cc: Miguel Ojeda <ojeda@xxxxxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxx>
> > Cc: Nicolas Schier <nsc@xxxxxxxxxx>
> > Cc: Gary Guo <gary@xxxxxxxxxxx>
> > Cc: "Thomas Weißschuh" <thomas.weissschuh@xxxxxxxxxxxxx>
> > Cc: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> > Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
> > Cc: Anand Moon <linux.amoon@xxxxxxxxx>
> > Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
> > Cc: codemender-patching+linux@xxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Cc: linux-hardening@xxxxxxxxxxxxxxx
> > ---
> >  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);
> > +}
>
> (...)