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@xxxxxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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 | 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