linux-kernel - Re: [PATCH] x86/perf: Default freeze_on

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: <YfFU+2nMjEC1Mo3m@hirez.programming.kicks-ass.net>
Date:   Wed, 26 Jan 2022 15:04:43 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Liang, Kan" <kan.liang@...ux.intel.com>
Cc:     Kyle Huey <me@...ehuey.com>,
        open list <linux-kernel@...r.kernel.org>,
        linux-perf-users@...r.kernel.org, "H. Peter Anvin" <hpa@...or.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Namhyung Kim <namhyung@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Mark Rutland <mark.rutland@....com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Robert O'Callahan <rocallahan@...il.com>,
        Keno Fischer <keno@...iacomputing.com>,
        Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH] x86/perf: Default freeze_on_smi on for Comet Lake and
 later.

On Tue, Jan 25, 2022 at 08:57:09AM -0500, Liang, Kan wrote:
> I see. I was thought the unprivileged user can observe the SMM code on the
> previous platforms. The CML+ change only makes part of the SMM code CPL0.
> Seems I'm wrong. The change looks like changing the previous CPL0 code to
> CPL3 code. If so, yes, I think we should prevent the information leaks for
> the unprivileged user.

Right.

> Changing it to all platforms seems a too big hammer. I agree we should limit
> it to the impacted platforms.
> 
> I've contacted the author of the white paper. I was told that the change is
> for the client vPro platforms. They are not sure whether it impacts Server
> platform or Atom platforms. I'm still working on it. I will let you and
> Peter know once I get more information.

For now I've updated the patch as per the below. I'm tempted to simply
apply it as is and let it be.

Having different defaults for vPro vs !vPro chips seems more confusing
than not.

We should also very much get this change reverted for future chips.

--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -6094,6 +6094,16 @@ __init int intel_pmu_init(void)
 			x86_pmu.commit_scheduling = intel_tfa_commit_scheduling;
 		}
 
+		if (boot_cpu_data.x86_model == INTEL_FAM6_COMETLAKE_L ||
+		    boot_cpu_data.x86_model == INTEL_FAM6_COMETLAKE) {
+			/*
+			 * For some idiotic reason SMM is visible to USR
+			 * counters. Since this is a privilege issue, default
+			 * disable counters in SMM for these chips.
+			 */
+			x86_pmu.attr_freeze_on_smi = 1;
+		}
+
 		pr_cont("Skylake events, ");
 		name = "skylake";
 		break;
@@ -6135,6 +6145,8 @@ __init int intel_pmu_init(void)
 		x86_pmu.num_topdown_events = 4;
 		x86_pmu.update_topdown_event = icl_update_topdown_event;
 		x86_pmu.set_topdown_event_period = icl_set_topdown_event_period;
+		/* SMM visible in USR, see above */
+		x86_pmu.attr_freeze_on_smi = 1;
 		pr_cont("Icelake events, ");
 		name = "icelake";
 		break;
@@ -6172,6 +6184,8 @@ __init int intel_pmu_init(void)
 		x86_pmu.num_topdown_events = 8;
 		x86_pmu.update_topdown_event = icl_update_topdown_event;
 		x86_pmu.set_topdown_event_period = icl_set_topdown_event_period;
+		/* SMM visible in USR, see above */
+		x86_pmu.attr_freeze_on_smi = 1;
 		pr_cont("Sapphire Rapids events, ");
 		name = "sapphire_rapids";
 		break;
@@ -6217,6 +6231,8 @@ __init int intel_pmu_init(void)
 		 * x86_pmu.rtm_abort_event.
 		 */
 		x86_pmu.rtm_abort_event = X86_CONFIG(.event=0xc9, .umask=0x04);
+		/* SMM visible in USR, see above */
+		x86_pmu.attr_freeze_on_smi = 1;
 
 		td_attr = adl_hybrid_events_attrs;
 		mem_attr = adl_hybrid_mem_attrs;