Re: [PATCH 18/27] selftests/landlock: Fix regex_escape() error handling in matches_log_fs_extra()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: errno clobbering by free() in matches_log_fs_extra()
--

commit fde0a2f164054d717ba379b4c41a67fff69162e2
Author: Günther Noack <gnoack3000@gmail.com>
Date:   Thu Oct 8 14:25:47 2026 +0000

selftests/landlock: Fix regex_escape() error handling in matches_log_fs_extra()

This commit updates regex_escape() to use a more conventional error handling
interface. It now returns a ssize_t containing either the number of bytes in
the output, or -1 with errno set on failure, and updates the call site
accordingly.

> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index b1ce0f810e10d..9abc2adef79db 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c

[ ... ]

> @@ -7471,13 +7472,14 @@ static int matches_log_fs_extra(struct __test_metadata *const _metadata,
>  
>  	log_match_remaining -= chunk_len;
>  	log_match_cursor += chunk_len;
> -	log_match_cursor = regex_escape(absolute_path, log_match_cursor,
> -					log_match_remaining);
> +	escaped_len = regex_escape(absolute_path, log_match_cursor,
> +				   log_match_remaining);
>  	free(absolute_path);
> -	if (log_match_cursor < 0)
> -		return (long long)log_match_cursor;
> +	if (escaped_len < 0)
> +		return -errno;

[Severity: Medium]
Could this free() call clobber errno before it is returned?

If regex_escape() fails due to an insufficient buffer, it returns -1 and
sets errno to ENOMEM. The code in matches_log_fs_extra() then unconditionally
calls free(absolute_path) before checking escaped_len and reading errno.

Since libc's free() is not strictly guaranteed to preserve errno, and may
clobber it during internal operations like unmap, the original ENOMEM could
be lost. This might cause the function to return an incorrect error code or 0
(success) instead of -ENOMEM, masking the test failure.

Would it be safer to cache errno into a local variable before calling free(),
or move the free() call after the error check?

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