Re: [PATCH v2 10/10] perf test: Fix dso data tests

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Oct 06, 2026 at 11:59:03PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] perf test: NULL pointer dereference on dso__new() failure
> - [Medium] perf: NULL pointer dereference when dso__new() fails
> --
> 
> commit 2688b984099e76fca1d46c74193628a04bf63e97
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf test: Fix dso data tests
> 
> This commit fixes the dso data tests by bypassing the ELF check for binary
> types so that read/write operations on the test file can proceed properly.
> 
> > diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
> > index 46bc3f5972600..5a027ffab541b 100644
> > --- a/tools/perf/tests/dso-data.c
> > +++ b/tools/perf/tests/dso-data.c
> > @@ -138,6 +138,9 @@ static int test__dso_data(struct test_suite *test __maybe_unused, int subtest __
> >  	dsos__init(&machine.dsos);
> >  
> >  	dso = dso__new(file);
> > +	/* override the binary type for dso data access */
> > +	dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
> > +	dso__set_bintype_searched(dso);
> 
> [Severity: Medium]
> Will this crash if dso__new() fails?
> 
> If memory allocation fails and dso__new() returns NULL, these newly added
> calls will unconditionally dereference the pointer, causing a segmentation
> fault instead of allowing dsos__add() to handle the NULL safely and fail
> the test gracefully.

Ok, will check the return value.

> 
> >  	TEST_ASSERT_VAL("Failed to add dso", !dsos__add(&machine.dsos, dso));
> >  	TEST_ASSERT_VAL("Failed to access to dso",
> >  			dso__data_fd(dso, &machine) >= 0);
> 
> [ ... ]
> 
> > @@ -225,6 +228,9 @@ static int dsos__create(int cnt, int size, struct dsos *dsos)
> >  
> >  		TEST_ASSERT_VAL("failed to get dso file", file);
> >  		dso = dso__new(file);
> > +		/* override the binary type for dso data access */
> > +		dso__set_binary_type(dso, DSO_BINARY_TYPE__SYSTEM_PATH_DSO);
> > +		dso__set_bintype_searched(dso);
> 
> [Severity: Medium]
> Similarly, does this bypass the subsequent NULL check for dso?
> 
> By unconditionally dereferencing dso, a failure from dso__new() will cause
> a crash before reaching the explicit TEST_ASSERT_VAL() check intended to safely
> handle this exact allocation failure.

Ditto.

Thanks,
Namhyung