Re: [PATCH v2 2/2] selftests/bpf: add tests to verify the enforcement of CONFIG_CGROUP_LSM_NUM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Resource Leak (Memory and File Descriptor) for bpf_link objects
- [High] Logic Flaw preventing graceful test skip on unsupported kernels
- [Medium] Test flakiness under concurrent execution due to global system limit
- [Medium] Test failure on kernels with different configurations due to hardcoded limit
--

commit 18126ecb98cd2c651e3993d4401ade44b2577a2f
Author: Paul Houssel <paulhoussel2@gmail.com>

selftests/bpf: add tests to verify the enforcement of CONFIG_CGROUP_LSM_NUM
    
This patch adds a selftest to verify that the kernel limits the number of
concurrently attachable per-cgroup LSM hook slots to the configured limit.

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_lsm_num.c b/tools/testing/selftests/bpf/prog_tests/cgroup_lsm_num.c
new file mode 100644
index 0000000000000..1c5825c6c3d07
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_lsm_num.c
[ ... ]
> void test_cgroup_lsm_num(void)
> {

Since CONFIG_CGROUP_LSM_NUM enforces a global limit across the system,
could parallel selftest execution (e.g. test_progs -j) consume slots and
cause this test to randomly fail?

Should this test be renamed to serial_test_cgroup_lsm_num() to prevent
concurrent execution issues?

[ ... ]
> 	bpf_object__for_each_program(prog, skel->obj) {
> 		struct bpf_link *link;
> 
> 		link = bpf_program__attach_cgroup(prog, cgroup_fd);
> 		if (!link) {
> 			if (errno == EOPNOTSUPP) {

When CONFIG_BPF_LSM is disabled, bpf_cgroup_atype_find() returns
-EOPNOTSUPP, but __cgroup_bpf_attach() translates any negative atype to
-EINVAL.

Does this mean the test will receive EINVAL instead of EOPNOTSUPP and fail
to skip gracefully on unsupported kernels?

> 				test__skip();
> 				goto out;
> 			}
> 			failed++;
> 		} else {
> 			attached++;
> 		}
> 	}

When a program successfully attaches, bpf_program__attach_cgroup()
allocates a struct bpf_link and a kernel file descriptor. Since the link
pointer is overwritten on each loop iteration and the skeleton destroy
function does not free manually attached links, do these links and file
descriptors leak and prevent the cgroup from being destroyed?

> 
> 	// CONFIG_CGROUP_LSM_NUM set to 10
> 	// -> 10 programs shall be attached
> 	ASSERT_EQ(attached, 10, "at least one attached");

BPF selftests frequently run against external or distribution kernels. If
the test runs on a kernel compiled with a larger limit (e.g., 20), all 12
programs will attach successfully and this assertion will fail.

Can we use extern int CONFIG_CGROUP_LSM_NUM __kconfig in the BPF code to
dynamically retrieve the actual limit instead of hardcoding 10?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260506131257.713895-1-paulhoussel2@gmail.com?part=2