Re: [PATCH -next 9/9] selftests/lsm: add lockdown securityfs test
- From
- sashiko-bot@kernel.org
- Date
- 2026-11-03 08:02:28
- Also in
- bpf
- Reply to
- Cai Xinchen, 2026-11-03
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/lsm: duplicated setup/teardown and double open in lockdown_test.c
--
commit 5cebacd6e5feb5064c2370aa4d99aad209688038
Author: Cai Xinchen [off-list ref]
selftests/lsm: add lockdown securityfs test
This commit adds a kselftest for the lockdown securityfs interface to
check the read format, reject unknown levels, and verify that an already
locked down kernel rejects the current or a lower level. It also moves
the file reading logic in common.c into a shared read_file() helper.
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/lsm/lockdown_test.c b/tools/testing/selftests/lsm/lockdown_test.c new file mode 100644 index 0000000000000..35109833da373 --- /dev/null +++ b/tools/testing/selftests/lsm/lockdown_test.c@@ -0,0 +1,176 @@
[ ... ]
quoted hunk ↗ jump to hunk
+static int write_status(const char *value) +{ + ssize_t len; + int fd, err; + + fd = open(LOCKDOWN_PATH, O_WRONLY); + if (fd < 0) + return -1; + + len = write(fd, value, strlen(value)); + err = errno; + close(fd); + if (len < 0) { + errno = err; + return -1; + } + + return 0; +} + +static int status_writable(void) +{ + int fd; + + fd = open(LOCKDOWN_PATH, O_WRONLY); + if (fd < 0) + return -1; + + close(fd); + return 0; +}
[ ... ]
quoted hunk ↗ jump to hunk
+TEST(lockdown_invalid_write) +{ + char before[128], after[128]; + + if (read_file(LOCKDOWN_PATH, before, sizeof(before))) { + if (errno == ENOENT) + SKIP(return, "%s is not present", LOCKDOWN_PATH); + SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH, + strerror(errno)); + } + if (geteuid() != 0) + SKIP(return, "requires root"); + if (status_writable()) + SKIP(return, "cannot open %s for writing: %s", + LOCKDOWN_PATH, strerror(errno)); + + errno = 0; + ASSERT_EQ(-1, write_status("invalid-level")); + ASSERT_EQ(EINVAL, errno); + + ASSERT_EQ(0, read_file(LOCKDOWN_PATH, after, sizeof(after))); + ASSERT_STREQ(before, after); +} + +TEST(lockdown_no_downgrade) +{ + char before[128], after[128], level[32]; + const char *lower; + + if (read_file(LOCKDOWN_PATH, before, sizeof(before))) { + if (errno == ENOENT) + SKIP(return, "%s is not present", LOCKDOWN_PATH); + SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH, + strerror(errno)); + } + if (geteuid() != 0) + SKIP(return, "requires root"); + if (status_writable()) + SKIP(return, "cannot open %s for writing: %s", + LOCKDOWN_PATH, strerror(errno)); + ASSERT_EQ(0, get_active_level(before, level, sizeof(level))); + + if (!strcmp(level, "confidentiality")) + lower = "integrity"; + else if (!strcmp(level, "integrity")) + lower = "none"; + else + SKIP(return, "kernel is not locked down, refusing to escalate"); + + errno = 0; + ASSERT_EQ(-1, write_status(lower)); + ASSERT_EQ(EPERM, errno); + + ASSERT_EQ(0, read_file(LOCKDOWN_PATH, after, sizeof(after))); + ASSERT_STREQ(before, after); +} + +TEST(lockdown_same_level_rejected) +{ + char before[128], after[128], level[32]; + + if (read_file(LOCKDOWN_PATH, before, sizeof(before))) { + if (errno == ENOENT) + SKIP(return, "%s is not present", LOCKDOWN_PATH); + SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH, + strerror(errno)); + } + if (geteuid() != 0) + SKIP(return, "requires root"); + if (status_writable()) + SKIP(return, "cannot open %s for writing: %s", + LOCKDOWN_PATH, strerror(errno)); + ASSERT_EQ(0, get_active_level(before, level, sizeof(level))); + + errno = 0; + ASSERT_EQ(-1, write_status(level)); + ASSERT_EQ(EPERM, errno); + + ASSERT_EQ(0, read_file(LOCKDOWN_PATH, after, sizeof(after))); + ASSERT_STREQ(before, after); +}
[Severity: Low] This isn't a bug, but lockdown_invalid_write, lockdown_no_downgrade, and lockdown_same_level_rejected duplicate the same initial read_file() and SKIP() checks, root check, status_writable() check, active level parsing, and trailing read_file() plus ASSERT_STREQ(before, after) check. Additionally, in each of those write tests, status_writable() opens LOCKDOWN_PATH with O_WRONLY and immediately closes it, only for write_status() to open and close LOCKDOWN_PATH with O_WRONLY a second time. Could this use FIXTURE(), FIXTURE_SETUP(), FIXTURE_TEARDOWN(), and TEST_F() from kselftest_harness.h (or a shared setup/write helper) to avoid the duplicated prerequisite/post-condition blocks and redundant file opens? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=9