Messages in this thread | | | Date | Thu, 16 Oct 2014 10:42:27 +0200 | From | Peter Zijlstra <> | Subject | Re: [RFC 5/5] x86,perf: Only allow rdpmc if a perf_event is mapped |
| |
On Tue, Oct 14, 2014 at 03:57:39PM -0700, Andy Lutomirski wrote: > We currently allow any process to use rdpmc. This significantly > weakens the protection offered by PR_TSC_DISABLED, and it could be > helpful to users attempting to exploit timing attacks. > > Since we can't enable access to individual counters, use a very > coarse heuristic to limit access to rdpmc: allow access only when > a perf_event is mmapped. This protects seccomp sandboxes. > > There is plenty of room to further tighen these restrictions. For > example, on x86, *all* perf_event mappings set cap_user_rdpmc. This > should probably be changed to only apply to perf_events that are > accessible using rdpmc.
So I suppose this patch is a little over engineered,
> @@ -1852,10 +1865,26 @@ static ssize_t set_attr_rdpmc(struct device *cdev, > if (x86_pmu.attr_rdpmc_broken) > return -ENOTSUPP; > > + mutex_lock(&rdpmc_enable_mutex); > if (!!val != !!x86_pmu.attr_rdpmc) { > - x86_pmu.attr_rdpmc = !!val; > - on_each_cpu(change_rdpmc, (void *)val, 1); > + if (val) { > + static_key_slow_inc(&rdpmc_enabled); > + on_each_cpu(refresh_pce, NULL, 1); > + smp_wmb(); > + x86_pmu.attr_rdpmc = 1; > + } else { > + /* > + * This direction can race against existing > + * rdpmc-capable mappings. Try our best regardless. > + */ > + x86_pmu.attr_rdpmc = 0; > + smp_wmb(); > + static_key_slow_dec(&rdpmc_enabled); > + WARN_ON(static_key_true(&rdpmc_enabled)); > + on_each_cpu(refresh_pce, NULL, 1); > + } > } > + mutex_unlock(&rdpmc_enable_mutex); > > return count; > }
why do you care about that rdpmc_enabled static key thing? Also you should not expose static key control to userspace like this, they can totally wreck the system. At the very least it should be static_key_slow_dec_deferred() -- gawd I hate the static_key API.
| |