Re: [PATCH v4 08/10] perf symbol: Do not set binary type from symtab type

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Oct 13, 2026 at 12:08:12AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] perf: fully stripped executables falsely rejected for data access
> --
> 
> commit 6b9e46855d4e0d8df1dcaaed600da2f693471adb
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf symbol: Do not set binary type from symtab type
> 
> This commit separates tracking of the DSO binary type from the symtab type.
> It removes the explicit setting of the binary type during symbol loading,
> deferring it until the DSO is actually opened.
> 
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 4a8c298039447..116870595f75c 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1943,19 +1943,6 @@ int dso__load(struct dso *dso, struct map *map)
> >  
> >  		if (next_slot) {
> >  			ss_pos++;
> > -
> > -			/*
> > -			 * The binary type is used to find the file containing
> > -			 * the executed instructions, so prefer the types that
> > -			 * refer to the actual object over debug-only files such
> > -			 * as DSO_BINARY_TYPE__DEBUGLINK.
> > -			 */
> > -			if (dso__binary_type(dso) == DSO_BINARY_TYPE__NOT_FOUND ||
> > -			    symtab_type == DSO_BINARY_TYPE__BUILD_ID_CACHE ||
> > -			    (symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_DSO &&
> > -			     dso__binary_type(dso) != DSO_BINARY_TYPE__BUILD_ID_CACHE))
> > -				dso__set_binary_type(dso, symtab_type);
> > -
> 
> [Severity: Medium]
> Will removing this explicit assignment cause fully stripped executables to be
> falsely rejected during data access operations like perf annotate?
> 
> By leaving the DSO binary type as NOT_FOUND here, disassembly paths are forced
> to fall back to dso__find_binary_type_locked() to identify the file type.
> That logic attempts to verify the file using filename__has_section() for the
> .text section:
> 
> tools/perf/util/dso.c:dso__find_binary_type_locked() {
>     ...
>     found = filename__has_section(path, ".text");
>     ...
> }
> 
> For stripped executables (which have had their section headers removed but
> still have valid PT_LOAD segments), this check returns false. Previously,
> because dso__load() set the binary type directly when locating the system
> path DSO, it bypassed the strict .text check.
> 
> Consequently, tools like objdump that can parse PT_LOAD segments are never
> invoked, and operations fail with COULDNT_DETERMINE_FILE_TYPE.

Hmm.. ok.  But I don't think this is a common case and we have many
places to check section headers.  So practically we require section
headers to support it.

> 
> Additionally, could this cause debuginfo files to be incorrectly selected for
> data reads, since their SHT_NOBITS .text sections might falsely satisfy the
> check?

The function never tries such debuginfo file types.  But probably better
to chek section types.

Thanks,
Namhyung