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;
}
