Re: [PATCH] security/landlock: Add KUnit test for mixed valid and unknown namespace bits

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] landlock: test_ns_types_to_bits_mixed() fails on
  !__WARN_FLAGS architectures
- [Low] landlock: weak non-zero check in test_ns_types_to_bits_mixed()
- [Low] landlock: conversational text and duplicated changelog in commit
  message
--

commit b79997e431c689890f9fc8f5cca0b886e3519ebb
Author: Terence White <terrywhite2025@gmail.com>

> security/landlock: Add KUnit test for mixed valid and unknown namespace bits
>
> Good evening,
>
> This patch adds a KUnit test case (test_ns_types_to_bits_mixed) to exercise
> landlock_ns_types_to_bits() with a mixed bitmask containing both a valid
> namespace type (CLONE_NEWNS) and an unknown high bit. This verifies that
> the WARN_ON_ONCE truncation guard behaves as expected under
> partial-taint inputs.
>
> Tested locally with all 40 KUnit tests passing successfully.
>
> With kind regards,
>
> Terry White
>
> Add a KUnit test case (test_ns_types_to_bits_mixed) to exercise
> landlock_ns_types_to_bits() with a mixed bitmask containing both
> a valid namespace type (CLONE_NEWNS) and an unknown high bit.
> This ensures the WARN_ON_ONCE truncation guard functions correctly
> under partial-taint inputs.
>
> Signed-off-by: Terry White <terrywhite2025@gmail.com>

[Severity: Low]
This isn't a bug, but the commit message includes conversational email
notes ("Good evening", "Tested locally...", "With kind regards") that
belong below the "---" separator, duplicates the description paragraph,
and has a name mismatch between Author ("Terence White") and Signed-off-by
("Terry White").

> diff --git a/security/landlock/ns.c b/security/landlock/ns.c
> index 06d9784746fe9..d820289423e32 100644
> --- a/security/landlock/ns.c
> +++ b/security/landlock/ns.c
> @@ -217,6 +217,20 @@ static void test_ns_types_to_bits_unknown(struct kunit *const test)
>  	}
>  }
>  
> +static void test_ns_types_to_bits_mixed(struct kunit *const test)
> +{
> +	if (!IS_ENABLED(CONFIG_BUG))
> +		kunit_skip(test, "requires CONFIG_BUG");
> +
> +	/* clang-format off */
> +	kunit_warning_suppress(test) {
> +		/* clang-format on */
> +		KUNIT_EXPECT_NE(test, 0ULL,
> +				landlock_ns_types_to_bits(CLONE_NEWNS | (1ULL << 62)));

[Severity: Low]
Should this assert KUNIT_EXPECT_EQ(test,
landlock_ns_type_to_bit(CLONE_NEWNS), ...) instead of KUNIT_EXPECT_NE(test,
0ULL, ...)?  Checking only for a non-zero return value would still pass if
the unknown bit leaked into the returned mask or if the wrong bit was set.

> +		KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> +	}
> +}

[Severity: Medium]
Does KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1) fail on architectures
that do not define __WARN_FLAGS, such as ARCH=um (the default for kunit.py)?

On those architectures, WARN_ON_ONCE() in include/asm-generic/bug.h expands
to DO_ONCE_LITE_IF(condition, WARN_ON, 1), which sets its static
__already_done flag before calling into warn_slowpath_fmt() where KUnit
checks kunit_is_suppressed_warning().  Because test_ns_types_to_bits_unknown()
runs earlier in test_cases[] and triggers the single WARN_ON_ONCE() site in
landlock_ns_types_to_bits(), __already_done is already set when
test_ns_types_to_bits_mixed() runs, so the second warning never fires and
the suppressed warning count is 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/CANkf1tZeotONOj+uYxE0k3BT_mrZpPyaYDA0vj9zFEbY2zYMrw@mail.gmail.com?part=1