* [PATCH v2] powerpc/perf: Fix kernel address leaks via Sampling registers
@ 2018-03-21 11:40 Madhavan Srinivasan
2018-03-21 11:40 ` [PATCH v2] powerpc/perf: Fix kernel address leak to userspace via BHRB buffer Madhavan Srinivasan
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Madhavan Srinivasan @ 2018-03-21 11:40 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, Madhavan Srinivasan
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 <maddy@linux.vnet.ibm.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2] powerpc/perf: Fix kernel address leak to userspace via BHRB buffer
2018-03-21 11:40 [PATCH v2] powerpc/perf: Fix kernel address leaks via Sampling registers Madhavan Srinivasan
@ 2018-03-21 11:40 ` Madhavan Srinivasan
2018-03-28 14:13 ` [v2] " Michael Ellerman
2018-03-21 11:40 ` [PATCH v2] powerpc/perf: Fix the kernel address leak to userspace via SDAR Madhavan Srinivasan
2018-03-28 14:13 ` [v2] powerpc/perf: Fix kernel address leaks via Sampling registers Michael Ellerman
2 siblings, 1 reply; 6+ messages in thread
From: Madhavan Srinivasan @ 2018-03-21 11:40 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, Madhavan Srinivasan
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 <maddy@linux.vnet.ibm.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] powerpc/perf: Fix the kernel address leak to userspace via SDAR
2018-03-21 11:40 [PATCH v2] powerpc/perf: Fix kernel address leaks via Sampling registers Madhavan Srinivasan
2018-03-21 11:40 ` [PATCH v2] powerpc/perf: Fix kernel address leak to userspace via BHRB buffer Madhavan Srinivasan
@ 2018-03-21 11:40 ` Madhavan Srinivasan
2018-03-28 14:13 ` [v2] " Michael Ellerman
2018-03-28 14:13 ` [v2] powerpc/perf: Fix kernel address leaks via Sampling registers Michael Ellerman
2 siblings, 1 reply; 6+ messages in thread
From: Madhavan Srinivasan @ 2018-03-21 11:40 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, Madhavan Srinivasan
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 <maddy@linux.vnet.ibm.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [v2] powerpc/perf: Fix kernel address leaks via Sampling registers
2018-03-21 11:40 [PATCH v2] powerpc/perf: Fix kernel address leaks via Sampling registers Madhavan Srinivasan
2018-03-21 11:40 ` [PATCH v2] powerpc/perf: Fix kernel address leak to userspace via BHRB buffer Madhavan Srinivasan
2018-03-21 11:40 ` [PATCH v2] powerpc/perf: Fix the kernel address leak to userspace via SDAR Madhavan Srinivasan
@ 2018-03-28 14:13 ` Michael Ellerman
2 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: Madhavan Srinivasan, linuxppc-dev
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 <maddy@linux.vnet.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/e1ebd0e5b9d0a10ba65e63a3514b6d
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-28 14:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-21 11:40 [PATCH v2] powerpc/perf: Fix kernel address leaks via Sampling registers Madhavan Srinivasan
2018-03-21 11:40 ` [PATCH v2] powerpc/perf: Fix kernel address leak to userspace via BHRB buffer Madhavan Srinivasan
2018-03-28 14:13 ` [v2] " Michael Ellerman
2018-03-21 11:40 ` [PATCH v2] powerpc/perf: Fix the kernel address leak to userspace via SDAR Madhavan Srinivasan
2018-03-28 14:13 ` [v2] " Michael Ellerman
2018-03-28 14:13 ` [v2] powerpc/perf: Fix kernel address leaks via Sampling registers Michael Ellerman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.