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 > <[email protected]> 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 > > > <[email protected]> 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 > > > > > <[email protected]> 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/[email protected]/ -Kees -- Kees Cook
