linux-kernel - Re: + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch

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-next>] [day] [month] [year] [list]

Message-ID: <c1c55bdb-78cd-4b74-b9e5-2667dfdf9b2a@p183>
Date:   Mon, 2 Oct 2023 15:38:25 +0300
From:   Alexey Dobriyan <adobriyan@...il.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-kernel@...r.kernel.org, shuah@...nel.org, hughd@...gle.com,
        swarupkotikalapudi@...il.com
Subject: Re: + selftests-proc-add-proc-pid-statm-output-validation.patch
 added to mm-nonmm-unstable branch

On Sun, Oct 01, 2023 at 12:37:40PM -0700, Andrew Morton wrote:
>      selftests-proc-add-proc-pid-statm-output-validation.patch

> Add /proc/${pid}/statm validation
> 
> /proc/$(pid)/statm output is expected to be:
>  "0 0 0 * 0 0 0\n"
> Here * can be any value
> 
> Read output of /proc/$(pid)/statm
> and compare length of output is
> equal or greater than expected output

> --- a/tools/testing/selftests/proc/proc-empty-vm.c~selftests-proc-add-proc-pid-statm-output-validation
> +++ a/tools/testing/selftests/proc/proc-empty-vm.c
> @@ -303,6 +303,37 @@ static int test_proc_pid_smaps_rollup(pi
>  	}
>  }
>  
> +static const char g_statm[] = "0 0 0 * 0 0 0\n";

This is both unreliable and incorrect.

4th value is "end_code - start_code" when exec is done which could be
anything not 1-digit number (although unlikely).

Testing for strlen is simply too weak of a test.

> +static int test_proc_pid_statm(pid_t pid)
> +{
> +	char buf[4096];
> +
> +	snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
> +
> +	int fd = open(buf, O_RDONLY);
> +
> +	if (fd == -1) {
> +		if (errno == ENOENT) {
> +			/*
> +			 * /proc/${pid}/statm is under CONFIG_PROC_PAGE_MONITOR,
> +			 * it doesn't necessarily exist.
> +			 */
> +			return EXIT_SUCCESS;
> +		}
> +		perror("open /proc/${pid}/statm");
> +		return EXIT_FAILURE;
> +	} else {
> +		ssize_t rv = read(fd, buf, sizeof(buf));
> +
> +		close(fd);
> +		size_t len = strlen(g_statm);
> +
> +		assert(rv >= len);
> +		return EXIT_SUCCESS;
> +	}
> +}
> +
>  int main(void)
>  {
>  	int rv = EXIT_SUCCESS;
> @@ -389,11 +420,8 @@ int main(void)
>  		if (rv == EXIT_SUCCESS) {
>  			rv = test_proc_pid_smaps_rollup(pid);
>  		}
> -		/*
> -		 * TODO test /proc/${pid}/statm, task_statm()
> -		 * ->start_code, ->end_code aren't updated by munmap().
> -		 * Output can be "0 0 0 2 0 0 0\n" where "2" can be anything.
> -		 */
> +		if (rv == EXIT_SUCCESS)
> +			rv = test_proc_pid_statm(pid);
>  
>  		/* Cut the rope. */