All of lore.kernel.org
 help / color / mirror / Atom feed
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

  reply	other threads:[~2025-11-11  0:40 UTC|newest]

Thread overview: 23+ 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
2025-11-21 17:48 ` [PATCH v2] security: Rename functions and add namespace mapping tests Ryan Foster
2025-11-25 15:15   ` Serge E. Hallyn
2025-12-04 21:56     ` [PATCH] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns Ryan Foster
2025-12-16 22:57       ` Paul Moore
2025-12-18 18:49       ` [PATCH v3 sec cap tests] " Ryan Foster
2025-12-28 19:45       ` commoncap KUnit tests v4 Ryan Foster
2025-12-28 19:45         ` [PATCH v4] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns Ryan Foster
2025-12-29  4:14           ` Serge E. Hallyn
2025-12-29 14:23             ` [PATCH v5] " Ryan Foster
2025-12-29 13:56           ` [PATCH v4] " kernel test robot
2025-12-29 18:35           ` kernel test robot
2025-12-30 15:13 ` [PATCH v5] " Ryan Foster
2026-01-06 21:07   ` Serge E. Hallyn
2026-01-07 21:51     ` [PATCH v6] " Ryan Foster
2026-01-10  4:50       ` Serge E. Hallyn
  -- strict thread matches above, loose matches on Subject: below --
2025-11-10  1:16 [PATCH] security: Add KUnit tests for rootid_owns_currentns() ryan foster
2025-11-10  1:13 ryan foster
2025-11-10 23:17 ` Paul Moore
2025-11-11  0:12   ` Paul Moore

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 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.