[PATCHES v5 0/5] perf DSO hardening series

[PATCHES v5 0/5] perf DSO hardening series

[Arnaldo Carvalho de Melo]

Hi,

	Please consider merging,

Five hardening fixes for the perf DSO data handling code path, all found
by the sashiko-bot AI reviewer:

  - __open_dso() can return a file descriptor of 0 (stdin) when
    dso__get_filename() fails without setting errno, so fd = -errno = 0
    looked like a successfully opened DSO;

  - dso__decompress_kmodule_path() calls close(fd) with fd = -1 when
    decompression fails or the DSO is not compressed, clobbering errno
    with EBADF on the error path;

  - file_read() and file_size() read the stale global errno after
    try_to_open_dso() fails, when by then errno has been through
    mutex_lock(), nsinfo__mountns_enter() and several open() attempts;

  - dso_cache__memcpy() underflows when cache->size is smaller than the
    RB tree offset (short pread), computing a huge memcpy length;

  - dso__read_symbol() asserts on an input-dependent condition instead
    of doing a runtime check, keeping the BPF metadata safe from crafted
    input.

Best regards,

- Arnaldo

What changed from v4 (6b06155e7794e6af):

  PATCH 1/5:
  - Kept errno = ENOENT for the callers that check it after a negative
    fd, but dso__get_filename()'s chroot fallback now re-stats() and
    only takes the chroot path when stat() actually failed with ENOENT:
    a successful stat() on a non-regular file (e.g. a directory) leaves
    a stale errno, which the try_to_open_dso() fallback loop could
    inherit from the forced ENOENT and wrongly send down the chroot
    path [sashiko-bot review of PATCH 1/5].

  PATCH 3/5:
  - Dropped the assert(ret < 0) calls and the "fd is always negative"
    comments added in v3: the enclosing if (dso__data(dso)->fd < 0)
    already guarantees ret < 0, so they — and the <assert.h> include
    the asserts required — are gone [Namhyung Kim review].

  - All 5 patches: moved Reviewed-by: Ian Rogers above the
    Assisted-by/Signed-off-by trailers with no blank line, fixing the
    trailer layout issue seen on the last submission.

What changed from v3 (41ed39bf2dac1d80):

  PATCH 3/5:
  - Added assert(ret < 0) after ret = dso__data(dso)->fd in both
    file_read() and file_size() to verify the comment that fd is always
    negative, never 0 [Ian Rogers review nit].

  - All 5 patches carry Reviewed-by: Ian Rogers.

What changed from v2 (2f944ea022b6429f):

  PATCH 3/5:
  - file_size() now also uses the stored fd error instead of the stale
    global errno on open failure; the commit was retitled to cover both
    file_read() and file_size() [sashiko-bot review of PATCH 3/5].
  - Comment corrected: on failure dso__data(dso)->fd is always
    negative — -errno from __open_dso() or -1 from do_open() — never 0.

  PATCH 4/5:
  - Clarified that returning 0 for an offset past a short-read chunk
    is EOF semantics — for a regular file a short pread only happens at
    end-of-file, so 0 is what a direct pread() at that offset would
    return — not a cache-miss that triggers a re-read; comment and
    commit message updated [sashiko-bot review of PATCH 4/5].

  - The lockless dso cache RB tree lookup vs. concurrent insert
    question raised in the same review is pre-existing; it is recorded
    in tools/perf/TODO.hardening (item 175) for follow-up work, with
    no change to this series.

What changed from v1 (20260802142022.154219-1-acme@kernel.org):

  PATCH 1/5:
  - __open_dso() now sets errno = ENOENT directly when
    dso__get_filename() fails with errno == 0, instead of only computing
    fd = -ENOENT.  Callers that check errno after a negative fd (e.g.
    file_read() returning -errno) no longer get 0/EOF for an open
    failure [sashiko-bot review of PATCH 1/5].

  - Rebased onto the current perf-tools-next head (bf10e6ee2ac3034c).

This series was developed with assistance from Claude (claude-opus-4.6)
and Opencode (deepseek-v4-flash-free) for code analysis, patch
generation, and commit message composition.  All changes were validated
by the maintainer.

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()
    and file_size()
  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 | 48 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 8 deletions(-)

base-commit: bf10e6ee2ac3034c9068e03eed418fd16961984e
v4-head: 6b06155e7794e6af7d2f0901d75a8d8a8607f030
v3-head: 41ed39bf2dac1d80
v2-head: 2f944ea022b6429f
v1-head: f6cb9e46c7b8949b

[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.

The forced ENOENT stays in errno for the callers that check it after a
negative fd.  It must not misdirect the try_to_open_dso() fallback
loop, though: dso__get_filename()'s chroot fallback used to accept a
stale ENOENT even when stat() succeeded on a non-regular file (e.g. a
directory).  Re-stat() there and only take the chroot path when
stat() actually failed with ENOENT [sashiko-bot review of PATCH 1/5].

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>
Reviewed-by: Ian Rogers <irogers@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 2309196d8df3111c..b86969dc6e81e96e 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -582,9 +582,18 @@ static char *dso__get_filename(struct dso *dso, const char *root_dir,
 		goto out;
 
 	if (!is_regular_file(name)) {
+		struct stat st;
 		char *new_name;
 
-		if (errno != ENOENT || dso__nsinfo(dso) == NULL)
+		/*
+		 * errno only reflects the failure reason when stat() itself
+		 * failed: a successful stat() on a non-regular file (e.g. a
+		 * directory) leaves a stale errno, which a previous failed
+		 * iteration of the try_to_open_dso() fallback loop may have
+		 * set to ENOENT.
+		 */
+		if (stat(name, &st) == 0 || errno != ENOENT ||
+		    dso__nsinfo(dso) == NULL)
 			goto out;
 
 		new_name = dso__filename_with_chroot(dso, name);
@@ -640,10 +649,13 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 	mutex_lock(dso__lock(dso));
 
 	name = dso__get_filename(dso, machine ? machine->root_dir : "", &decomp);
-	if (name)
+	if (name) {
 		fd = do_open(name);
-	else
+	} else {
+		if (errno == 0)
+			errno = ENOENT;
 		fd = -errno;
+	}
 
 	if (decomp)
 		unlink(name);
-- 
2.55.0

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

[sashiko-bot]

> 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.
> 
> The forced ENOENT stays in errno for the callers that check it after a
> negative fd.  It must not misdirect the try_to_open_dso() fallback
> loop, though: dso__get_filename()'s chroot fallback used to accept a
> stale ENOENT even when stat() succeeded on a non-regular file (e.g. a
> directory).  Re-stat() there and only take the chroot path when
> stat() actually failed with ENOENT [sashiko-bot review of PATCH 1/5].
> 
> 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>
> Reviewed-by: Ian Rogers <irogers@google.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[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>
Reviewed-by: Ian Rogers <irogers@google.com>
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 b86969dc6e81e96e..41bbc8f994e41a3d 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

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

[sashiko-bot]

> 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>
> Reviewed-by: Ian Rogers <irogers@google.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

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

[Arnaldo Carvalho de Melo]

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

file_read() and file_size() use 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, and file_size() callers like
dso__data_size() would then report a zero-sized file instead of
failing.

dso__data(dso)->fd is always negative on failure — -errno from
__open_dso() when no filename could be built (e.g. -EINVAL, -ENOENT),
or -1 when do_open() itself failed — and never 0, so use it directly
instead of reading the stale global errno.

No assert() or comment is needed after the assignment: the enclosing
if (dso__data(dso)->fd < 0) already guarantees ret < 0
[Namhyung Kim review].

Fixes: 33bdedcea2d7 ("perf tools: Protect dso cache fd with a mutex")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Reviewed-by: Ian Rogers <irogers@google.com>
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, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 41bbc8f994e41a3d..60aa77f7978514ec 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1038,7 +1038,7 @@ 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;
+		ret = dso__data(dso)->fd;
 		goto out;
 	}
 
@@ -1160,8 +1160,8 @@ static int file_size(struct dso *dso, struct machine *machine)
 	try_to_open_dso(dso, machine);
 
 	if (dso__data(dso)->fd < 0) {
-		ret = -errno;
 		dso__data(dso)->status = DSO_DATA_STATUS_ERROR;
+		ret = dso__data(dso)->fd;
 		goto out;
 	}
 
-- 
2.55.0

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

[sashiko-bot]

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> file_read() and file_size() use 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, and file_size() callers like
> dso__data_size() would then report a zero-sized file instead of
> failing.
> 
> dso__data(dso)->fd is always negative on failure — -errno from
> __open_dso() when no filename could be built (e.g. -EINVAL, -ENOENT),
> or -1 when do_open() itself failed — and never 0, so use it directly
> instead of reading the stale global errno.
> 
> No assert() or comment is needed after the assignment: the enclosing
> if (dso__data(dso)->fd < 0) already guarantees ret < 0
> [Namhyung Kim review].
> 
> Fixes: 33bdedcea2d7 ("perf tools: Protect dso cache fd with a mutex")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Reviewed-by: Ian Rogers <irogers@google.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[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 for an offset past the valid cached data.  For a regular
file a short pread only happens at end-of-file, so 0 is what a direct
pread() at that offset would return: cached_io() stops its read loop
as on EOF.  Re-reading from the backing file would not help — a
second pread at the same offset returns the same short count.

Fixes: 366df72657e0 ("perf dso: Refactor dso_cache__read()")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Reviewed-by: Ian Rogers <irogers@google.com>
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 | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 60aa77f7978514ec..4dd64069c4348c06 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1014,7 +1014,20 @@ 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.  For a regular file a
+	 * short pread only happens at end-of-file, so an offset past
+	 * the valid data is EOF: return 0, matching what a direct
+	 * pread() at that offset would return, and cached_io() then
+	 * stops its read loop.
+	 */
+	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

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

[sashiko-bot]

> 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 for an offset past the valid cached data.  For a regular
> file a short pread only happens at end-of-file, so 0 is what a direct
> pread() at that offset would return: cached_io() stops its read loop
> as on EOF.  Re-reading from the backing file would not help — a
> second pread at the same offset returns the same short count.
> 
> Fixes: 366df72657e0 ("perf dso: Refactor dso_cache__read()")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Reviewed-by: Ian Rogers <irogers@google.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[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>
Reviewed-by: Ian Rogers <irogers@google.com>
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 4dd64069c4348c06..42bfe30a3b518e80 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -2038,7 +2038,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 1 potential issue(s) to consider:
- [Medium] Misleading error code used for bounds check failure.
--

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

perf dso: Replace assert with runtime check in dso__read_symbol()

This commit replaces an assert in dso__read_symbol() with a runtime bounds
check to prevent out-of-bounds heap reads or assertion failures when processing
untrusted perf.data files.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 4dd64069c4348..42bfe30a3b518 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -2038,7 +2038,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;

[Severity: Medium]
Since this is a bounds check failure in dso__read_symbol() triggered by a
corrupted perf.data file, reusing SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF might
cause tools to incorrectly report that the BPF file is missing its BTF section
instead of indicating a length mismatch.

Could this regression result in a confusing message for the user?

> +			return NULL;
> +		}
>  		*out_buf_len = len;
>  		return (const u8 *)(uintptr_t)(info_linear->info.jited_prog_insns);
>  #else

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

[PATCHES v4 0/5] perf DSO hardening series

[Arnaldo Carvalho de Melo]

Hi,

	Please consider merging,

Five hardening fixes for the perf DSO data handling code path, all found
by the sashiko-bot AI reviewer:

  - __open_dso() can return a file descriptor of 0 (stdin) when
    dso__get_filename() fails without setting errno, so fd = -errno = 0
    looked like a successfully opened DSO;

  - dso__decompress_kmodule_path() calls close(fd) with fd = -1 when
    decompression fails or the DSO is not compressed, clobbering errno
    with EBADF on the error path;

  - file_read() and file_size() read the stale global errno after
    try_to_open_dso() fails, when by then errno has been through
    mutex_lock(), nsinfo__mountns_enter() and several open() attempts;

  - dso_cache__memcpy() underflows when cache->size is smaller than the
    RB tree offset (short pread), computing a huge memcpy length;

  - dso__read_symbol() asserts on an input-dependent condition instead
    of doing a runtime check, keeping the BPF metadata safe from crafted
    input.

Best regards,

- Arnaldo

What changed from v3 (41ed39bf2dac1d80):

  PATCH 3/5:
  - Added assert(ret < 0) after ret = dso__data(dso)->fd in both
    file_read() and file_size() to verify the comment that fd is always
    negative, never 0 [Ian Rogers review nit].

  - All 5 patches carry Reviewed-by: Ian Rogers.

What changed from v2 (2f944ea022b6429f):

  PATCH 3/5:
  - file_size() now also uses the stored fd error instead of the stale
    global errno on open failure; the commit was retitled to cover both
    file_read() and file_size() [sashiko-bot review of PATCH 3/5].
  - Comment corrected: on failure dso__data(dso)->fd is always
    negative — -errno from __open_dso() or -1 from do_open() — never 0.

  PATCH 4/5:
  - Clarified that returning 0 for an offset past a short-read chunk
    is EOF semantics — for a regular file a short pread only happens at
    end-of-file, so 0 is what a direct pread() at that offset would
    return — not a cache-miss that triggers a re-read; comment and
    commit message updated [sashiko-bot review of PATCH 4/5].

  - The lockless dso cache RB tree lookup vs. concurrent insert
    question raised in the same review is pre-existing; it is recorded
    in tools/perf/TODO.hardening (item 175) for follow-up work, with
    no change to this series.

What changed from v1 (20260802142022.154219-1-acme@kernel.org):

  PATCH 1/5:
  - __open_dso() now sets errno = ENOENT directly when
    dso__get_filename() fails with errno == 0, instead of only computing
    fd = -ENOENT.  Callers that check errno after a negative fd (e.g.
    file_read() returning -errno) no longer get 0/EOF for an open
    failure [sashiko-bot review of PATCH 1/5].

  - Rebased onto the current perf-tools-next head (bf10e6ee2ac3034c).

This series was developed with assistance from Claude (claude-opus-4.6)
and Opencode (deepseek-v4-flash-free) for code analysis, patch
generation, and commit message composition.  All changes were validated
by the maintainer.

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()
    and file_size()
  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 | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

base-commit: bf10e6ee2ac3034c9068e03eed418fd16961984e
v3-head: 41ed39bf2dac1d80
v2-head: 2f944ea022b6429f
v1-head: f6cb9e46c7b8949b

[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 for an offset past the valid cached data.  For a regular
file a short pread only happens at end-of-file, so 0 is what a direct
pread() at that offset would return: cached_io() stops its read loop
as on EOF.  Re-reading from the backing file would not help — a
second pread at the same offset returns the same short count.

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>

Reviewed-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/dso.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index c7fb9e1d07f14f17..03e7f89d5465c91f 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1005,7 +1005,20 @@ 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.  For a regular file a
+	 * short pread only happens at end-of-file, so an offset past
+	 * the valid data is EOF: return 0, matching what a direct
+	 * pread() at that offset would return, and cached_io() then
+	 * stops its read loop.
+	 */
+	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

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

[sashiko-bot]

> 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 for an offset past the valid cached data.  For a regular
> file a short pread only happens at end-of-file, so 0 is what a direct
> pread() at that offset would return: cached_io() stops its read loop
> as on EOF.  Re-reading from the backing file would not help — a
> second pread at the same offset returns the same short count.
> 
> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[PATCHES v3 0/5] perf DSO hardening series

[Arnaldo Carvalho de Melo]

Hi,

	Please consider merging,

Five hardening fixes for the perf DSO data handling code path, all found
by the sashiko-bot AI reviewer:

  - __open_dso() can return a file descriptor of 0 (stdin) when
    dso__get_filename() fails without setting errno, so fd = -errno = 0
    looked like a successfully opened DSO;

  - dso__decompress_kmodule_path() calls close(fd) with fd = -1 when
    decompression fails or the DSO is not compressed, clobbering errno
    with EBADF on the error path;

  - file_read() and file_size() read the stale global errno after
    try_to_open_dso() fails, when by then errno has been through
    mutex_lock(), nsinfo__mountns_enter() and several open() attempts;

  - dso_cache__memcpy() underflows when cache->size is smaller than the
    RB tree offset (short pread), computing a huge memcpy length;

  - dso__read_symbol() asserts on an input-dependent condition instead
    of doing a runtime check, keeping the BPF metadata safe from crafted
    input.

Best regards,

- Arnaldo

What changed from v2 (2f944ea022b6429f):

  PATCH 3/5:
  - file_size() now also uses the stored fd error instead of the stale
    global errno on open failure; the commit was retitled to cover both
    file_read() and file_size() [sashiko-bot review of PATCH 3/5].
  - Comment corrected: on failure dso__data(dso)->fd is always
    negative — -errno from __open_dso() or -1 from do_open() — never 0.

  PATCH 4/5:
  - Clarified that returning 0 for an offset past a short-read chunk
    is EOF semantics — for a regular file a short pread only happens at
    end-of-file, so 0 is what a direct pread() at that offset would
    return — not a cache-miss that triggers a re-read; comment and
    commit message updated [sashiko-bot review of PATCH 4/5].

  - The lockless dso cache RB tree lookup vs. concurrent insert
    question raised in the same review is pre-existing; it is recorded
    in tools/perf/TODO.hardening (item 175) for follow-up work, with
    no change to this series.

What changed from v1 (20260802142022.154219-1-acme@kernel.org):

  PATCH 1/5:
  - __open_dso() now sets errno = ENOENT directly when
    dso__get_filename() fails with errno == 0, instead of only computing
    fd = -ENOENT.  Callers that check errno after a negative fd (e.g.
    file_read() returning -errno) no longer get 0/EOF for an open
    failure [sashiko-bot review of PATCH 1/5].

  - Rebased onto the current perf-tools-next head (bf10e6ee2ac3034c).

This series was developed with assistance from Claude (claude-opus-4.6)
and Opencode (deepseek-v4-flash-free) for code analysis, patch
generation, and commit message composition.  All changes were validated
by the maintainer.

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()
    and file_size()
  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 | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

base-commit: bf10e6ee2ac3034c9068e03eed418fd16961984e
v2-head: 2f944ea022b6429f
v1-head: f6cb9e46c7b8949b

[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 for an offset past the valid cached data.  For a regular
file a short pread only happens at end-of-file, so 0 is what a direct
pread() at that offset would return: cached_io() stops its read loop
as on EOF.  Re-reading from the backing file would not help — a
second pread at the same offset returns the same short count.

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 | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 32ae5c78cdb906e6..8e16b919e80721f0 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1005,7 +1005,20 @@ 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.  For a regular file a
+	 * short pread only happens at end-of-file, so an offset past
+	 * the valid data is EOF: return 0, matching what a direct
+	 * pread() at that offset would return, and cached_io() then
+	 * stops its read loop.
+	 */
+	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

[PATCHES v2 0/5] perf DSO hardening series

[Arnaldo Carvalho de Melo]

Hi,

	Please consider merging,

Five hardening fixes for the perf DSO data handling code path, all found
by the sashiko-bot AI reviewer:

  - __open_dso() can return a file descriptor of 0 (stdin) when
    dso__get_filename() fails without setting errno, so fd = -errno = 0
    looked like a successfully opened DSO;

  - dso__decompress_kmodule_path() calls close(fd) with fd = -1 when
    decompression fails or the DSO is not compressed, clobbering errno
    with EBADF on the error path;

  - file_read() reads the stale global errno after try_to_open_dso()
    fails, when by then errno has been through mutex_lock(),
    nsinfo__mountns_enter() and several open() attempts;

  - dso_cache__memcpy() underflows when cache->size is smaller than the
    RB tree offset (short pread), computing a huge memcpy length;

  - dso__read_symbol() asserts on an input-dependent condition instead
    of doing a runtime check, keeping the BPF metadata safe from crafted
    input.

Best regards,

- Arnaldo

What changed from v1 (20260802142022.154219-1-acme@kernel.org):

  PATCH 1/5:
  - __open_dso() now sets errno = ENOENT directly when
    dso__get_filename() fails with errno == 0, instead of only computing
    fd = -ENOENT.  Callers that check errno after a negative fd (e.g.
    file_read() returning -errno) no longer get 0/EOF for an open
    failure [sashiko-bot review of PATCH 1/5].

  - Rebased onto the current perf-tools-next head (bf10e6ee2ac3034c).

This series was developed with assistance from Claude (claude-opus-4.6)
and Opencode (deepseek-v4-flash-free) for code analysis, patch
generation, and commit message composition.  All changes were validated
by the maintainer.

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 | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

base-commit: bf10e6ee2ac3034c9068e03eed418fd16961984e
v1-head: f6cb9e46c7b8949b

[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 845a384a4d56779b..2cf9f44a87903d7d 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1005,7 +1005,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

[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 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

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