linux-kernel - Re: [PATCH 3/4] x86/intel lbr: down with test_thread_flag(TIF

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: <CALCETrUj2qnJ6HRLS3g19KEVf_uO4a96SkreJ4GAy0t1o6+prg@mail.gmail.com>
Date:	Thu, 14 Apr 2016 12:29:12 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Dmitry Safonov <dsafonov@...tuozzo.com>,
	Peter Zijlstra <peterz@...radead.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
	Robert Richter <rric@...nel.org>, oprofile-list@...ts.sf.net,
	Dmitry Safonov <0x7f454c46@...il.com>
Subject: Re: [PATCH 3/4] x86/intel lbr: down with test_thread_flag(TIF_IA32)

On Thu, Apr 14, 2016 at 11:10 AM, Dmitry Safonov <dsafonov@...tuozzo.com> wrote:
> Use user_mode64_bit to check process state. For that pass
> interrupt register set from irq handler.
> This should fix opcode decoder misinterpreting ABI for
> tasks that change their code selector.
>
> Signed-off-by: Dmitry Safonov <dsafonov@...tuozzo.com>
> ---
>  arch/x86/events/intel/core.c |  2 +-
>  arch/x86/events/intel/lbr.c  | 17 ++++++++++-------
>  arch/x86/events/perf_event.h |  2 +-
>  3 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 68fa55b4d42e..df13d1d6dbf6 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -1860,7 +1860,7 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
>
>         loops = 0;
>  again:
> -       intel_pmu_lbr_read();
> +       intel_pmu_lbr_read(regs);
>         intel_pmu_ack_status(status);
>         if (++loops > 100) {
>                 static bool warned = false;
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 6c3b7c1780c9..f1a1dbc77dea 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -136,7 +136,9 @@ enum {
>          X86_BR_IRQ             |\
>          X86_BR_INT)
>
> -static void intel_pmu_lbr_filter(struct cpu_hw_events *cpuc);
> +
> +static void
> +intel_pmu_lbr_filter(struct pt_regs *regs, struct cpu_hw_events *cpuc);
>
>  /*
>   * We only support LBR implementations that have FREEZE_LBRS_ON_PMI
> @@ -500,7 +502,7 @@ static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
>         cpuc->lbr_stack.nr = out;
>  }
>
> -void intel_pmu_lbr_read(void)
> +void intel_pmu_lbr_read(struct pt_regs *regs)
>  {
>         struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>
> @@ -512,7 +514,7 @@ void intel_pmu_lbr_read(void)
>         else
>                 intel_pmu_lbr_read_64(cpuc);
>
> -       intel_pmu_lbr_filter(cpuc);
> +       intel_pmu_lbr_filter(regs, cpuc);
>  }
>
>  /*
> @@ -658,7 +660,8 @@ int intel_pmu_setup_lbr_filter(struct perf_event *event)
>   * decoded (e.g., text page not present), then X86_BR_NONE is
>   * returned.
>   */
> -static int branch_type(unsigned long from, unsigned long to, int abort)
> +static int branch_type(unsigned long from, unsigned long to, int abort,
> +               struct pt_regs *regs)
>  {
>         struct insn insn;
>         void *addr;
> @@ -724,7 +727,7 @@ static int branch_type(unsigned long from, unsigned long to, int abort)
>          * on 64-bit systems running 32-bit apps
>          */
>  #ifdef CONFIG_X86_64
> -       is64 = kernel_ip((unsigned long)addr) || !test_thread_flag(TIF_IA32);
> +       is64 = kernel_ip((unsigned long)addr) || user_64bit_mode(regs);

Peterz, looking at this some more, would it make sense to pass
user_regs and interrupt_regs (or whatever we'd call it) all the way
through to here?

--Andy