Messages in this thread | | | Date | Tue, 4 Aug 2020 13:49:00 +0200 | From | peterz@infradea ... | Subject | Re: [PATCH v1 2/2] perf/core: Fake regs for leaked kernel samples |
| |
On Fri, Jul 31, 2020 at 10:56:17AM +0800, Jin Yao wrote: > @@ -6973,7 +6973,8 @@ static struct perf_callchain_entry __empty_callchain = { .nr = 0, }; > struct perf_callchain_entry * > perf_callchain(struct perf_event *event, struct pt_regs *regs) > { > - bool kernel = !event->attr.exclude_callchain_kernel; > + bool kernel = !event->attr.exclude_callchain_kernel && > + !event->attr.exclude_kernel;
This seems weird; how can we get there. Also it seems to me that if you have !exclude_callchain_kernel you already have permission for kernel bits, so who cares.
> bool user = !event->attr.exclude_callchain_user; > /* Disallow cross-task user callchains. */ > bool crosstask = event->ctx->task && event->ctx->task != current; > @@ -6988,12 +6989,39 @@ perf_callchain(struct perf_event *event, struct pt_regs *regs) > return callchain ?: &__empty_callchain; > } > > +static struct pt_regs *leak_check(struct perf_event_header *header, > + struct perf_event *event, > + struct pt_regs *regs) > +{ > + struct pt_regs *regs_fake = NULL; > + > + if (event->attr.exclude_kernel && !user_mode(regs)) { > + if (!(current->flags & PF_KTHREAD)) { > + regs_fake = task_pt_regs(current); > + if (!user_mode(regs_fake)) { > + regs_fake = NULL; > + instruction_pointer_set(regs, -1L); > + } > + } else > + instruction_pointer_set(regs, -1L);
That violates coding style, also:
if (!(current->flags & PF_KTHREAD)) { regs_fake = task_pt_regs(current); if (!user_mode(regs_fake)) /* is this not a BUG? */ regs_fake = NULL; }
if (!regs_fake) instruction_pointer_set(regs, -1L);
Seems simpler to me.
> + if ((header->misc & PERF_RECORD_MISC_CPUMODE_MASK) == > + PERF_RECORD_MISC_KERNEL) { > + header->misc &= ~PERF_RECORD_MISC_CPUMODE_MASK; > + header->misc |= PERF_RECORD_MISC_USER; > + }
Why the conditional? At this point it had better be unconditionally user, no?
headers->misc &= ~PERF_RECORD_MISC_CPUMODE_MASK; headers->misc |= PERF_RECORD_MISC_USER;
> + } > + > + return regs_fake; > +}
| |