Re: [PATCH v4 bpf-next 3/3] selftests/bpf: add tests for bpf_init_inode_xattr kfunc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The tests use predictable temporary file paths with `open(..., O_CREAT)` but without `O_EXCL` or prior cleanup, leading to test flakiness and a symlink attack vulnerability.
- [Medium] The slot limit test triggers duplicate xattr name creations during `inode_init_security`, causing the test to fail on filesystems like ext4 that enforce `XATTR_CREATE`.
--

commit 9defa5666243f46c3a058ad117a6ee0ace1b9f5c
Author: David Windsor <dwindsor@gmail.com>

selftests/bpf: add tests for bpf_init_inode_xattr kfunc
    
Test bpf atomic inode xattr labeling in inode_init_security.

> diff --git a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c
> index 43a26ec69a8e3..8b2e0d433aea2 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c
> @@ -268,6 +269,116 @@ static void test_fsverity(void)
>  	remove(testfile);
>  }
>  
> +static void test_init_inode_xattr(void)
> +{
> +	struct test_init_inode_xattr *skel = NULL;
> +	int fd = -1, err;
> +	char value_out[64];
> +	const char *testfile_new = "/tmp/test_progs_fs_kfuncs_new";
> +
> +	skel = test_init_inode_xattr__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "test_init_inode_xattr__open_and_load"))
> +		return;
> +
> +	skel->bss->monitored_pid = getpid();
> +	err = test_init_inode_xattr__attach(skel);
> +	if (!ASSERT_OK(err, "test_init_inode_xattr__attach"))
> +		goto out;
> +
> +	/* Trigger inode_init_security */
> +	fd = open(testfile_new, O_CREAT | O_RDWR, 0644);

[Severity: Medium]
Since this test runs as root and uses predictable paths in /tmp/, can this
allow an attacker to place a symlink to overwrite arbitrary system files?

Additionally, if a previous run leaves the file behind (e.g., via panic or
early abort), open(O_CREAT) without O_EXCL will open the existing file
instead of creating a new one. This bypasses the inode_init_security hook
entirely, which can cause the test's assertions on init_result to fail
spuriously.

Would it be better to add O_EXCL to the open() flags or unlink the file
before opening?

> +	if (!ASSERT_GE(fd, 0, "create_file"))
> +		goto out;

[ ... ]

> +static void test_init_inode_xattr_slot_limit(void)
> +{
> +	struct test_init_inode_xattr *skel[INIT_INODE_XATTR_MAX + 1] = {};
> +	struct bpf_link *link[INIT_INODE_XATTR_MAX + 1] = {};
> +	const char *testfile_slot = "/tmp/test_progs_fs_kfuncs_slot";
> +	int ok = 0, nospc = 0, other = 0;
> +	int i, fd = -1;
> +
> +	/* All programs attach successfully; there is no attach-time cap. */
> +	for (i = 0; i <= INIT_INODE_XATTR_MAX; i++) {
> +		skel[i] = test_init_inode_xattr__open_and_load();
> +		if (!ASSERT_OK_PTR(skel[i], "open_and_load"))
> +			goto out;
> +
> +		skel[i]->bss->monitored_pid = getpid();
> +
> +		link[i] = bpf_program__attach_lsm(skel[i]->progs.test_init_inode_xattr);
> +		if (!ASSERT_OK_PTR(link[i], "attach"))
> +			goto out;
> +	}
> +
> +	/* Trigger inode_init_security once with all programs attached. */
> +	fd = open(testfile_slot, O_CREAT | O_RDWR, 0644);

[Severity: Medium]
Will this test reliably fail on filesystems like ext4?

The test attaches 5 identical BPF programs to inode_init_security, which all
attempt to add an xattr with the exact same name (bpf.test_label).

On ext4, ext4_initxattrs() passes the XATTR_CREATE flag to
ext4_xattr_set_handle(), meaning the second identical xattr creation fails
with -EEXIST. This failure propagates up to the VFS, causing the open()
syscall here to fail with -EEXIST and breaking the test before it can validate
the slot limits.

Should the attached BPF programs generate unique xattr names?

(Also, similar to test_init_inode_xattr(), this open() call might benefit
from O_EXCL to prevent symlink attacks and test flakiness).

> +	if (!ASSERT_GE(fd, 0, "create_file"))
> +		goto out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630183956.281293-1-dwindsor@gmail.com?part=3