Re: [PATCH 07/11] perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete fix: The patch introduces an inconsistency where uncompressed DSOs are protected from FD leaks via O_CLOEXEC, but compressed DSOs and temporary files using mkstemp() still leak.
--

commit 3d059467c232a3fc60a5f840954aaa8bdad70992
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code

This commit adds O_CLOEXEC to open() calls in dso.c and symbol-elf.c to prevent
file descriptors from leaking to child processes during symbol resolution.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 7dced896c64ea..fb2e78fe2aa8e 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -344,7 +344,7 @@ int filename__decompress(const char *name, char *pathname,
>  	 * descriptor to the uncompressed file.
>  	 */
>  	if (!compressions[comp].is_compressed(name))
> -		return open(name, O_RDONLY);
> +		return open(name, O_RDONLY | O_CLOEXEC);
>  
>  	fd = mkstemp(tmpbuf);

[Severity: Medium]
Is this fix incomplete for compressed DSOs? The open() call for uncompressed
files now has O_CLOEXEC, but the mkstemp() call directly below it does not set
the close-on-exec flag. Should mkostemp() with O_CLOEXEC be used here to
prevent leaking the compressed DSO file descriptors to child processes?

> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 186e6d92ac3d7..c2bdfd0003df2 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
[ ... ]
> @@ -1985,7 +1985,7 @@ static int kcore__init(struct kcore *kcore, char *filename, int elfclass,
>  	if (temp)
>  		kcore->fd = mkstemp(filename);
>  	else
> -		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0400);
> +		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0400);
>  	if (kcore->fd == -1)

[Severity: Medium]
Does the temporary file branch also need similar protection? The else branch
now correctly uses O_CLOEXEC, but mkstemp() in the if (temp) branch will still
leak its file descriptor. 

Additionally, there appears to be another mkstemp() call without O_CLOEXEC in
read_gnu_debugdata() in the same file. Do all instances of mkstemp() creating
temporary files for debugging need to be updated to mkostemp() to fully 
address the leak?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608201753.1979464-1-acme@kernel.org?part=7