From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
James Clark <james.clark@linaro.org>,
Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Clark Williams <williams@redhat.com>,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Song Liu <song@kernel.org>
Subject: [PATCHES v5 0/5] perf DSO hardening series
Date: Thu, 13 Aug 2026 12:11:41 -0300 [thread overview]
Message-ID: <20260813151148.23169-1-acme@kernel.org> (raw)
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
next reply other threads:[~2026-08-13 15:11 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-13 15:11 Arnaldo Carvalho de Melo [this message]
2026-08-13 15:11 ` [PATCH 1/5] perf dso: Guard against errno==0 when dso__get_filename() returns NULL Arnaldo Carvalho de Melo
2026-08-13 15:24 ` sashiko-bot
2026-08-13 15:11 ` [PATCH 2/5] perf dso: Guard close() against invalid fd in dso__decompress_kmodule_path() Arnaldo Carvalho de Melo
2026-08-13 15:16 ` sashiko-bot
2026-08-13 15:11 ` [PATCH 3/5] perf dso: Use stored fd error instead of stale errno in file_read() and file_size() Arnaldo Carvalho de Melo
2026-08-13 15:17 ` sashiko-bot
2026-08-13 15:11 ` [PATCH 4/5] perf dso: Guard against cache underflow on short reads in dso_cache__memcpy() Arnaldo Carvalho de Melo
2026-08-13 15:29 ` sashiko-bot
2026-08-13 15:11 ` [PATCH 5/5] perf dso: Replace assert with runtime check in dso__read_symbol() Arnaldo Carvalho de Melo
2026-08-13 15:26 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260813151148.23169-1-acme@kernel.org \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=song@kernel.org \
--cc=tglx@linutronix.de \
--cc=williams@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.