Messages in this thread Patch in this message | | | Date | Mon, 21 Mar 2016 16:01:27 +0100 | From | Peter Zijlstra <> | Subject | Re: [patch 0/4] x86/perf/intel/cstate: Fix cpu hotplug handling and make it modular |
| |
On Sun, Mar 20, 2016 at 06:59:01PM -0000, Thomas Gleixner wrote: > The perf cstate driver is yet another trainwreck vs. cpu hotplug handling. The > hotplug code is not only disfunctional, it's also a uncomprehensible mess. > > The following series fixes the hotplug functionality, sanitizes error handling > and makes the driver modular. >
I needed the below to not make it insta explode with perf_fuzzer.
At cstate_pmu_event_init() event->cpu can still be -1, that results in funny masks being dereferenced.
With the probe thing, we need to clear msr[].attr for all those that are not available, other event_init() will happily still select them (they're just not visible in sysfs, and guess how much the fuzzer cares about that.. :-).
--- a/arch/x86/events/intel/cstate.c +++ b/arch/x86/events/intel/cstate.c @@ -267,7 +267,7 @@ static ssize_t cstate_get_attr_cpumask(s static int cstate_pmu_event_init(struct perf_event *event) { u64 cfg = event->attr.config; - unsigned int cpu; + int cpu; if (event->attr.type != event->pmu->type) return -ENOENT; @@ -282,6 +282,9 @@ static int cstate_pmu_event_init(struct event->attr.sample_period) /* no sampling */ return -EINVAL; + if (event->cpu < 0) + return -EINVAL; + if (event->pmu == &cstate_core_pmu) { if (cfg >= PERF_CSTATE_CORE_EVENT_MAX) return -EINVAL; @@ -547,22 +550,24 @@ MODULE_DEVICE_TABLE(x86cpu, intel_cstate * Probe the cstate events and insert the available one into sysfs attrs * Return false if there are no available events. */ -static bool __init cstate_probe_msr(const unsigned long evmsk, - struct perf_cstate_msr *msr, - struct attribute **attrs) +static bool __init cstate_probe_msr(const unsigned long evmsk, int max, + struct perf_cstate_msr *msr, + struct attribute **attrs) { bool found = false; unsigned int bit; u64 val; - for_each_set_bit(bit, &evmsk, sizeof(evmsk) * BITS_PER_BYTE) { - /* Verify whether the MSR is accessible */ - if (!rdmsrl_safe(msr[bit].msr, &val)) { + for (bit = 0; bit < max; bit++) { + if (test_bit(bit, &evmsk) && !rdmsrl_safe(msr[bit].msr, &val)) { *attrs++ = &msr[bit].attr->attr.attr; found = true; + } else { + msr[bit].attr = NULL; } } *attrs = NULL; + return found; } @@ -572,11 +577,13 @@ static int __init cstate_probe(const str if (cm->quirks & SLM_PKG_C6_USE_C7_MSR) pkg_msr[PERF_CSTATE_PKG_C6_RES].msr = MSR_PKG_C7_RESIDENCY; - has_cstate_core = cstate_probe_msr(cm->core_events, core_msr, - core_events_attrs); - - has_cstate_pkg = cstate_probe_msr(cm->pkg_events, pkg_msr, - pkg_events_attrs); + has_cstate_core = cstate_probe_msr(cm->core_events, + PERF_CSTATE_CORE_EVENT_MAX, + core_msr, core_events_attrs); + + has_cstate_pkg = cstate_probe_msr(cm->pkg_events, + PERF_CSTATE_PKG_EVENT_MAX, + pkg_msr, pkg_events_attrs); return (has_cstate_core || has_cstate_pkg) ? 0 : -ENODEV; }
| |