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

["Günther Noack" <gnoack@google.com>]

On Thu, Sep 10, 2026 at 04:48:34PM +0200, Mickaël Salaün wrote:
> The ioctl audit test uses FIONREAD, whose command value fits in 16 bits.
> It therefore cannot detect truncation of encoded direction and size
> bits.
> 
> Use an architecture-independent unknown command with set upper bits and
> require its complete value in the audit record.  Landlock rejects the
> command before the device handles it, and the existing ioctl enforcement
> test already exercises the same request.  The new expectation fails with
> ioctlcmd=0xfeee on an unfixed kernel and passes with ioctlcmd=0xc00ffeee
> after the shared audit field is widened.
> 
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  tools/testing/selftests/landlock/fs_test.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 18dbdb99aeba..1abdcc9ee784 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -4146,6 +4146,9 @@ TEST_F_FORK(layout1, o_path_ftruncate_and_ioctl)
>  	ASSERT_EQ(0, close(fd));
>  }
>  
> +/* Arbitrary command with nonzero bits in both 16-bit halves. */
> +static const unsigned int unknown_ioctl_cmd = 0xc00ffeee;
> +
>  /*
>   * ioctl_error - generically call the given ioctl with a pointer to a
>   * sufficiently large zeroed-out memory region.
> @@ -4249,7 +4252,7 @@ TEST_F_FORK(layout1, blanket_permitted_ioctls)
>  	EXPECT_EQ(EACCES, ioctl_error(_metadata, fd, FS_IOC_ZERO_RANGE));
>  
>  	/* Default case is also blocked. */
> -	EXPECT_EQ(EACCES, ioctl_error(_metadata, fd, 0xc00ffeee));
> +	EXPECT_EQ(EACCES, ioctl_error(_metadata, fd, unknown_ioctl_cmd));
>  
>  	ASSERT_EQ(0, close(fd));
>  }
> @@ -7943,6 +7946,7 @@ TEST_F(audit_layout1, truncate)
>  	EXPECT_EQ(1, records.domain);
>  }
>  
> +/* Checks that audit records preserve every ioctl command bit. */
>  TEST_F(audit_layout1, ioctl_dev)
>  {
>  	struct audit_records records;
> @@ -7952,10 +7956,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"));

Minor: I don't see much upside in extracting the shared unknown_ioctl_cmd
variable here; if we used the 0xc00ffeee literally in the
ioctl_error() call here, it would be clearer in this test how the
test's stimulus of calling ioctl() with 0xc00ffeee related to the
expected output, without having to do that side-lookup.

Logic looks good though, good to test it more exhaustively.

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

Reviewed-by: Günther Noack <gnoack@google.com>