Re: [PATCH v4 6/8] selftests/landlock: Add capability restriction tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/landlock: host hostname modification in cap_without_nnp test failure
--

commit 359c59ecc043d1cac20779ee8cc6bc19e3b8e705
Author: Mickaël Salaün <mic@digikod.net>
selftests/landlock: Add capability restriction tests

This commit adds tests to exercise capability restrictions enforced by
Landlock. It covers various standard capability evaluations, stacking
combinations, and non-standard contexts such as enforcing a domain
without no_new_privs.

> diff --git a/tools/testing/selftests/landlock/cap_test.c b/tools/testing/selftests/landlock/cap_test.c
> new file mode 100644
> index 0000000000000..8673934aac11e
> --- /dev/null
> +++ b/tools/testing/selftests/landlock/cap_test.c
[ ... ]
> +TEST(cap_without_nnp)
> +{
> +	int ruleset_fd;
> +
> +	disable_caps(_metadata);
> +
> +	ruleset_fd = create_cap_ruleset();
> +	ASSERT_LE(0, ruleset_fd);
> +
> +	/* Allow CAP_SYS_CHROOT but not CAP_SYS_ADMIN. */
> +	ASSERT_EQ(0, add_cap_rule(ruleset_fd, CAP_SYS_CHROOT));
> +
> +	/*
> +	 * Enforce WITHOUT NNP: landlock_restrict_self() succeeds when the
> +	 * caller has CAP_SYS_ADMIN (checked before the new domain takes
> +	 * effect).
> +	 */
> +	set_cap(_metadata, CAP_SYS_ADMIN);
> +	ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0));
> +	EXPECT_EQ(0, close(ruleset_fd));
> +
> +	/*
> +	 * CAP_SYS_ADMIN is still in effective set but Landlock denies it:
> +	 * cap_capable() returns 0, then hook_capable() returns -EPERM.
> +	 */
> +	EXPECT_EQ(-1, sethostname("test", 4));

[Severity: Medium]
Will this modify the host system's hostname if the test fails?

The cap_without_nnp test does not appear to isolate its UTS namespace via
unshare(CLONE_NEWUTS) before verifying the sethostname() restriction. If the
Landlock enforcement does not block the operation during a failing test run,
could the sethostname("test", 4) call succeed and unexpectedly change the
test machine's global hostname?

> +	EXPECT_EQ(EPERM, errno);
> +
> +	/* CAP_SYS_CHROOT is allowed by the rule. */
> +	set_cap(_metadata, CAP_SYS_CHROOT);
> +	EXPECT_EQ(0, chroot("/"));
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002124409.1277970-1-mic@digikod.net?part=6