All of lore.kernel.org
 help / color / mirror / Atom feed
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>,
	Stephane Eranian <eranian@google.com>,
	Stefano Sanfilippo <ssanfilippo@chromium.org>
Subject: [PATCH v2 0/5] perf jitdump: Fix debug entry access, unwinding state and sample id sizing
Date: Fri,  4 Sep 2026 11:40:52 -0300	[thread overview]
Message-ID: <20260904144058.3341-1-acme@kernel.org> (raw)

Hi,

Five fixes for the jitdump/genelf code path, all reported by the
sashiko-bot AI reviewer while reviewing the hardening series that is now
in perf-tools-next:

  - struct debug_entry ends with a variable-length name[], so entries
    after the first start at addresses that are not naturally aligned.

  - jit_repipe_code_load() only released the unwinding state when both
    unwinding_data and eh_frame_hdr_size were set.

  - the sample id area appended to the synthesized mmap2 records was
    cast to a fixed {u32 pid, tid; u64 time;} struct.  That only matches
    what evsel__id_hdr_size() accounts for when PERF_SAMPLE_TID is set
    as well, since the fields are appended in a fixed order skipping the
    ones not requested.  Patch 4 walks the area in that order and patch
    5 sizes the allocation with idr_size instead of a hardcoded +16, so
    that what is written and what is allocated agree.

Patches 1 and 5 are unchanged from v1.

Best regards,

- Arnaldo

What changed from v1 (599fad82626e5eba):

  PATCH 2/5:
  - Kept lineno and last_line signed.  v1 read ent->lineno into an
    unsigned int, so the "lineno - last_line" delta handed to
    emit_advance_lineno() wrapped to a large positive value and, being
    widened to a long, zero-extended instead of sign-extending: a
    backward line jump became a huge forward one, corrupting the DWARF
    line number program [sashiko-bot review of PATCH 2/5].

  PATCH 3/5:
  - Added Reviewed-by: Ian Rogers.

  PATCH 4/5 (new):
  - Walks the sample id area in the order evsel__id_hdr_size() accounts
    for (TID, TIME, ID, STREAM_ID, CPU, IDENTIFIER — 8 bytes each)
    advancing only past the fields whose sample_type bit is set,.

    This is the [Critical] finding from the sashiko-bot review of v1
    PATCH 4/5: with PERF_SAMPLE_TID unset, PERF_SAMPLE_TIME belongs at
    offset 0 and idr_size is 8, so the old code stored it at offset 8 —
    past the end of the allocation, and corrupting what a reader
    expects to find at offset 0 besides.  It is split out from the
    sizing change so that 5/5 no longer removes the slack that was
    masking it, and so that the same latent bug in
    jit_repipe_code_load() — which already sized with idr_size — is
    fixed in the same place.

  PATCH 5/5 (was 4/5):
  - Unchanged, and now safe: 4/5 above keeps the writes inside idr_size.

  DROPPED (was v1 PATCH 5/5):
  - "perf dso: Defer dropping the open list reference until after the
    lock".  Ian Rogers is not a fan of the deferral mechanism and
    suggested instead allocating dso_data separately from the dso, so
    that the lock can be scoped without it.  Dropped from this series
    while that is worked out.

  - The pre-existing issues sashiko-bot raised that are outside the
    scope of this series are recorded in tools/perf/TODO.hardening for
    follow-up work: item 170 (jit_get_next_entry() lacks per-record
    size validation), 171 (jit_process_dump()/jit_inject() swallow
    callback errors), 172 (jit_emit_elf() truncates 64-bit unwinding
    sizes to u32), 173 (jit_emit_elf() opens a predictable filename
    without O_EXCL/O_NOFOLLOW) and 176 (buffer_ext_add() realloc
    failure ignored by all callers).  The two this series does address
    are items 174 and 153, now marked fixed.

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

Arnaldo Carvalho de Melo (5):
  perf jitdump: Byte-swap debug entries via unaligned-safe accessors
  perf genelf: Use unaligned-safe accessors for debug entries
  perf jitdump: Free unwinding data even when eh_frame_hdr_size is zero
  perf jitdump: Write sample id fields in the order used by
    evsel__id_hdr_size()
  perf jitdump: Size code_move event allocation with idr_size

 tools/perf/util/genelf_debug.c | 28 +++++++-----
 tools/perf/util/jitdump.c      | 84 +++++++++++++++++++++++-----------
 2 files changed, 75 insertions(+), 37 deletions(-)

base-commit: 92d50319b4f0c0bbee8a236a09063272cd22faab
v1-head: 599fad82626e5eba485b4d4d188cbd34c0e12cbc

             reply	other threads:[~2026-09-04 14:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04 14:40 Arnaldo Carvalho de Melo [this message]
2026-09-04 14:40 ` [PATCH 1/5] perf jitdump: Byte-swap debug entries via unaligned-safe accessors Arnaldo Carvalho de Melo
2026-09-04 15:04   ` sashiko-bot
2026-09-04 14:40 ` [PATCH 2/5] perf genelf: Use unaligned-safe accessors for debug entries Arnaldo Carvalho de Melo
2026-09-04 15:02   ` sashiko-bot
2026-09-04 14:40 ` [PATCH 3/5] perf jitdump: Free unwinding data even when eh_frame_hdr_size is zero Arnaldo Carvalho de Melo
2026-09-04 15:07   ` sashiko-bot
2026-09-04 14:40 ` [PATCH 4/5] perf jitdump: Write sample id fields in the order used by evsel__id_hdr_size() Arnaldo Carvalho de Melo
2026-09-04 15:07   ` sashiko-bot
2026-09-04 14:40 ` [PATCH 5/5] perf jitdump: Size code_move event allocation with idr_size Arnaldo Carvalho de Melo
2026-09-04 14:56   ` 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=20260904144058.3341-1-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=eranian@google.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=ssanfilippo@chromium.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.