Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Date:   Mon, 9 Oct 2023 23:46:57 +0530
From:   swarup <swarupkotikalapudi@gmail.com>
To:     Alexey Dobriyan <adobriyan@gmail.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, shuah@kernel.org, hughd@google.com
Subject: Re: + selftests-proc-add-proc-pid-statm-output-validation.patch
 added to mm-nonmm-unstable branch
Message-ID: <ZSRDmTwxARRENuqK@swarup-virtual-machine>
References: <20231004201701.87CB5C433C7@smtp.kernel.org>
 <9ce82fc2-4e09-40c4-b5a5-a9a049c2f493@p183>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <9ce82fc2-4e09-40c4-b5a5-a9a049c2f493@p183>
Precedence: bulk

eOn Mon, Oct 09, 2023 at 09:14:53AM +0300, Alexey Dobriyan wrote:
> On Wed, Oct 04, 2023 at 01:17:00PM -0700, Andrew Morton wrote:
> > 
> > The patch titled
> >      Subject: selftests: proc: add /proc/$(pid)/statm output validation
> > has been added to the -mm mm-nonmm-unstable branch.  Its filename is
> >      selftests-proc-add-proc-pid-statm-output-validation.patch
> > 
> > This patch will shortly appear at
> >      https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/selftests-proc-add-proc-pid-statm-output-validation.patch
> > 
> > This patch will later appear in the mm-nonmm-unstable branch at
> >     git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > 
> > Before you just go and hit "reply", please:
> >    a) Consider who else should be cc'ed
> >    b) Prefer to cc a suitable mailing list as well
> >    c) Ideally: find the original patch on the mailing list and do a
> >       reply-to-all to that, adding suitable additional cc's
> > 
> > *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
> > 
> > The -mm tree is included into linux-next via the mm-everything
> > branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > and is updated there every 2-3 working days
> > 
> > ------------------------------------------------------
> > From: Swarup Laxman Kotiaklapudi <swarupkotikalapudi@gmail.com>
> > Subject: selftests: proc: add /proc/$(pid)/statm output validation
> > Date: Wed, 4 Oct 2023 01:13:19 +0530
> > 
> > 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 check except for 4th position, all
> > other positions have value zero.
> > 
> > Link: https://lkml.kernel.org/r/20231003194319.602646-1-swarupkotikalapudi@gmail.com
> > Signed-off-by: Swarup Laxman Kotiaklapudi <swarupkotikalapudi@gmail.com>
> > Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > Cc: Hugh Dickins <hughd@google.com>
> > Cc: Shuah Khan <shuah@kernel.org>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > 
> >  tools/testing/selftests/proc/proc-empty-vm.c |   57 +++++++++++++++--
> >  1 file changed, 52 insertions(+), 5 deletions(-)
> > 
> > --- 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,56 @@ static int test_proc_pid_smaps_rollup(pi
> >  	}
> >  }
> >  
> > +static int test_proc_pid_statm(pid_t pid)
> > +{
> > +	char buf[4096];
> > +	char *tok;
> > +	char *string;
> > +	int non_zero_value_indx = 4;
> > +	int i = 1;
> > +
> > +	snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
> > +
> > +	/*
> > +	 *  Output can be "0 0 0 2 0 0 0\n" where "2" can be anything.
> > +	 */
> > +	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);
> > +		assert(rv);
> > +		string = buf;
> > +
> > +		while ((tok = strsep(&string, " ")) != NULL) {
> 
> This is unreliable too. read() doesn't terminate the buffer so this relies
> on termination from
> 
> 	snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
> 
> Buggy kernel could return a lot of data and overwrite it.

Hi Alexey Dobriyan,
I will try to correct read() function.
Thanks,
Swarup