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

[Shuah Khan]

On 10/1/24 07:49, Tycho Andersen wrote:
> From: Tycho Andersen <tandersen@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> ---
>   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