stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Kan Liang <kan.liang@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Sasha Levin <sashal@kernel.org>,
	linux-perf-users@vger.kernel.org
Subject: [PATCH AUTOSEL 5.13 66/85] perf/x86: Reset the dirty counter to prevent the leak for an RDPMC task
Date: Sun,  4 Jul 2021 19:04:01 -0400	[thread overview]
Message-ID: <20210704230420.1488358-66-sashal@kernel.org> (raw)
In-Reply-To: <20210704230420.1488358-1-sashal@kernel.org>

From: Kan Liang <kan.liang@linux.intel.com>

[ Upstream commit 5471eea5d3bf850316f1064a6f57b34c444bce67 ]

The counter value of a perf task may leak to another RDPMC task.
For example, a perf stat task as below is running on CPU 0.

    perf stat -e 'branches,cycles' -- taskset -c 0 ./workload

In the meantime, an RDPMC task, which is also running on CPU 0, may read
the GP counters periodically. (The RDPMC task creates a fixed event,
but read four GP counters.)

    $./rdpmc_read_all_counters
    index 0x0 value 0x8001e5970f99
    index 0x1 value 0x8005d750edb6
    index 0x2 value 0x0
    index 0x3 value 0x0

    index 0x0 value 0x8002358e48a5
    index 0x1 value 0x8006bd1e3bc9
    index 0x2 value 0x0
    index 0x3 value 0x0

It is a potential security issue. Once the attacker knows what the other
thread is counting. The PerfMon counter can be used as a side-channel to
attack cryptosystems.

The counter value of the perf stat task leaks to the RDPMC task because
perf never clears the counter when it's stopped.

Three methods were considered to address the issue.

 - Unconditionally reset the counter in x86_pmu_del(). It can bring extra
   overhead even when there is no RDPMC task running.

 - Only reset the un-assigned dirty counters when the RDPMC task is
   scheduled in via sched_task(). It fails for the below case.

	Thread A			Thread B

	clone(CLONE_THREAD) --->
	set_affine(0)
					set_affine(1)
					while (!event-enabled)
						;
	event = perf_event_open()
	mmap(event)
	ioctl(event, IOC_ENABLE); --->
					RDPMC

   Counters are still leaked to the thread B.

 - Only reset the un-assigned dirty counters before updating the CR4.PCE
   bit. The method is implemented here.

The dirty counter is a counter, on which the assigned event has been
deleted, but the counter is not reset. To track the dirty counters,
add a 'dirty' variable in the struct cpu_hw_events.

The security issue can only be found with an RDPMC task. To enable the
RDMPC, the CR4.PCE bit has to be updated. Add a
perf_clear_dirty_counters() right before updating the CR4.PCE bit to
clear the existing dirty counters. Only the current un-assigned dirty
counters are reset, because the RDPMC assigned dirty counters will be
updated soon.

After applying the patch,

        $ ./rdpmc_read_all_counters
        index 0x0 value 0x0
        index 0x1 value 0x0
        index 0x2 value 0x0
        index 0x3 value 0x0

        index 0x0 value 0x0
        index 0x1 value 0x0
        index 0x2 value 0x0
        index 0x3 value 0x0

Performance

The performance of a context switch only be impacted when there are two
or more perf users and one of the users must be an RDPMC user. In other
cases, there is no performance impact.

The worst-case occurs when there are two users: the RDPMC user only
uses one counter; while the other user uses all available counters.
When the RDPMC task is scheduled in, all the counters, other than the
RDPMC assigned one, have to be reset.

Test results for the worst-case, using a modified lat_ctx as measured
on an Ice Lake platform, which has 8 GP and 3 FP counters (ignoring
SLOTS).

    lat_ctx -s 128K -N 1000 processes 2

Without the patch:
  The context switch time is 4.97 us

With the patch:
  The context switch time is 5.16 us

There is ~4% performance drop for the context switching time in the
worst-case.

Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/1623693582-187370-1-git-send-email-kan.liang@linux.intel.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/x86/events/core.c            | 28 +++++++++++++++++++++++++++-
 arch/x86/events/perf_event.h      |  1 +
 arch/x86/include/asm/perf_event.h |  1 +
 arch/x86/mm/tlb.c                 | 10 ++++++++--
 4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 8f71dd72ef95..1eb45139fcc6 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -1626,6 +1626,8 @@ static void x86_pmu_del(struct perf_event *event, int flags)
 	if (cpuc->txn_flags & PERF_PMU_TXN_ADD)
 		goto do_del;
 
+	__set_bit(event->hw.idx, cpuc->dirty);
+
 	/*
 	 * Not a TXN, therefore cleanup properly.
 	 */
@@ -2474,6 +2476,31 @@ static int x86_pmu_event_init(struct perf_event *event)
 	return err;
 }
 
+void perf_clear_dirty_counters(void)
+{
+	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+	int i;
+
+	 /* Don't need to clear the assigned counter. */
+	for (i = 0; i < cpuc->n_events; i++)
+		__clear_bit(cpuc->assign[i], cpuc->dirty);
+
+	if (bitmap_empty(cpuc->dirty, X86_PMC_IDX_MAX))
+		return;
+
+	for_each_set_bit(i, cpuc->dirty, X86_PMC_IDX_MAX) {
+		/* Metrics and fake events don't have corresponding HW counters. */
+		if (is_metric_idx(i) || (i == INTEL_PMC_IDX_FIXED_VLBR))
+			continue;
+		else if (i >= INTEL_PMC_IDX_FIXED)
+			wrmsrl(MSR_ARCH_PERFMON_FIXED_CTR0 + (i - INTEL_PMC_IDX_FIXED), 0);
+		else
+			wrmsrl(x86_pmu_event_addr(i), 0);
+	}
+
+	bitmap_zero(cpuc->dirty, X86_PMC_IDX_MAX);
+}
+
 static void x86_pmu_event_mapped(struct perf_event *event, struct mm_struct *mm)
 {
 	if (!(event->hw.flags & PERF_X86_EVENT_RDPMC_ALLOWED))
@@ -2497,7 +2524,6 @@ static void x86_pmu_event_mapped(struct perf_event *event, struct mm_struct *mm)
 
 static void x86_pmu_event_unmapped(struct perf_event *event, struct mm_struct *mm)
 {
-
 	if (!(event->hw.flags & PERF_X86_EVENT_RDPMC_ALLOWED))
 		return;
 
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index ad87cb36f7c8..2bf1c7ea2758 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -229,6 +229,7 @@ struct cpu_hw_events {
 	 */
 	struct perf_event	*events[X86_PMC_IDX_MAX]; /* in counter order */
 	unsigned long		active_mask[BITS_TO_LONGS(X86_PMC_IDX_MAX)];
+	unsigned long		dirty[BITS_TO_LONGS(X86_PMC_IDX_MAX)];
 	int			enabled;
 
 	int			n_events; /* the # of events in the below arrays */
diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 544f41a179fb..8fc1b5003713 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -478,6 +478,7 @@ struct x86_pmu_lbr {
 
 extern void perf_get_x86_pmu_capability(struct x86_pmu_capability *cap);
 extern void perf_check_microcode(void);
+extern void perf_clear_dirty_counters(void);
 extern int x86_perf_rdpmc_index(struct perf_event *event);
 #else
 static inline void perf_get_x86_pmu_capability(struct x86_pmu_capability *cap)
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 78804680e923..cfe6b1e85fa6 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -14,6 +14,7 @@
 #include <asm/nospec-branch.h>
 #include <asm/cache.h>
 #include <asm/apic.h>
+#include <asm/perf_event.h>
 
 #include "mm_internal.h"
 
@@ -404,9 +405,14 @@ static inline void cr4_update_pce_mm(struct mm_struct *mm)
 {
 	if (static_branch_unlikely(&rdpmc_always_available_key) ||
 	    (!static_branch_unlikely(&rdpmc_never_available_key) &&
-	     atomic_read(&mm->context.perf_rdpmc_allowed)))
+	     atomic_read(&mm->context.perf_rdpmc_allowed))) {
+		/*
+		 * Clear the existing dirty counters to
+		 * prevent the leak for an RDPMC task.
+		 */
+		perf_clear_dirty_counters();
 		cr4_set_bits_irqsoff(X86_CR4_PCE);
-	else
+	} else
 		cr4_clear_bits_irqsoff(X86_CR4_PCE);
 }
 
-- 
2.30.2


  parent reply	other threads:[~2021-07-04 23:06 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-04 23:02 [PATCH AUTOSEL 5.13 01/85] spi: Make of_register_spi_device also set the fwnode Sasha Levin
2021-07-04 23:02 ` [PATCH AUTOSEL 5.13 02/85] Add a reference to ucounts for each cred Sasha Levin
2021-07-04 23:02 ` [PATCH AUTOSEL 5.13 03/85] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Sasha Levin
2021-07-04 23:02 ` [PATCH AUTOSEL 5.13 04/85] media: i2c: imx334: fix the pm runtime get logic Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 05/85] media: marvel-ccic: fix some issues when getting pm_runtime Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 06/85] media: mdk-mdp: fix pm_runtime_get_sync() usage count Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 07/85] media: s5p: " Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 08/85] media: am437x: " Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 09/85] media: sh_vou: " Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 10/85] media: mtk-vcodec: fix PM runtime get logic Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 11/85] media: s5p-jpeg: fix pm_runtime_get_sync() usage count Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 12/85] media: sunxi: " Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 13/85] media: sti/bdisp: " Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 14/85] media: exynos4-is: " Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 15/85] media: exynos-gsc: " Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 16/85] spi: spi-loopback-test: Fix 'tx_buf' might be 'rx_buf' Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 17/85] spi: spi-topcliff-pch: Fix potential double free in pch_spi_process_messages() Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 18/85] spi: omap-100k: Fix the length judgment problem Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 19/85] regulator: uniphier: Add missing MODULE_DEVICE_TABLE Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 20/85] sched/core: Initialize the idle task with preemption disabled Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 21/85] hwrng: exynos - Fix runtime PM imbalance on error Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 22/85] crypto: nx - add missing MODULE_DEVICE_TABLE Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 23/85] regmap-i2c: Set regmap max raw r/w from quirks Sasha Levin
2021-07-05 12:09   ` Mark Brown
2021-07-09 14:02     ` Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 24/85] media: sti: fix obj-$(config) targets Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 25/85] sched: Make the idle task quack like a per-CPU kthread Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 26/85] media: cpia2: fix memory leak in cpia2_usb_probe Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 27/85] media: cobalt: fix race condition in setting HPD Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 28/85] media: hevc: Fix dependent slice segment flags Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 29/85] media: pvrusb2: fix warning in pvr2_i2c_core_done Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 30/85] media: imx: imx7_mipi_csis: Fix logging of only error event counters Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 31/85] crypto: qat - check return code of qat_hal_rd_rel_reg() Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 32/85] crypto: qat - remove unused macro in FW loader Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 33/85] crypto: qce: skcipher: Fix incorrect sg count for dma transfers Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 34/85] crypto: ecdh - fix ecdh-nist-p192's entry in testmgr Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 35/85] crypto: ecdh - fix 'ecdh_init' Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 36/85] arm64: perf: Convert snprintf to sysfs_emit Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 37/85] sched/fair: Fix ascii art by relpacing tabs Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 38/85] ima: Don't remove security.ima if file must not be appraised Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 39/85] media: i2c: ov2659: Use clk_{prepare_enable,disable_unprepare}() to set xvclk on/off Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 40/85] media: bt878: do not schedule tasklet when it is not setup Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 41/85] media: em28xx: Fix possible memory leak of em28xx struct Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 42/85] media: hantro: Fix .buf_prepare Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 43/85] media: cedrus: " Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 44/85] media: v4l2-core: Avoid the dangling pointer in v4l2_fh_release Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 45/85] media: bt8xx: Fix a missing check bug in bt878_probe Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 46/85] media: st-hva: Fix potential NULL pointer dereferences Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 47/85] crypto: hisilicon/sec - fixup 3des minimum key size declaration Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 48/85] arm64: entry: don't instrument entry code with KCOV Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 49/85] Makefile: fix GDB warning with CONFIG_RELR Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 50/85] media: dvd_usb: memory leak in cinergyt2_fe_attach Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 51/85] memstick: rtsx_usb_ms: fix UAF Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 52/85] mmc: sdhci-sprd: use sdhci_sprd_writew Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 53/85] mmc: via-sdmmc: add a check against NULL pointer dereference Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 54/85] mmc: sdhci-of-aspeed: Turn down a phase correction warning Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 55/85] spi: meson-spicc: fix a wrong goto jump for avoiding memory leak Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 56/85] spi: meson-spicc: fix memory leak in meson_spicc_probe Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 57/85] regulator: mt6315: Fix checking return value of devm_regmap_init_spmi_ext Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 58/85] crypto: shash - avoid comparing pointers to exported functions under CFI Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 59/85] media: dvb_net: avoid speculation from net slot Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 60/85] media: dvbdev: fix error logic at dvb_register_device() Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 61/85] media: siano: fix device register error path Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 62/85] media: imx-csi: Skip first few frames from a BT.656 source Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 63/85] hwmon: (max31790) Report correct current pwm duty cycles Sasha Levin
2021-07-04 23:03 ` [PATCH AUTOSEL 5.13 64/85] hwmon: (max31790) Fix pwmX_enable attributes Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 65/85] sched/fair: Take thermal pressure into account while estimating energy Sasha Levin
2021-07-04 23:04 ` Sasha Levin [this message]
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 67/85] drivers/perf: fix the missed ida_simple_remove() in ddr_perf_probe() Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 68/85] KVM: arm64: Restore PMU configuration on first run Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 69/85] KVM: PPC: Book3S HV: Fix TLB management on SMT8 POWER9 and POWER10 processors Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 70/85] btrfs: fix error handling in __btrfs_update_delayed_inode Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 71/85] btrfs: abort transaction if we fail to update the delayed inode Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 72/85] btrfs: always abort the transaction if we abort a trans handle Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 73/85] btrfs: sysfs: fix format string for some discard stats Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 74/85] btrfs: scrub: fix subpage repair error caused by hard coded PAGE_SIZE Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 75/85] btrfs: make Private2 lifespan more consistent Sasha Levin
2021-07-07 11:10   ` David Sterba
2021-07-08 11:09     ` Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 76/85] btrfs: fix the filemap_range_has_page() call in btrfs_punch_hole_lock_range() Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 77/85] btrfs: don't clear page extent mapped if we're not invalidating the full page Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 78/85] btrfs: disable build on platforms having page size 256K Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 79/85] locking/lockdep: Fix the dep path printing for backwards BFS Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 80/85] lockding/lockdep: Avoid to find wrong lock dep path in check_irq_usage() Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 81/85] KVM: s390: get rid of register asm usage Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 82/85] regulator: mt6358: Fix vdram2 .vsel_mask Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 83/85] regulator: da9052: Ensure enough delay time for .set_voltage_time_sel Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 84/85] media: Fix Media Controller API config checks Sasha Levin
2021-07-04 23:04 ` [PATCH AUTOSEL 5.13 85/85] seccomp: Support atomic "addfd + send reply" Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210704230420.1488358-66-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).