linux-kernel - Re: [RFC][PATCH] perf, intel: Don't touch MSR_IA32

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1347531789.15764.124.camel@twins>
Date:	Thu, 13 Sep 2012 12:23:09 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Stephane Eranian <eranian@...gle.com>
Cc:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Oleg Nesterov <oleg@...hat.com>,
	linux-kernel <linux-kernel@...r.kernel.org>, mingo@...nel.org
Subject: Re: [RFC][PATCH] perf, intel: Don't touch MSR_IA32_DEBUGCTLMSR from
 NMI context

On Wed, 2012-09-12 at 18:22 +0200, Peter Zijlstra wrote:
> Oleg and Sebastian found that touching MSR_IA32_DEBUGCTLMSR from NMI
> context is problematic since the only way to change the various
> unrelated bits in there is:
> 
>   debugctl = get_debugctlmsr()
>   /* frob flags in debugctl */
>   update_debugctlmsr(debugctl);
> 
> Which is entirely unsafe if we prod at the MSR from NMI context.
> 
> In particular the path that is responsible is:
> 
>   x86_pmu_handle_irq() (NMI handler)
>     x86_pmu_stop()
>       x86_pmu.disable -> intel_pmu_disable_event()
>         intel_pmu_lbr_disable()
>           __intel_pmu_lbr_disable()
>             wrmsrl(MSR_IA32_DEBUGCTLMSR,... );
> 
> So remove intel_pmu_lbr_{en,dis}able() from
> intel_pmu_{en,dis}able_event() and stick them in
> intel_{get,put}_event_constraints() which are only ever called from
> regular contexts.
> 
> We combine intel_pmu_needs_lbr_smpl(), which tells us if the events
> wants LBR data, with event->hw.branch_reg.alloc, which given
> intel_shared_regs_constraints() is set if our event got the shared
> resource, to give us a correctly balanced condition in
> {get,put}_event_constraints() for intel_pmu_lbr_{en,dis}able().
> 
> Also change core_pmu to only use x86_get_event_constraints since it
> doesn't support any of the fancy DS (BTS,PEBS), LBR and OFFCORE features
> that make up intel_{get,put}_event_constraints. 

OK, so with the added stipulation that touching the MSR from the NMI
isn't a problem as long as we ensure we leave the MSR in the same state
that we found it in, and the above is the only path found so far that
could cause this to be violated, the patch is fine?

In particular we could note that both LBR and BTS use the DEBUGCTL MSR,
but BTS doesn't throttle, so the LBR code is the only thing needing
attention as per the above description.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/