From: "Serge E. Hallyn" <serge@hallyn.com>
To: Ryan Foster <foster.ryan.r@gmail.com>
Cc: selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, serge@hallyn.com,
paul@paul-moore.com
Subject: Re: [PATCH] security: Add KUnit tests for rootid_owns_currentns()
Date: Mon, 10 Nov 2025 18:33:29 -0600 [thread overview]
Message-ID: <aRKEWVse2AzperzG@mail.hallyn.com> (raw)
In-Reply-To: <20251110143748.4144288-1-foster.ryan.r@gmail.com>
On Mon, Nov 10, 2025 at 06:37:48AM -0800, Ryan Foster wrote:
> The rootid_owns_currentns() function in security/commoncap.c is a
> security-critical function used to determine if a root ID from a
> parent namespace owns the current namespace. This function is used
> in multiple security-critical paths:
>
> 1. cap_inode_getsecurity() - file capability retrieval
> 2. get_vfs_caps_from_disk() - file capability loading during exec
>
> Despite its security-critical nature, this function had no test
> coverage. This patch adds KUnit tests to verify the function's
> behavior in various scenarios:
>
> - Root ID in init namespace (positive case)
> - Invalid vfsuid (negative case)
> - Non-root UID (negative case)
I'd be more excited about this if it tested the actual
functionality.
The rootid_owns_currentns() could be split so that it calls
a (static if not testing) inline rootid_owns_userns(), and
then you test the latter. Then create a few user namespaces
with different values for the namespace's uid 0, and make
sure rootid_owns_userns(testns, testuid) behaves correctly.
Actually, this function should probably be renamed.
"rootid_owns_currentns()" is not correct. It should just be
"uid_owns_currentns()".
> The function is made testable by exporting it when
> CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, while maintaining
> static visibility in production builds.
>
> This follows the pattern established by other security subsystems
> (IPE, Landlock) for KUnit testing.
>
> Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
> ---
> security/Kconfig | 17 ++++++
> security/Makefile | 1 +
> security/commoncap.c | 5 ++
> security/commoncap_test.c | 109 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 132 insertions(+)
> create mode 100644 security/commoncap_test.c
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 285f284dfcac..c7b3f42ef875 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -284,6 +284,23 @@ config LSM
>
> If unsure, leave this as the default.
>
> +config SECURITY_COMMONCAP_KUNIT_TEST
> + bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
> + depends on KUNIT=y
> + default KUNIT_ALL_TESTS
> + help
> + This builds the commoncap KUnit tests.
> +
> + KUnit tests run during boot and output the results to the debug log
> + in TAP format (https://testanything.org/). Only useful for kernel devs
> + running KUnit test harness and are not for inclusion into a
> + production build.
> +
> + For more information on KUnit and unit tests in general please refer
> + to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> +
> source "security/Kconfig.hardening"
>
> endmenu
> diff --git a/security/Makefile b/security/Makefile
> index 22ff4c8bd8ce..5b1285fde552 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_KEYS) += keys/
>
> # always enable default capabilities
> obj-y += commoncap.o
> +obj-$(CONFIG_SECURITY_COMMONCAP_KUNIT_TEST) += commoncap_test.o
> obj-$(CONFIG_SECURITY) += lsm_syscalls.o
> obj-$(CONFIG_MMU) += min_addr.o
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6bd4adeb4795..4d0e014ce7cd 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -358,7 +358,12 @@ int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry)
> return error;
> }
>
> +#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
> +bool rootid_owns_currentns(vfsuid_t rootvfsuid);
> +bool rootid_owns_currentns(vfsuid_t rootvfsuid)
> +#else
> static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
> +#endif
> {
> struct user_namespace *ns;
> kuid_t kroot;
> diff --git a/security/commoncap_test.c b/security/commoncap_test.c
> new file mode 100644
> index 000000000000..9757d433d314
> --- /dev/null
> +++ b/security/commoncap_test.c
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * KUnit tests for commoncap.c security functions
> + *
> + * Tests for security-critical functions in the capability subsystem,
> + * particularly namespace-related capability checks.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/user_namespace.h>
> +#include <linux/uidgid.h>
> +#include <linux/module.h>
> +
> +/* We need to include the actual vfsuid_t definition but avoid the problematic
> + * inline functions in mnt_idmapping.h. Include just the type definitions.
> + */
> +#include <linux/types.h>
> +
> +/* Forward declare the minimal types we need - match the actual kernel definitions */
> +struct mnt_idmap;
> +typedef struct {
> + uid_t val;
> +} vfsuid_t;
> +
> +/* Minimal macros we need - match kernel definitions from mnt_idmapping.h */
> +static inline uid_t __vfsuid_val(vfsuid_t uid)
> +{
> + return uid.val;
> +}
> +
> +#define VFSUIDT_INIT(val) ((vfsuid_t){ __kuid_val(val) })
> +#define INVALID_VFSUID VFSUIDT_INIT(INVALID_UID)
> +
> +#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
> +
> +/* Forward declaration - function is exported when KUNIT_TEST is enabled */
> +extern bool rootid_owns_currentns(vfsuid_t rootvfsuid);
> +
> +/**
> + * test_rootid_owns_currentns_init_ns - Test rootid_owns_currentns with init ns
> + *
> + * Verifies that a root ID in the init namespace correctly owns the current
> + * namespace when running in init_user_ns.
> + */
> +static void test_rootid_owns_currentns_init_ns(struct kunit *test)
> +{
> + vfsuid_t root_vfsuid;
> + kuid_t root_kuid;
> +
> + /* Create a root UID in init namespace */
> + root_kuid = KUIDT_INIT(0);
> + root_vfsuid = VFSUIDT_INIT(root_kuid);
> +
> + /* In init namespace, root should own current namespace */
> + KUNIT_EXPECT_TRUE(test, rootid_owns_currentns(root_vfsuid));
> +}
> +
> +/**
> + * test_rootid_owns_currentns_invalid - Test rootid_owns_currentns with invalid vfsuid
> + *
> + * Verifies that an invalid vfsuid correctly returns false.
> + */
> +static void test_rootid_owns_currentns_invalid(struct kunit *test)
> +{
> + vfsuid_t invalid_vfsuid;
> +
> + /* Use the predefined invalid vfsuid */
> + invalid_vfsuid = INVALID_VFSUID;
> +
> + /* Invalid vfsuid should return false */
> + KUNIT_EXPECT_FALSE(test, rootid_owns_currentns(invalid_vfsuid));
> +}
> +
> +/**
> + * test_rootid_owns_currentns_nonroot - Test rootid_owns_currentns with non-root UID
> + *
> + * Verifies that a non-root UID correctly returns false.
> + */
> +static void test_rootid_owns_currentns_nonroot(struct kunit *test)
> +{
> + vfsuid_t nonroot_vfsuid;
> + kuid_t nonroot_kuid;
> +
> + /* Create a non-root UID */
> + nonroot_kuid = KUIDT_INIT(1000);
> + nonroot_vfsuid = VFSUIDT_INIT(nonroot_kuid);
> +
> + /* Non-root UID should return false */
> + KUNIT_EXPECT_FALSE(test, rootid_owns_currentns(nonroot_vfsuid));
> +}
> +
> +static struct kunit_case commoncap_test_cases[] = {
> + KUNIT_CASE(test_rootid_owns_currentns_init_ns),
> + KUNIT_CASE(test_rootid_owns_currentns_invalid),
> + KUNIT_CASE(test_rootid_owns_currentns_nonroot),
> + {}
> +};
> +
> +static struct kunit_suite commoncap_test_suite = {
> + .name = "commoncap",
> + .test_cases = commoncap_test_cases,
> +};
> +
> +kunit_test_suite(commoncap_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +
> +#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
> +
> --
> 2.43.0
next prev parent reply other threads:[~2025-11-11 0:40 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-10 14:37 [PATCH] security: Add KUnit tests for rootid_owns_currentns() Ryan Foster
2025-11-11 0:33 ` Serge E. Hallyn [this message]
2025-11-11 6:52 ` kernel test robot
2025-11-11 7:03 ` kernel test robot
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=aRKEWVse2AzperzG@mail.hallyn.com \
--to=serge@hallyn.com \
--cc=foster.ryan.r@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=selinux@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).