Re: [PATCH 14/27] selftests/landlock: Make audit_message large enough for any exe filter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/landlock: OOB read in regexec due to missing null terminator
--

commit fcf4b6b95bf6821e426c4d029e2300800d7024cd
Author: Günther Noack <gnoack3000@gmail.com>

selftests/landlock: Make audit_message large enough for any exe filter

This commit increases the size of the data array in struct audit_message to
accommodate a full audit_rule_data structure and a PATH_MAX string. It also
adds a bounds check in audit_filter_exe() to return -E2BIG if the request
size exceeds the new message buffer size.

> diff --git a/tools/testing/selftests/landlock/audit.h b/tools/testing/selftests/landlock/audit.h
> --- a/tools/testing/selftests/landlock/audit.h
> +++ b/tools/testing/selftests/landlock/audit.h
> @@ -41,7 +41,11 @@ struct audit_message {
>  		struct audit_features features;
>  		struct audit_rule_data rule;
>  		struct nlmsgerr err;
> -		char data[PATH_MAX + 200];
> +		/*
> +		 * Large enough for an audit_rule_data followed by a PATH_MAX
> +		 * string (see audit_filter_exe()), and for received records.
> +		 */
> +		char data[sizeof(struct audit_rule_data) + PATH_MAX];

[Severity: Medium]
Could this lead to an out-of-bounds read during a selftest run?

The buffer size was increased here, but there is no explicit space reserved
for, or assignment of, a null terminator.

If kauditd sends an audit message equal to or larger than sizeof(*msg) (for
example, if the executable path is extremely long and hex-encoded), the
recvfrom() call in audit_recv() will truncate it and read exactly that many
bytes:

tools/testing/selftests/landlock/audit.h:audit_recv() {
    ...
    err = recvfrom(fd, msg, sizeof(*msg), 0,
                   (struct sockaddr *)&addr, &addrlen);
    ...
}

Because the raw payload lacks a null terminator and fills the buffer entirely,
msg.data will not be null-terminated. Later, regexec() in
audit_match_record() assumes msg.data is a valid C string:

tools/testing/selftests/landlock/audit.h:audit_match_record() {
    ...
    ret = regexec(&regex, msg.data, ARRAY_SIZE(matches), matches, 0);
    ...
}

Since regexec() expects a null-terminated string, passing the completely
filled buffer could result in an out-of-bounds read on the stack,
potentially crashing the test. Should we ensure the received payload is
null-terminated, or perhaps reserve an extra byte for it in the buffer?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008142604.39107-2-gnoack3000@gmail.com?part=14