Re: [PATCH v3 4/4] perf test: Update PE file test for other locations

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Oct 13, 2026 at 12:08:54AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] perf test: unconditional leak of get_argv_exec_path() and NULL deref
> --
> 
> commit 9107444f4267d3db60d0e4c357f3b28957bdb1bc
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf test: Update PE file test for other locations
> 
> This commit updates the PE file parsing test to check multiple directory paths
> for the test data. The logic is modified to try other paths similar to the
> shell_tests__dir_fd() fallback mechanism.
> 
> > diff --git a/tools/perf/tests/pe-file-parsing.c b/tools/perf/tests/pe-file-parsing.c
> > index 3979f38bf127c..190520ffc3ad6 100644
> > --- a/tools/perf/tests/pe-file-parsing.c
> > +++ b/tools/perf/tests/pe-file-parsing.c
> > @@ -74,17 +74,20 @@ static int test__pe_file_parsing(struct test_suite *test __maybe_unused,
> >  {
> >  	struct stat st;
> >  	char path_dir[PATH_MAX];
> > -
> > -	/* First try development tree tests. */
> > -	if (!lstat("./tests", &st))
> > -		return run_dir("./tests");
> > -
> > -	/* Then installed path. */
> > +	const char * const tests_dirs[] = {
> > +		"./tools/perf/tests",
> > +		"./tests",
> > +		"./source/tests",
> > +		path_dir,
> > +	};
> >  	snprintf(path_dir, PATH_MAX, "%s/tests", get_argv_exec_path());
> 
> [Severity: Low]
> Can this code in test__pe_file_parsing() lead to a memory leak and a potential
> NULL pointer dereference?
> 
> The get_argv_exec_path() function returns a dynamically allocated string that
> must be freed by the caller. Passing it directly to snprintf() inline causes
> the pointer to be permanently lost.
> 
> Additionally, since the patch moved this snprintf() call before the directory
> search loop, it now unconditionally allocates this string even if an earlier
> directory in the tests_dirs array would have succeeded.
> 
> Furthermore, if get_argv_exec_path() fails under memory pressure and returns
> NULL, snprintf("%s") could crash on strictly compliant libcs like musl.

These are pre-existing issues, will fix separately.

Thanks,
Namhyung

> 
> >  
> > -	if (!lstat(path_dir, &st))
> > -		return run_dir(path_dir);
> > +	for (size_t i = 0; i < ARRAY_SIZE(tests_dirs); ++i) {
> > +		if (!lstat(tests_dirs[i], &st))
> > +			return run_dir(tests_dirs[i]);
> > +	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261012235938.87912-1-namhyung@kernel.org?part=4