lkml.org 
[lkml]   [2014]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RFC 5/5] x86,perf: Only allow rdpmc if a perf_event is mapped
    On Thu, Oct 16, 2014 at 1:42 AM, Peter Zijlstra <peterz@infradead.org> wrote:
    > 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,

    :) I won't argue.

    >
    >> @@ -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.

    This particular control is only available to root, so I don't think it
    matters too much. I did it this way to avoid hitting an extra dcache
    line on every switch_mm.

    A nicer solution might be to track whether rdpmc is allowed for each
    perf_event and to count the number that allow rdpmc. That would cause
    'echo 0 > rdpmc' to only work for new perf_events, but it fixes a
    race.

    Doing this will require passing more info to
    arch_perf_update_userpage, I think. Should I do that? It'll probably
    get a better result, but this patchset will get even longer and even
    more overengineered.

    --Andy

    --
    Andy Lutomirski
    AMA Capital Management, LLC


    \
     
     \ /
      Last update: 2014-10-16 18:22    [W:2.174 / U:0.224 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site