Re: [PATCH 08/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:
- [High] The patch incompletely fixes the file descriptor leak because it misses multiple mkstemp() calls that do not set O_CLOEXEC.
--

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

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

This patch adds O_CLOEXEC to open() calls in dso.c and symbol-elf.c to
prevent file descriptors from leaking to child processes spawned during
symbol resolution. This aims to prevent fd exhaustion during long
profiling sessions or when processing many DSOs.

> 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: High]
This is a pre-existing issue, but does mkstemp() also leak file descriptors
to child processes?

Since mkstemp() does not automatically set the O_CLOEXEC flag, the file
descriptor created here for the decompressed kernel module might still remain
open and leak to child processes (like addr2line) spawned during symbol
resolution.

Should a subsequent fcntl(fd, F_SETFD, FD_CLOEXEC) be added to fully resolve
the file descriptor exhaustion issue this patch targets?

>  	if (fd < 0) {

[ ... ]

> 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);

[Severity: High]
This is a pre-existing issue, but this mkstemp() call also creates a
temporary file for kcore without setting O_CLOEXEC.

Additionally, looking at read_gnu_debugdata() in the same file, there is
another call to mkstemp() used for extracted .gnu_debugdata:

tools/perf/util/symbol-elf.c:read_gnu_debugdata() {
    ...
    temp_fd = mkstemp(temp_filename);
    ...
}

Do these mkstemp() descriptors also need to have the FD_CLOEXEC flag set to
prevent them from leaking to child processes?

>  	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)
>  		return -1;

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