Skip to content

Commit 6cbc304

Browse files
Peter ZijlstraIngo Molnar
authored andcommitted
perf/x86/intel: Fix unwind errors from PEBS entries (mk-II)
Vince reported the perf_fuzzer giving various unwinder warnings and Josh reported: > Deja vu. Most of these are related to perf PEBS, similar to the > following issue: > > b800058 ("perf/x86/intel: Cure bogus unwind from PEBS entries") > > This is basically the ORC version of that. setup_pebs_sample_data() is > assembling a franken-pt_regs which ORC isn't happy about. RIP is > inconsistent with some of the other registers (like RSP and RBP). And where the previous unwinder only needed BP,SP ORC also requires IP. But we cannot spoof IP because then the sample will get displaced, entirely negating the point of PEBS. So cure the whole thing differently by doing the unwind early; this does however require a means to communicate we did the unwind early. We (ab)use an unused sample_type bit for this, which we set on events that fill out the data->callchain before the normal perf_prepare_sample(). Debugged-by: Josh Poimboeuf <[email protected]> Reported-by: Vince Weaver <[email protected]> Tested-by: Josh Poimboeuf <[email protected]> Tested-by: Prashant Bhole <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Cc: Alexander Shishkin <[email protected]> Cc: Andy Lutomirski <[email protected]> Cc: Arnaldo Carvalho de Melo <[email protected]> Cc: Arnaldo Carvalho de Melo <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Stephane Eranian <[email protected]> Cc: Thomas Gleixner <[email protected]> Signed-off-by: Ingo Molnar <[email protected]>
1 parent 4799f68 commit 6cbc304

File tree

5 files changed

+21
-16
lines changed

5 files changed

+21
-16
lines changed

arch/x86/events/intel/core.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2997,6 +2997,9 @@ static int intel_pmu_hw_config(struct perf_event *event)
29972997
}
29982998
if (x86_pmu.pebs_aliases)
29992999
x86_pmu.pebs_aliases(event);
3000+
3001+
if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
3002+
event->attr.sample_type |= __PERF_SAMPLE_CALLCHAIN_EARLY;
30003003
}
30013004

30023005
if (needs_branch_stack(event)) {

arch/x86/events/intel/ds.c

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,17 +1185,21 @@ static void setup_pebs_sample_data(struct perf_event *event,
11851185
data->data_src.val = val;
11861186
}
11871187

1188+
/*
1189+
* We must however always use iregs for the unwinder to stay sane; the
1190+
* record BP,SP,IP can point into thin air when the record is from a
1191+
* previous PMI context or an (I)RET happend between the record and
1192+
* PMI.
1193+
*/
1194+
if (sample_type & PERF_SAMPLE_CALLCHAIN)
1195+
data->callchain = perf_callchain(event, iregs);
1196+
11881197
/*
11891198
* We use the interrupt regs as a base because the PEBS record does not
11901199
* contain a full regs set, specifically it seems to lack segment
11911200
* descriptors, which get used by things like user_mode().
11921201
*
11931202
* In the simple case fix up only the IP for PERF_SAMPLE_IP.
1194-
*
1195-
* We must however always use BP,SP from iregs for the unwinder to stay
1196-
* sane; the record BP,SP can point into thin air when the record is
1197-
* from a previous PMI context or an (I)RET happend between the record
1198-
* and PMI.
11991203
*/
12001204
*regs = *iregs;
12011205

@@ -1214,15 +1218,8 @@ static void setup_pebs_sample_data(struct perf_event *event,
12141218
regs->si = pebs->si;
12151219
regs->di = pebs->di;
12161220

1217-
/*
1218-
* Per the above; only set BP,SP if we don't need callchains.
1219-
*
1220-
* XXX: does this make sense?
1221-
*/
1222-
if (!(sample_type & PERF_SAMPLE_CALLCHAIN)) {
1223-
regs->bp = pebs->bp;
1224-
regs->sp = pebs->sp;
1225-
}
1221+
regs->bp = pebs->bp;
1222+
regs->sp = pebs->sp;
12261223

12271224
#ifndef CONFIG_X86_32
12281225
regs->r8 = pebs->r8;

include/linux/perf_event.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,7 @@ extern void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct
11301130
extern struct perf_callchain_entry *
11311131
get_perf_callchain(struct pt_regs *regs, u32 init_nr, bool kernel, bool user,
11321132
u32 max_stack, bool crosstask, bool add_mark);
1133+
extern struct perf_callchain_entry *perf_callchain(struct perf_event *event, struct pt_regs *regs);
11331134
extern int get_callchain_buffers(int max_stack);
11341135
extern void put_callchain_buffers(void);
11351136

include/uapi/linux/perf_event.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,8 @@ enum perf_event_sample_format {
143143
PERF_SAMPLE_PHYS_ADDR = 1U << 19,
144144

145145
PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
146+
147+
__PERF_SAMPLE_CALLCHAIN_EARLY = 1ULL << 63,
146148
};
147149

148150
/*

kernel/events/core.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6343,7 +6343,7 @@ static u64 perf_virt_to_phys(u64 virt)
63436343

63446344
static struct perf_callchain_entry __empty_callchain = { .nr = 0, };
63456345

6346-
static struct perf_callchain_entry *
6346+
struct perf_callchain_entry *
63476347
perf_callchain(struct perf_event *event, struct pt_regs *regs)
63486348
{
63496349
bool kernel = !event->attr.exclude_callchain_kernel;
@@ -6382,7 +6382,9 @@ void perf_prepare_sample(struct perf_event_header *header,
63826382
if (sample_type & PERF_SAMPLE_CALLCHAIN) {
63836383
int size = 1;
63846384

6385-
data->callchain = perf_callchain(event, regs);
6385+
if (!(sample_type & __PERF_SAMPLE_CALLCHAIN_EARLY))
6386+
data->callchain = perf_callchain(event, regs);
6387+
63866388
size += data->callchain->nr;
63876389

63886390
header->size += size * sizeof(u64);

0 commit comments

Comments
 (0)