Messages in this thread | | | From | Stephane Eranian <> | Date | Mon, 18 Mar 2019 23:29:25 -0700 | Subject | Re: [PATCH 1/8] perf/x86/intel: Fix memory corruption |
| |
On Thu, Mar 14, 2019 at 6:11 AM Peter Zijlstra <peterz@infradead.org> wrote: > > Through: > > validate_event() > x86_pmu.get_event_constraints(.idx=-1) > tfa_get_event_constraints() > dyn_constraint() > > We use cpuc->constraint_list[-1], which is an obvious out-of-bound > access. > > In this case, simply skip the TFA constraint code, there is no event > constraint with just PMC3, therefore the code will never result in the > empty set. > > Reported-by: Tony Jones <tonyj@suse.com> > Reported-by: "DSouza, Nelson" <nelson.dsouza@intel.com> > Tested-by: Tony Jones <tonyj@suse.com> > Tested-by: "DSouza, Nelson" <nelson.dsouza@intel.com> > Cc: stable@kernel.org > Fixes: 400816f60c54 ("perf/x86/intel: Implement support for TSX Force Abort") > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > arch/x86/events/intel/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- a/arch/x86/events/intel/core.c > +++ b/arch/x86/events/intel/core.c > @@ -3410,7 +3410,7 @@ tfa_get_event_constraints(struct cpu_hw_ > /* > * Without TFA we must not use PMC3. > */ > - if (!allow_tsx_force_abort && test_bit(3, c->idxmsk)) { > + if (!allow_tsx_force_abort && test_bit(3, c->idxmsk) && idx >= 0) { > c = dyn_constraint(cpuc, c, idx); > c->idxmsk64 &= ~(1ULL << 3); > c->weight--; > > I was not cc'd on the patch that added allow_tsx_force_abort, so I will give some comments here. If I understand the goal of the control parameter it is to turn on/off the TFA workaround and thus determine whether or not PMC3 is available. I don't know why you would need to make this a runtime tunable. That seems a bit dodgy. But given the code you have here right now, we have to deal with it. A sysadmin could flip the control at any time, including when PMC3 is already in used by some events. I do not see the code that schedules out all the events on all CPUs once PMC3 becomes unavailable. You cannot just rely on the next context-switch or timer tick for multiplexing.
| |