[PATCH v2] powerpc/perf: Fix kernel address leaks via Sampling registers

From: Michael Ellerman <mpe@ellerman.id.au>

Current code in power_pmu_disable() does not clear the sampling
registers like Sampling Instruction Address Register (SAIR) and
Sampling Data Address Register (SDAR) after disabling the PMU.
Since these are userspace readable and could contain kernel
address, add code to explicitly clear the content of these registers.
Patch also adds a "context synchronizing instruction" to enforce
no further updates to these registers as mandated by PowerISA.

"If an mtspr instruction is executed that changes the
value of a Performance Monitor register other than
SIAR, SDAR, and SIER, the change is not guaranteed
to have taken effect until after a subsequent context
synchronizing instruction has been executed (see
Chapter 11. "Synchronization Requirements for Con-
text Alterations" on page 1133)."

Signed-off-by: Madhavan Srinivasan <redacted>
---
Changelog v1:
  - Added config flags to avoid 32bit build breaks.

 arch/powerpc/perf/core-book3s.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index f89bbd54ecec..39846226c702 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c

@@ -1226,6 +1226,7 @@ static void power_pmu_disable(struct pmu *pmu)
 		 */
 		write_mmcr0(cpuhw, val);
 		mb();
+		isync();
 
 		/*
 		 * Disable instruction sampling if it was enabled

@@ -1234,12 +1235,26 @@ static void power_pmu_disable(struct pmu *pmu)
 			mtspr(SPRN_MMCRA,
 			      cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
 			mb();
+			isync();
 		}
 
 		cpuhw->disabled = 1;
 		cpuhw->n_added = 0;
 
 		ebb_switch_out(mmcr0);
+
+#ifdef CONFIG_PPC64
+		/*
+		 * These are readable by userspace, may contain kernel
+		 * addresses and are not switched by context switch, so clear
+		 * them now to avoid leaking anything to userspace in general
+		 * including to another process.
+		 */
+		if (ppmu->flags & PPMU_ARCH_207S) {
+			mtspr(SPRN_SDAR, 0);
+			mtspr(SPRN_SIAR, 0);
+		}
+#endif
 	}
 
 	local_irq_restore(flags);

-- 
2.7.4

The current Branch History Rolling Buffer (BHRB) code does
not check for any privilege levels before updating the data
from BHRB. This leaks kernel addresses to userspace even when
profiling only with userspace privileges. Add proper checks
to prevent it.

Acked-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Madhavan Srinivasan <redacted>
---
Changelog -v1:
 - Added comment.

 arch/powerpc/perf/core-book3s.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index f89bbd54ecec..37d24c22557d 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c

@@ -457,6 +457,16 @@ static void power_pmu_bhrb_read(struct cpu_hw_events *cpuhw)
 				/* invalid entry */
 				continue;
 
+			/*
+			 * BHRB rolling buffer could very much contain the kernel
+			 * addresses at this point. Check the privileges before
+			 * exporting it to userspace (avoid exposure of regions
+			 * where we could have speculative execution)
+			 */
+			if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN) &&
+				is_kernel_addr(addr))
+				continue;
+
 			/* Branches are read most recent first (ie. mfbhrb 0 is
 			 * the most recent branch).
 			 * There are two types of valid entries:

-- 
2.7.4

Sampled Data Address Register (SDAR) is a 64-bit
register that contains the effective address of
the storage operand of an instruction that was
being executed, possibly out-of-order, at or around
the time that the Performance Monitor alert occurred.

In certain scenario SDAR happen to contain the kernel
address even for userspace only sampling. Add checks
to prevent it.

Signed-off-by: Madhavan Srinivasan <redacted>
---
Changelog v1:
- Removed the event exclusive_ check. Will take it up in a separate patch

 arch/powerpc/perf/core-book3s.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index f89bbd54ecec..e143f85a4098 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c

@@ -95,7 +95,7 @@ static inline unsigned long perf_ip_adjust(struct pt_regs *regs)
 {
 	return 0;
 }
-static inline void perf_get_data_addr(struct pt_regs *regs, u64 *addrp) { }
+static inline void perf_get_data_addr(struct pt_regs *regs, u64 *addrp, struct perf_event *event) { }
 static inline u32 perf_get_misc_flags(struct pt_regs *regs)
 {
 	return 0;

@@ -174,7 +174,7 @@ static inline unsigned long perf_ip_adjust(struct pt_regs *regs)
  * pointed to by SIAR; this is indicated by the [POWER6_]MMCRA_SDSYNC, the
  * [POWER7P_]MMCRA_SDAR_VALID bit in MMCRA, or the SDAR_VALID bit in SIER.
  */
-static inline void perf_get_data_addr(struct pt_regs *regs, u64 *addrp)
+static inline void perf_get_data_addr(struct pt_regs *regs, u64 *addrp, struct perf_event *event)
 {
 	unsigned long mmcra = regs->dsisr;
 	bool sdar_valid;

@@ -198,6 +198,10 @@ static inline void perf_get_data_addr(struct pt_regs *regs, u64 *addrp)
 
 	if (!(mmcra & MMCRA_SAMPLE_ENABLE) || sdar_valid)
 		*addrp = mfspr(SPRN_SDAR);
+
+	if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN) &&
+		is_kernel_addr(mfspr(SPRN_SDAR)))
+		*addrp = 0;
 }
 
 static bool regs_sihv(struct pt_regs *regs)

@@ -2050,7 +2054,7 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
 
 		if (event->attr.sample_type &
 		    (PERF_SAMPLE_ADDR | PERF_SAMPLE_PHYS_ADDR))
-			perf_get_data_addr(regs, &data.addr);
+			perf_get_data_addr(regs, &data.addr, event);
 
 		if (event->attr.sample_type & PERF_SAMPLE_BRANCH_STACK) {
 			struct cpu_hw_events *cpuhw;

-- 
2.7.4

On Wed, 2018-03-21 at 11:40:24 UTC, Madhavan Srinivasan wrote:

From: Michael Ellerman <mpe@ellerman.id.au>

Current code in power_pmu_disable() does not clear the sampling
registers like Sampling Instruction Address Register (SAIR) and
Sampling Data Address Register (SDAR) after disabling the PMU.
Since these are userspace readable and could contain kernel
address, add code to explicitly clear the content of these registers.
Patch also adds a "context synchronizing instruction" to enforce
no further updates to these registers as mandated by PowerISA.

"If an mtspr instruction is executed that changes the
value of a Performance Monitor register other than
SIAR, SDAR, and SIER, the change is not guaranteed
to have taken effect until after a subsequent context
synchronizing instruction has been executed (see
Chapter 11. "Synchronization Requirements for Con-
text Alterations" on page 1133)."

Signed-off-by: Madhavan Srinivasan <redacted>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/e1ebd0e5b9d0a10ba65e63a3514b6d

cheers

On Wed, 2018-03-21 at 11:40:25 UTC, Madhavan Srinivasan wrote:

The current Branch History Rolling Buffer (BHRB) code does
not check for any privilege levels before updating the data
from BHRB. This leaks kernel addresses to userspace even when
profiling only with userspace privileges. Add proper checks
to prevent it.

Acked-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Madhavan Srinivasan <redacted>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/bb19af816025d495376bd76bf6fbcf

cheers

On Wed, 2018-03-21 at 11:40:26 UTC, Madhavan Srinivasan wrote:

Sampled Data Address Register (SDAR) is a 64-bit
register that contains the effective address of
the storage operand of an instruction that was
being executed, possibly out-of-order, at or around
the time that the Performance Monitor alert occurred.

In certain scenario SDAR happen to contain the kernel
address even for userspace only sampling. Add checks
to prevent it.

Signed-off-by: Madhavan Srinivasan <redacted>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/cd1231d7035fea894118d5155ff984

cheers