linux-kernel - Re: [PATCH][V3] arm64: perf: Get the wrong PC value in REGS_ABI

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: <20200623174456.GA5087@willie-the-truck>
Date:   Tue, 23 Jun 2020 18:44:56 +0100
From:   Will Deacon <will@...nel.org>
To:     Mark Rutland <mark.rutland@....com>
Cc:     Jiping Ma <Jiping.Ma2@...driver.com>, zhe.he@...driver.com,
        bruce.ashfield@...il.com, yue.tao@...driver.com,
        will.deacon@....com, linux-kernel@...r.kernel.org,
        paul.gortmaker@...driver.com, catalin.marinas@....com,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH][V3] arm64: perf: Get the wrong PC value in REGS_ABI_32
 mode

On Tue, Jun 23, 2020 at 06:19:10PM +0100, Will Deacon wrote:
> So, I think we should take this patch (which puts the PC where you'd expect
> to find it for compat tasks) and then we could consider removing the current
> lr/sp fudging as a separate patch, which we could revert if it causes a
> problem. However, I'm not sure I want to open that up.

Patch below...

Will

--->8

>From 7452148b87ed8c82826474366dbe536fd960d3a7 Mon Sep 17 00:00:00 2001
From: Jiping Ma <jiping.ma2@...driver.com>
Date: Mon, 11 May 2020 10:52:07 +0800
Subject: [PATCH] arm64: perf: Report the PC value in REGS_ABI_32 mode

A 32-bit perf querying the registers of a compat task using REGS_ABI_32
will receive zeroes from w15, when it expects to find the PC.

Return the PC value for register dwarf register 15 when returning register
values for a compat task to perf.

Signed-off-by: Jiping Ma <jiping.ma2@...driver.com>
Link: https://lore.kernel.org/r/1589165527-188401-1-git-send-email-jiping.ma2@windriver.com
[will: Shuffled code and added a comment]
Signed-off-by: Will Deacon <will@...nel.org>
---
 arch/arm64/kernel/perf_regs.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c
index 0bbac612146e..952b26a05d0f 100644
--- a/arch/arm64/kernel/perf_regs.c
+++ b/arch/arm64/kernel/perf_regs.c
@@ -15,15 +15,25 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
 		return 0;
 
 	/*
-	 * Compat (i.e. 32 bit) mode:
-	 * - PC has been set in the pt_regs struct in kernel_entry,
-	 * - Handle SP and LR here.
+	 * Our handling of compat tasks (PERF_SAMPLE_REGS_ABI_32) is weird. For
+	 * a 32-bit perf inspecting a 32-bit task, then it will look at the
+	 * first 16 registers. These correspond directly to the registers saved
+	 * in our pt_regs structure, with the exception of the PC, so we copy
+	 * that down (x15 corresponds to SP_hyp in the architecture). So far, so
+	 * good. The oddity arises when a 64-bit perf looks at a 32-bit task and
+	 * asks for registers beyond PERF_REG_ARM_MAX. In this case, we return
+	 * SP_usr, LR_usr and PC in the positions where the AArch64 registers
+	 * would normally live. The initial idea was to allow a 64-bit unwinder
+	 * to unwinder a 32-bit task and, although it's not clear how well that
+	 * works in practice, we're kind of stuck with this interface now.
 	 */
 	if (compat_user_mode(regs)) {
 		if ((u32)idx == PERF_REG_ARM64_SP)
 			return regs->compat_sp;
 		if ((u32)idx == PERF_REG_ARM64_LR)
 			return regs->compat_lr;
+		if (idx == 15)
+			return regs->pc;
 	}
 
 	if ((u32)idx == PERF_REG_ARM64_SP)
-- 
2.27.0.111.gc72c7da667-goog