linux-kernel - Re: [PATCH v3 2/2] selftests/exec: add a test to enforce execveat()'s comm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7ad58665-ed3f-4b20-b7ee-5d8314de3cc2@linuxfoundation.org>
Date: Tue, 1 Oct 2024 09:14:49 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Tycho Andersen <tycho@...ho.pizza>,
 Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 Eric Biederman <ebiederm@...ssion.com>, Kees Cook <kees@...nel.org>
Cc: linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
 Tycho Andersen <tandersen@...flix.com>,
 Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH v3 2/2] selftests/exec: add a test to enforce execveat()'s
 comm

On 10/1/24 07:49, Tycho Andersen wrote:
> From: Tycho Andersen <tandersen@...flix.com>
> 
> We want to ensure that /proc/self/comm stays useful for execveat() callers.

This commit message is vague? What does staying useful mean?
Elaborate on the staying useful and the tests added to ensure.
Add test results as well.

> 
> Signed-off-by: Tycho Andersen <tandersen@...flix.com>
> ---
>   tools/testing/selftests/exec/execveat.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c
> index 071e03532cba..091029f4ca9b 100644
> --- a/tools/testing/selftests/exec/execveat.c
> +++ b/tools/testing/selftests/exec/execveat.c
> @@ -419,6 +419,9 @@ int main(int argc, char **argv)
>   	if (argc >= 2) {
>   		/* If we are invoked with an argument, don't run tests. */
>   		const char *in_test = getenv("IN_TEST");
> +		/* TASK_COMM_LEN == 16 */
> +		char buf[32];
> +		int fd;
>   
>   		if (verbose) {
>   			ksft_print_msg("invoked with:\n");
> @@ -432,6 +435,28 @@ int main(int argc, char **argv)
>   			return 1;
>   		}
>   
> +		fd = open("/proc/self/comm", O_RDONLY);
> +		if (fd < 0) {
> +			perror("open comm");

The existing code in this file uses ksft_perror() - please keep
the new code consistent with the existing code.

> +			return 1;
> +		}
> +
> +		if (read(fd, buf, sizeof(buf)) < 0) {
> +			close(fd);
> +			perror("read comm");

Same comment as above.

> +			return 1;
> +		}
> +		close(fd);
> +
> +		/*
> +		 * /proc/self/comm should fail to convert to an integer, i.e.
> +		 * atoi() should return 0.
> +		 */
> +		if (atoi(buf) != 0) {
> +			ksft_print_msg("bad /proc/self/comm: %s", buf);
> +			return 1;
> +		}
> +
>   		/* Use the final argument as an exit code. */
>   		rc = atoi(argv[argc - 1]);
>   		exit(rc);

thanks,
-- Shuah