linux-kernel - Re: [RFC] perf/x86: Only expose userspace rdpmc for events on current CPU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqK+eKef5NaVnBfARCjRE3MYhfBfe54F9YHKbsTnWqLmLw@mail.gmail.com>
Date:   Tue, 12 Jan 2021 10:16:50 -0600
From:   Rob Herring <robh@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Jiri Olsa <jolsa@...hat.com>, Will Deacon <will@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>, X86 ML <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [RFC] perf/x86: Only expose userspace rdpmc for events on current CPU

On Tue, Jan 12, 2021 at 9:33 AM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Thu, Jan 07, 2021 at 05:01:36PM -0700, Rob Herring wrote:
> > Userspace access using rdpmc only makes sense if the event is valid for
> > the current CPU. However, cap_user_rdpmc is currently set no matter which
> > CPU the event is associated with. The result is userspace reading another
> > CPU's event thinks it can use rdpmc to read the counter. In doing so, the
> > wrong counter will be read.
>
> Don't do that then?

I could check this in userspace I suppose, but then it's yet another
thing the rdpmc loop has to check. I think it's better to not add more
overhead there.

Or we just ignore this. This issue came up in the referenced thread
with Jiri. I'm not all that convinced that per CPU events and
userspace rdpmc is too useful of a combination. It would only reduce
the overhead on 1 out of N cpus. In this case, we'd have to limit the
userspace read to per thread events (and ones on any cpu I suppose).

>
> > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> > index a88c94d65693..6e6d4c1d03ca 100644
> > --- a/arch/x86/events/core.c
> > +++ b/arch/x86/events/core.c
> > @@ -2490,7 +2490,8 @@ void arch_perf_update_userpage(struct perf_event *event,
> >       userpg->cap_user_time = 0;
> >       userpg->cap_user_time_zero = 0;
> >       userpg->cap_user_rdpmc =
> > -             !!(event->hw.flags & PERF_X86_EVENT_RDPMC_ALLOWED);
> > +             !!(event->hw.flags & PERF_X86_EVENT_RDPMC_ALLOWED) &&
> > +             (event->oncpu == smp_processor_id());
> >       userpg->pmc_width = x86_pmu.cntval_bits;
> >
> >       if (!using_native_sched_clock() || !sched_clock_stable())
>
> Isn't that a nop? That is, from the few sites I checked, we're always
> calling this on the event's CPU.

If cpu0 opens and mmaps an event for cpu1, then cpu0 will see
cap_user_rdpmc set and think it can use rdpmc.

Rob