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:

Pre-existing issues:
- [Medium] Incomplete fix for file descriptor leaks to child processes (O_CLOEXEC missed for mkstemp).
--

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

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

This commit adds the O_CLOEXEC flag to open() calls in dso.c and symbol-elf.c.
This aims to prevent file descriptors from leaking to child processes spawned
during symbol resolution, which can exhaust the file descriptor limit.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> --- 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]
This isn't a bug introduced by this patch, but since the goal is to prevent
file descriptor leaks, doesn't mkstemp() also leave the file descriptor
without the O_CLOEXEC flag?

If the file is compressed, the returned decompressed file descriptor might
still leak to child processes. Could mkstemp(tmpbuf) be changed to
mkostemp(tmpbuf, O_CLOEXEC) to fully close the leak?

> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> --- 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);

[Severity: Medium]
This is a pre-existing issue, but updating only the open() call creates an
inconsistency between the temp and non-temp paths. Will temporary kcore file
descriptors still leak to child processes when temp is true?

Additionally, looking at read_gnu_debugdata() in this same file, there is
another pre-existing mkstemp() call used for temporary files. Would it make
sense to convert all of these mkstemp() calls to mkostemp(..., O_CLOEXEC) in
this series to completely prevent file descriptor exhaustion?

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