Re: [PATCH v1 2/2] selftests/landlock: Check full ioctl commands in audit records

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete application of the architecture-independent ioctl test update, leaving `audit_quiet_layout1_test_body()` still using the architecture-dependent `FIONREAD`.
--

commit e4cb547991b6b44a81dce30858992f55ca217a5f
Author: Mickaël Salaün <mic@digikod.net>

selftests/landlock: Check full ioctl commands in audit records

This commit updates the ioctl audit test to use a 32-bit
architecture-independent command instead of FIONREAD. This ensures the test
can detect if the kernel truncates the encoded direction and size bits in the
audit record.

> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 86e08aa6e0a7e..92d2f720ab054 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c

[ ... ]

> @@ -7702,10 +7706,10 @@ TEST_F(audit_layout1, ioctl_dev)
>  
>  	fd = open("/dev/null", O_RDONLY | O_CLOEXEC);
>  	ASSERT_LE(0, fd);
> -	EXPECT_EQ(EACCES, ioctl_error(_metadata, fd, FIONREAD));
> +	EXPECT_EQ(EACCES, ioctl_error(_metadata, fd, unknown_ioctl_cmd));
>  	EXPECT_EQ(0, matches_log_fs_extra(_metadata, self->audit_fd,
>  					  "fs\\.ioctl_dev", "/dev/null",
> -					  " ioctlcmd=0x541b"));
> +					  " ioctlcmd=0xc00ffeee"));

[Severity: Medium]
Should the audit_quiet_layout1_test_body() function receive this same update?

It appears that audit_quiet_layout1_test_body() still uses FIONREAD and
hardcodes the x86 16-bit value:

tools/testing/selftests/landlock/fs_test.c:audit_quiet_layout1_test_body() {
    ...
    ret = ioctl_error(_metadata, fd, FIONREAD);
    ...
    if (target->audit_ioctl)
        ASSERT_EQ(0, matches_log_fs_extra(_metadata, self->audit_fd,
                                          "fs\\.ioctl_dev", target->target,
                                          " ioctlcmd=0x541b\\+"));
    ...
}

Leaving this unchanged means the quiet audit tests will fail to verify the
32-bit truncation fix. It also leaves the test broken on non-x86 architectures
like Alpha and MIPS where FIONREAD has a different value.

>  
>  	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
>  	EXPECT_EQ(0, records.access);

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