[PATCHES 0/5] perf DSO hardening series

[PATCHES 0/5] perf DSO hardening series

[Arnaldo Carvalho de Melo]

Hi,

	Please consider merging,

- Arnaldo

Arnaldo Carvalho de Melo (5):
  perf dso: Guard against errno==0 when dso__get_filename() returns NULL
  perf dso: Guard close() against invalid fd in dso__decompress_kmodule_path()
  perf dso: Use stored fd error instead of stale errno in file_read()
  perf dso: Guard against cache underflow on short reads in dso_cache__memcpy()
  perf dso: Replace assert with runtime check in dso__read_symbol()

 tools/perf/util/dso.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

-- 
2.55.0

[PATCH 1/5] perf dso: Guard against errno==0 when dso__get_filename() returns NULL

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

__open_dso() computes fd = -errno when dso__get_filename() returns NULL.
Some failure paths in dso__get_filename() (e.g. binary type mismatch)
return NULL without making a syscall, leaving errno at 0 from a prior
successful call.  fd = -0 = 0, which is stdin — subsequent code treats
it as a valid file descriptor.

Fall back to ENOENT when errno is 0, ensuring fd is always negative on
failure.

Fixes: eba5102d2f0b ("perf tools: Add global list of opened dso objects")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 2309196d8df3111c..e087a89066bdbc02 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -643,7 +643,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 	if (name)
 		fd = do_open(name);
 	else
-		fd = -errno;
+		fd = errno ? -errno : -ENOENT;
 
 	if (decomp)
 		unlink(name);
-- 
2.55.0

[PATCH 2/5] perf dso: Guard close() against invalid fd in dso__decompress_kmodule_path()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

dso__decompress_kmodule_path() unconditionally calls close(fd) on the
return value of decompress_kmodule().  When decompression fails or the
DSO is not compressed, decompress_kmodule() returns -1.  close(-1)
fails with EBADF and clobbers errno, which callers up the chain
(dso__get_filename → __open_dso) depend on for error propagation.

Guard the close() call with fd >= 0 so only valid file descriptors are
closed.

Fixes: 42b3fa670825 ("perf tools: Introduce dso__decompress_kmodule_{fd,path}")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index e087a89066bdbc02..d9c008465f377e79 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -395,7 +395,9 @@ int dso__decompress_kmodule_path(struct dso *dso, const char *name,
 {
 	int fd = decompress_kmodule(dso, name, pathname, len);
 
-	close(fd);
+	/* decompress_kmodule() returns -1 on failure, don't close(-1) */
+	if (fd >= 0)
+		close(fd);
 	return fd >= 0 ? 0 : -1;
 }
 
-- 
2.55.0

[PATCH 3/5] perf dso: Use stored fd error instead of stale errno in file_read()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

file_read() uses ret = -errno when dso__data(dso)->fd is negative after
try_to_open_dso() fails.  By this point errno has been through
mutex_lock(), nsinfo__mountns_enter(), and multiple open() attempts
inside try_to_open_dso() — it no longer reflects the actual open
failure.  If errno happens to be 0, ret = 0 looks like EOF rather than
an error.

The fd field already carries the negated errno from __open_dso()
(e.g. -EINVAL, -ENOENT), so use it directly instead of reading the
stale global errno.

Fixes: 33bdedcea2d7 ("perf tools: Protect dso cache fd with a mutex")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index d9c008465f377e79..207f8744aac97e8c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1026,7 +1026,8 @@ static ssize_t file_read(struct dso *dso, struct machine *machine,
 
 	if (dso__data(dso)->fd < 0) {
 		dso__data(dso)->status = DSO_DATA_STATUS_ERROR;
-		ret = -errno;
+		/* fd already carries the negated errno from __open_dso() */
+		ret = dso__data(dso)->fd;
 		goto out;
 	}
 
-- 
2.55.0

[PATCH 4/5] perf dso: Guard against cache underflow on short reads in dso_cache__memcpy()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

dso_cache__memcpy() computes cache_offset = offset - cache->offset,
then cache_size = min(cache->size - cache_offset, size).  The RB tree
lookup in __dso_cache__find() matches using the full
DSO__DATA_CACHE_SIZE window, but cache->size reflects the actual pread
return value from dso_cache__populate().

A short pread (e.g. near end-of-file) makes cache->size smaller than
DSO__DATA_CACHE_SIZE.  If a subsequent access targets an offset past
cache->offset + cache->size but within the DSO__DATA_CACHE_SIZE
window, the cache entry is found but cache_offset exceeds cache->size.
Since both are u64, the subtraction cache->size - cache_offset wraps
to a large value, min() selects the caller's size, and memcpy reads
out of bounds.

Return 0 (cache miss) when cache_offset falls outside the valid cached
range, so the caller re-reads from the backing file.

Fixes: 366df72657e0 ("perf dso: Refactor dso_cache__read()")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 207f8744aac97e8c..a0de56c93592a5dd 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1002,7 +1002,17 @@ static ssize_t dso_cache__memcpy(struct dso_cache *cache, u64 offset, u8 *data,
 				 u64 size, bool out)
 {
 	u64 cache_offset = offset - cache->offset;
-	u64 cache_size   = min(cache->size - cache_offset, size);
+	u64 cache_size;
+
+	/*
+	 * The RB tree matches using DSO__DATA_CACHE_SIZE, but a short
+	 * pread may leave cache->size smaller.  Treat an offset past
+	 * the valid data as a cache miss so the caller re-reads.
+	 */
+	if (cache_offset >= cache->size)
+		return 0;
+
+	cache_size = min(cache->size - cache_offset, size);
 
 	if (out)
 		memcpy(data, cache->data + cache_offset, cache_size);
-- 
2.55.0

[PATCH 5/5] perf dso: Replace assert with runtime check in dso__read_symbol()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

dso__read_symbol() asserts that len <= jited_prog_len, where len comes
from sym->end - sym->start (parsed from PERF_RECORD_KSYMBOL in
perf.data).  Both values originate from untrusted file input.

With NDEBUG (production builds), the assert is compiled out, allowing
an out-of-bounds heap read when the BPF program buffer is accessed.
Without NDEBUG, a crafted perf.data crashes perf with an assertion
failure.

Replace the assert with a runtime bounds check that returns NULL with
an appropriate error code, matching the existing error handling
pattern in this function.

Fixes: aa04707f507e ("perf dso: Support BPF programs in dso__read_symbol()")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Cc: Song Liu <song@kernel.org>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index a0de56c93592a5dd..f3209f285db7675b 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -2024,7 +2024,12 @@ const u8 *dso__read_symbol(struct dso *dso, const char *symfs_filename,
 			errno = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
 			return NULL;
 		}
-		assert(len <= info_linear->info.jited_prog_len);
+		if (len > info_linear->info.jited_prog_len) {
+			pr_debug("BPF symbol length %zu exceeds jited_prog_len %u\n",
+				 len, info_linear->info.jited_prog_len);
+			errno = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
+			return NULL;
+		}
 		*out_buf_len = len;
 		return (const u8 *)(uintptr_t)(info_linear->info.jited_prog_insns);
 #else
-- 
2.55.0

Re: [PATCH 5/5] perf dso: Replace assert with runtime check in dso__read_symbol()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Medium] dso__read_symbol() fails to offset into the concatenated BPF JIT instruction buffer for subprograms, unconditionally returning the instructions for the first subprogram.
- [Medium] The `is_64bit` flag for BPF JIT disassembly is hardcoded to the host's `sizeof(void *)` instead of reflecting the target's architecture, breaking cross-platform disassembly.
--

--- Patch [5]: [PATCH 5/5] perf dso: Replace assert with runtime check in dso__read_symbol() ---
commit 53ccbff8c0716057dbbc219812e142e5e96f1fe9
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

    perf dso: Replace assert with runtime check in dso__read_symbol()
    
    dso__read_symbol() asserts that len <= jited_prog_len, where both
    values originate from untrusted file input. With NDEBUG, the assert
    is compiled out, allowing an out-of-bounds heap read. This replaces
    the assert with a runtime bounds check that returns NULL.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index a0de56c93592a..f3209f285db76 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -2024,7 +2024,12 @@ const u8 *dso__read_symbol(struct dso *dso, const char *symfs_filename,

[Severity: Medium]
This is a pre-existing issue in dso__read_symbol(), but should the
is_64bit flag be based on the host's architecture?

Earlier in this function, it sets:

    *is_64bit = sizeof(void *) == sizeof(u64);

If analyzing a perf.data file generated on a 32-bit target using a 64-bit
host, won't this cause cross-platform disassembly to break because downstream
disassemblers receive the wrong bitness flag? Could we dynamically inspect
the target machine architecture from perf_env (e.g. e_machine) instead?

>  			errno = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
>  			return NULL;
>  		}
> -		assert(len <= info_linear->info.jited_prog_len);
> +		if (len > info_linear->info.jited_prog_len) {
> +			pr_debug("BPF symbol length %zu exceeds jited_prog_len %u\n",
> +				 len, info_linear->info.jited_prog_len);
> +			errno = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
> +			return NULL;
> +		}
>  		*out_buf_len = len;
>  		return (const u8 *)(uintptr_t)(info_linear->info.jited_prog_insns);

[Severity: Medium]
This isn't a bug introduced by this patch, but does returning the base of
the JITed instructions handle subprograms correctly?

If the perf.data file contains a multi-function BPF program (where
dso__bpf_prog(dso)->sub_id > 0), this appears to blindly return the
instructions for the first subprogram instead of the requested subprogram.

Should this calculate the byte offset for the target subprogram using
jited_func_lens?

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

Re: [PATCH 4/5] perf dso: Guard against cache underflow on short reads in dso_cache__memcpy()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Returning 0 from `dso_cache__memcpy()` causes a permanent EOF rather than triggering a cache miss and re-read as intended, breaking reads from dynamically growing files.

Pre-existing issues:
- [High] The RB-tree traversal in `__dso_cache__find()` is performed locklessly, creating a data race with concurrent cache insertions.
--

--- Patch [4]: [PATCH 4/5] perf dso: Guard against cache underflow on short reads in dso_cache__memcpy() ---
commit 06837abfd5cca9da5670ee54ec02e3164860900c
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf dso: Guard against cache underflow on short reads in dso_cache__memcpy()

dso_cache__memcpy() computes cache_size based on cache->size, but a short
pread can leave cache->size smaller than the RB tree lookup window. This
causes a subtraction wrap-around and an out-of-bounds memcpy when accessed
past the cached size. This patch attempts to fix it by returning 0 (cache
miss) when the offset falls outside the valid cached range so the caller
re-reads from the backing file.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 207f8744aac97..a0de56c93592a 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1002,7 +1002,17 @@ static ssize_t dso_cache__memcpy(struct dso_cache *cache, u64 offset, u8 *data,
>  				 u64 size, bool out)
>  {
>  	u64 cache_offset = offset - cache->offset;
> -	u64 cache_size   = min(cache->size - cache_offset, size);
> +	u64 cache_size;
> +
> +	/*
> +	 * The RB tree matches using DSO__DATA_CACHE_SIZE, but a short
> +	 * pread may leave cache->size smaller.  Treat an offset past
> +	 * the valid data as a cache miss so the caller re-reads.
> +	 */
> +	if (cache_offset >= cache->size)
> +		return 0;

[Severity: High]
Does returning 0 here actually trigger a re-read?

Looking at cached_io() which calls this function, it seems to treat a 0 return
value strictly as an EOF condition:

cached_io() {
    ...
    ret = dso_cache__memcpy(cache, offset, data, size, out);
    if (!ret)
        break;
    ...
}

If the file has grown and we hit this condition, won't this cause cached_io()
to terminate the loop with a silent EOF instead of reading the new data from
the backing file?

> +
> +	cache_size = min(cache->size - cache_offset, size);
>  
>  	if (out)
>  		memcpy(data, cache->data + cache_offset, cache_size);

[Severity: High]
This is a pre-existing issue, but I noticed a potential data race when looking
at the RB-tree lookups in this file.

In __dso_cache__find(), the RB-tree traversal is performed locklessly:

    while (*p != NULL) {
        ...
        cache = rb_entry(parent, struct dso_cache, rb_node);
        ...
        if (offset < cache->offset)
            p = &(*p)->rb_left;
        else if (offset >= end)
            p = &(*p)->rb_right;
    }

However, dso_cache__insert() modifies the RB-tree while correctly holding
the dso__lock(dso) mutex:

    mutex_lock(dso__lock(dso));
    ...
    rb_link_node(&new->rb_node, parent, p);
    rb_insert_color(&new->rb_node, root);

If a lockless reader in __dso_cache__find() traverses the tree concurrently
with a cache miss that triggers dso_cache__insert(), could the reader observe
inconsistent tree pointers during the rb_insert_color() rotations?

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

Re: [PATCH 1/5] perf dso: Guard against errno==0 when dso__get_filename() returns NULL

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Incomplete fix in `__open_dso()` causes error swallowing in callers by returning negative fd without setting `errno`.

Pre-existing issues:
- [Medium] Data race on adjacent `struct dso` bitfields leads to corruption and a TOCTOU mount namespace leak.
--

--- Patch [1]: [PATCH 1/5] perf dso: Guard against errno==0 when dso__get_filename() returns NULL ---
commit db31953321ccfc3f2f28e5db31230d5fef57578f
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf dso: Guard against errno==0 when dso__get_filename() returns NULL

This commit updates __open_dso() to fall back to ENOENT when
dso__get_filename() returns NULL but leaves errno as 0. This ensures fd
is always negative on failure.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 2309196d8df31..e087a89066bdb 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -643,7 +643,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
>  	if (name)
>  		fd = do_open(name);
>  	else
> -		fd = -errno;
> +		fd = errno ? -errno : -ENOENT;

[Severity: Medium]
Does this code need to explicitly set errno to ENOENT as well?

Callers of __open_dso() appear to check if the returned file descriptor is
negative, and then compute their return value using errno. For example, in
file_read():

tools/perf/util/dso.c:file_read() {
    ...
    fd = data_file_open(dso, machine);
    if (fd < 0)
        return -errno;
    ...
}

Since errno is left as 0, callers like file_read() will evaluate -errno
to 0, which incorrectly reports EOF instead of an open failure. Should we
update errno directly?

[Severity: Medium]
This is a pre-existing issue, but can the adjacent bitfields in struct dso
cause a data race that leaks mount namespaces?

Looking at the struct dso definition:

tools/perf/util/dso.h:struct dso {
    ...
    enum dso_binary_type symtab_type:8;
    enum dso_binary_type binary_type:8;
    ...
}

If one thread calls try_to_open_dso() and locklessly modifies binary_type,
while another concurrently modifies symtab_type holding dso__lock(dso),
the concurrent read-modify-write on the shared memory word can corrupt
binary_type.

This corruption appears to break the namespace exit check in open_dso():

tools/perf/util/dso.c:open_dso() {
    ...
    if (dso__binary_type(dso) != DSO_BINARY_TYPE__BUILD_ID_CACHE) {
        mutex_lock(dso__lock(dso));
        nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
        mutex_unlock(dso__lock(dso));
    }
    fd = __open_dso(dso, machine);
    if (dso__binary_type(dso) != DSO_BINARY_TYPE__BUILD_ID_CACHE)
        nsinfo__mountns_exit(&nsc);
    ...
}

If the bitfield gets corrupted during __open_dso(), the exit check might
fail, skipping nsinfo__mountns_exit() and permanently stranding the thread
in the wrong mount namespace. Should access to these bitfields be protected
by dso__lock, or should they be separated to avoid sharing memory bytes?

>  
>  	if (decomp)
>  		unlink(name);

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

Re: [PATCH 1/5] perf dso: Guard against errno==0 when dso__get_filename() returns NULL

[David Laight]

On Sun,  2 Aug 2026 11:20:18 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> __open_dso() computes fd = -errno when dso__get_filename() returns NULL.
> Some failure paths in dso__get_filename() (e.g. binary type mismatch)
> return NULL without making a syscall, leaving errno at 0 from a prior
> successful call.

Except that errno isn't set to zero by successful syscalls.
It is only ever set by ones that fail.
The application may set errno to zero (eg to detect errors from some
library functions).

So paths that don't make syscalls will be returning a 'random' errno.

	David

>  fd = -0 = 0, which is stdin — subsequent code treats
> it as a valid file descriptor.
> 
> Fall back to ENOENT when errno is 0, ensuring fd is always negative on
> failure.
> 
> Fixes: eba5102d2f0b ("perf tools: Add global list of opened dso objects")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/dso.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 2309196d8df3111c..e087a89066bdbc02 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -643,7 +643,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
>  	if (name)
>  		fd = do_open(name);
>  	else
> -		fd = -errno;
> +		fd = errno ? -errno : -ENOENT;
>  
>  	if (decomp)
>  		unlink(name);

Re: [PATCH 1/5] perf dso: Guard against errno==0 when dso__get_filename() returns NULL

[Arnaldo Carvalho de Melo]

On Sun, Aug 02, 2026 at 02:58:28PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] Incomplete fix in `__open_dso()` causes error swallowing in callers by returning negative fd without setting `errno`.
> 
> Pre-existing issues:
> - [Medium] Data race on adjacent `struct dso` bitfields leads to corruption and a TOCTOU mount namespace leak.
> --
> 
> --- Patch [1]: [PATCH 1/5] perf dso: Guard against errno==0 when dso__get_filename() returns NULL ---
> commit db31953321ccfc3f2f28e5db31230d5fef57578f
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> perf dso: Guard against errno==0 when dso__get_filename() returns NULL
> 
> This commit updates __open_dso() to fall back to ENOENT when
> dso__get_filename() returns NULL but leaves errno as 0. This ensures fd
> is always negative on failure.
> 
> > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> > index 2309196d8df31..e087a89066bdb 100644
> > --- a/tools/perf/util/dso.c
> > +++ b/tools/perf/util/dso.c
> > @@ -643,7 +643,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
> >  	if (name)
> >  		fd = do_open(name);
> >  	else
> > -		fd = -errno;
> > +		fd = errno ? -errno : -ENOENT;
> 
> [Severity: Medium]
> Does this code need to explicitly set errno to ENOENT as well?
> 
> Callers of __open_dso() appear to check if the returned file descriptor is
> negative, and then compute their return value using errno. For example, in
> file_read():
> 
> tools/perf/util/dso.c:file_read() {
>     ...
>     fd = data_file_open(dso, machine);
>     if (fd < 0)
>         return -errno;
>     ...
> }
> 
> Since errno is left as 0, callers like file_read() will evaluate -errno
> to 0, which incorrectly reports EOF instead of an open failure. Should we
> update errno directly?

Agreed, will fix for v2.

- Arnaldo
 
> [Severity: Medium]
> This is a pre-existing issue, but can the adjacent bitfields in struct dso
> cause a data race that leaks mount namespaces?
> 
> Looking at the struct dso definition:
> 
> tools/perf/util/dso.h:struct dso {
>     ...
>     enum dso_binary_type symtab_type:8;
>     enum dso_binary_type binary_type:8;
>     ...
> }
> 
> If one thread calls try_to_open_dso() and locklessly modifies binary_type,
> while another concurrently modifies symtab_type holding dso__lock(dso),
> the concurrent read-modify-write on the shared memory word can corrupt
> binary_type.
> 
> This corruption appears to break the namespace exit check in open_dso():
> 
> tools/perf/util/dso.c:open_dso() {
>     ...
>     if (dso__binary_type(dso) != DSO_BINARY_TYPE__BUILD_ID_CACHE) {
>         mutex_lock(dso__lock(dso));
>         nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
>         mutex_unlock(dso__lock(dso));
>     }
>     fd = __open_dso(dso, machine);
>     if (dso__binary_type(dso) != DSO_BINARY_TYPE__BUILD_ID_CACHE)
>         nsinfo__mountns_exit(&nsc);
>     ...
> }
> 
> If the bitfield gets corrupted during __open_dso(), the exit check might
> fail, skipping nsinfo__mountns_exit() and permanently stranding the thread
> in the wrong mount namespace. Should access to these bitfields be protected
> by dso__lock, or should they be separated to avoid sharing memory bytes?
> 
> >  
> >  	if (decomp)
> >  		unlink(name);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260802142022.154219-1-acme@kernel.org?part=1