Re: [PATCH 2/2] powerpc/perf: Fix the kernel address leak to userspace via SDAR
From: Naveen N. Rao <hidden>
Date: 2018-03-05 08:21:29
Madhavan Srinivasan wrote:
quoted hunk ↗ jump to hunk
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. =20 In certain scenario SDAR happen to contain the kernel address even for userspace only sampling. Add checks to prevent it. =20 Signed-off-by: Madhavan Srinivasan <redacted> --- arch/powerpc/perf/core-book3s.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) =20diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-boo=
k3s.c
quoted hunk ↗ jump to hunk
index 337db5831749..c4525323d691 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_re=
gs *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) { }quoted hunk ↗ jump to hunk
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, t=
he
* [POWER7P_]MMCRA_SDAR_VALID bit in MMCRA, or the SDAR_VALID bit in SIE=
R.
*/ -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)
quoted hunk ↗ jump to hunk
{ unsigned long mmcra =3D regs->dsisr; bool sdar_valid;@@ -198,6 +198,11 @@ static inline void perf_get_data_addr(struct pt_regs=
*regs, u64 *addrp)
=20 if (!(mmcra & MMCRA_SAMPLE_ENABLE) || sdar_valid) *addrp =3D mfspr(SPRN_SDAR); + + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN) && + (event->attr.exclude_kernel || event->attr.exclude_hv) &&
I may be missing something, but if !capable(CAP_SYS_ADMIN), should we=20 still check the exclude_kernel/exclude_hv fields in the event attribute? =20 Aren't those user controlled? - Naveen
quoted hunk ↗ jump to hunk
+ is_kernel_addr(mfspr(SPRN_SDAR))) + *addrp =3D 0; } =20 static bool regs_sihv(struct pt_regs *regs)@@ -2054,7 +2059,7 @@ static void record_and_restart(struct perf_event *e=
vent, unsigned long val,
=20
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);
=20
if (event->attr.sample_type & PERF_SAMPLE_BRANCH_STACK) {
struct cpu_hw_events *cpuhw;
--=20
2.7.4
=20
=20=