All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Bill Wendling <morbo@google.com>, Christian Brauner <brauner@kernel.org>
Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>,
	"Bradley Morgan" <brads@mainlining.org>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	"Aleksa Sarai" <cyphar@cyphar.com>, "Jan Kara" <jack@suse.cz>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Thomas Gleixner" <tglx@kernel.org>,
	"Nicolas Schier" <nsc@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Douglas Anderson" <dianders@chromium.org>,
	"Anand Moon" <linux.amoon@gmail.com>,
	"Oleg Nesterov" <oleg@redhat.com>,
	codemender-patching+linux@google.com,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map
Date: Fri, 4 Sep 2026 16:34:33 -0700	[thread overview]
Message-ID: <202609041620.81446C2A@keescook> (raw)
In-Reply-To: <CAGG=3QV1G0=19riYPPWhTXrW2X0YvyESdY=GesWm2F_8FEZwMg@mail.gmail.com>

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

  reply	other threads:[~2026-09-04 23:34 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-23 12:51 [PATCH 0/2] Add __counted_by_ptr attribute to struct uid_gid_map Bill Wendling
2026-08-23 12:51 ` [PATCH 1/2] userns: " Bill Wendling
2026-08-23 13:50   ` Bradley Morgan
2026-08-23 14:52   ` Oleg Nesterov
2026-08-26 22:05     ` Bill Wendling
2026-08-24 19:18   ` Gustavo A. R. Silva
2026-08-23 12:51 ` [PATCH 2/2] userns: Add KUnit test suite for uid_gid_map Bill Wendling
2026-08-23 12:53   ` Bill Wendling
2026-08-26 20:43     ` [PATCH v3 " Bill Wendling
2026-08-26 21:21       ` Bradley Morgan
2026-08-26 22:04         ` Bill Wendling
2026-08-23 13:48   ` [PATCH " Bradley Morgan
2026-08-26 20:37     ` Bill Wendling
2026-08-24  6:13   ` Thomas Weißschuh
2026-08-26 20:40     ` Bill Wendling
2026-08-27 13:25       ` Thomas Weißschuh
2026-09-08 22:27   ` [PATCH v5 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map Bill Wendling
2026-09-08 22:27     ` [PATCH v5 2/2] userns: Add KUnit test suite for uid_gid_map Bill Wendling
2026-09-09 15:36       ` Bradley Morgan
2026-09-09 19:54         ` Bill Wendling
2026-09-08 23:01     ` [PATCH v5 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map Oleg Nesterov
2026-09-09  7:46     ` Christian Brauner
2026-08-26 22:00 ` [PATCH v4 " Bill Wendling
2026-08-26 22:00   ` [PATCH v4 2/2] userns: Add KUnit test suite for uid_gid_map Bill Wendling
2026-08-26 22:27     ` Bradley Morgan
2026-08-27 13:36     ` Thomas Weißschuh
2026-08-27 19:27       ` Bill Wendling
2026-08-31  9:22         ` Thomas Weißschuh
2026-09-03 20:21           ` Bill Wendling
2026-09-04  8:02             ` Thomas Weißschuh
2026-09-04 20:37               ` Bill Wendling
2026-09-04 23:34                 ` Kees Cook [this message]
2026-09-08 17:44                   ` Bill Wendling
2026-09-04 23:41     ` Kees Cook
2026-09-08 17:46       ` Bill Wendling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202609041620.81446C2A@keescook \
    --to=kees@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=brads@mainlining.org \
    --cc=brauner@kernel.org \
    --cc=codemender-patching+linux@google.com \
    --cc=cyphar@cyphar.com \
    --cc=dianders@chromium.org \
    --cc=gary@garyguo.net \
    --cc=gustavoars@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux.amoon@gmail.com \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=nsc@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=oleg@redhat.com \
    --cc=tglx@kernel.org \
    --cc=thomas.weissschuh@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.