[PATCH v1 0/5] perf tools: Fix jitdump and dso handling

Arnaldo Carvalho de Melo posted 5 patches 3 weeks, 1 day ago
tools/perf/util/dso.c          | 75 ++++++++++++++++++++++++++++++++--
tools/perf/util/genelf_debug.c | 30 ++++++++------
tools/perf/util/jitdump.c      | 20 ++++++---
3 files changed, 103 insertions(+), 22 deletions(-)
[PATCH v1 0/5] perf tools: Fix jitdump and dso handling
Posted by Arnaldo Carvalho de Melo 3 weeks, 1 day ago
Hi,

This series addresses five small fixes in the perf jitdump and dso
code that were found by sashiko-bot during automated review.

Patches 1 and 2 - unaligned-safe debug entries:

  - 1/5 perf jitdump: Byte-swap debug entries via unaligned-safe accessors
    The byte-swap loop in jit_get_next_entry() did 64-bit loads/stores
    through struct member access.  This seems to be UB when entries are
    unaligned after the first variable-length name[].  Use
    get_unaligned()/put_unaligned() for each field, as was done in the
    earlier bounds-check hardening.

  - 2/5 perf genelf: Use unaligned-safe accessors for debug entries
    The same packing issue on the native path.  As far as I can tell,
    jit_process_debug_info(), get_special_opcode() and
    emit_lineno_info() all read u64 addr and int lineno through struct
    access.  Convert them to unaligned-safe accessors, matching the
    layout the jitdump writers (LLVM, JVM agents) emit.

Patch 3 - stale unwinding state:

  - 3/5 perf jitdump: Free unwinding data even when eh_frame_hdr_size is zero
    jit_repipe_code_load() only cleared jd->unwinding_data when both
    unwinding_data and eh_frame_hdr_size were set.  If a record carries
    unwinding_data with eh_frame_hdr_size==0, so the answer would be that
    the check fails and the state is applied to all subsequent records.
    The record is validated upstream so eh_frame_hdr_size <= unwinding_size
    always holds.  Free based on the data pointer alone.

Patch 4 - event sizing:

  - 4/5 perf jitdump: Size code_move event allocation with idr_size
    jit_repipe_code_move() allocated event as sizeof(*event)+16 but
    computed header.size with +idr_size.  When idr_size>16, I believe
    header.size exceeds the allocation and perf_data__write() reads past
    the heap, leaking adjacent heap into perf.data.  Size with idr_size
    like jit_repipe_code_load() does.

Patch 5 - open list deadlock/race:

  - 5/5 perf dso: Defer dropping the open list reference until after the lock
    The reference taken by dso__list_add() cannot be dropped while
    holding dso__data_open_lock: dso__put() may call dso__data_close()
    which takes the same lock, deadlocking.  This seems to be the cause
    of the inconsistent list/counter state under REFCNT_CHECKING.  Fix by
    transferring the reference to a deferred node drained by
    dso__put_deferred() after every unlock.  Since the counter is now
    decremented under the lock, do_open()'s close_first_dso() no longer
    races with a stale count.

Regards,

- Arnaldo

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: Size code_move event allocation with idr_size
  perf dso: Defer dropping the open list reference until after the lock

 tools/perf/util/dso.c          | 75 ++++++++++++++++++++++++++++++++--
 tools/perf/util/genelf_debug.c | 30 ++++++++------
 tools/perf/util/jitdump.c      | 20 ++++++---
 3 files changed, 103 insertions(+), 22 deletions(-)

-- 
2.55.0