linux-kernel - [PATCH v2 1/2] arm64: entry: Skip single stepping interrupt handlers

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: <20220413065458.88541-2-sumit.garg@linaro.org>
Date:   Wed, 13 Apr 2022 12:24:57 +0530
From:   Sumit Garg <sumit.garg@...aro.org>
To:     linux-arm-kernel@...ts.infradead.org, dianders@...omium.org,
        will@...nel.org, liwei391@...wei.com
Cc:     catalin.marinas@....com, mark.rutland@....com, mhiramat@...nel.org,
        daniel.thompson@...aro.org, jason.wessel@...driver.com,
        maz@...nel.org, linux-kernel@...r.kernel.org,
        Sumit Garg <sumit.garg@...aro.org>
Subject: [PATCH v2 1/2] arm64: entry: Skip single stepping interrupt handlers

Current implementation allows single stepping into interrupt handlers
for interrupts that were received during single stepping. But interrupt
handlers aren't something that the user expect to debug. Moreover single
stepping interrupt handlers is risky as it may sometimes leads to
unbalanced locking when we resume from single-step debug.

Fix broken single-step implementation via skipping single-step over
interrupt handlers. The methodology is when we receive an interrupt from
EL1, check if we are single stepping (pstate.SS). If yes then we save
MDSCR_EL1.SS and clear the register bit if it was set. Then unmask only
D and leave I set. On return from the interrupt, set D and restore
MDSCR_EL1.SS. Along with this skip reschedule if we were stepping.

Suggested-by: Will Deacon <will@...nel.org>
Signed-off-by: Sumit Garg <sumit.garg@...aro.org>
---
 arch/arm64/kernel/entry-common.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 878c65aa7206..dd2d3af615de 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -458,19 +458,35 @@ static __always_inline void __el1_irq(struct pt_regs *regs,
 	do_interrupt_handler(regs, handler);
 	irq_exit_rcu();
 
-	arm64_preempt_schedule_irq();
+	/* Don't reschedule in case we are single stepping */
+	if (!(regs->pstate & DBG_SPSR_SS))
+		arm64_preempt_schedule_irq();
 
 	exit_to_kernel_mode(regs);
 }
+
 static void noinstr el1_interrupt(struct pt_regs *regs,
 				  void (*handler)(struct pt_regs *))
 {
+	unsigned long reg;
+
+	/* Disable single stepping within interrupt handler */
+	if (regs->pstate & DBG_SPSR_SS) {
+		reg = read_sysreg(mdscr_el1);
+		write_sysreg(reg & ~DBG_MDSCR_SS, mdscr_el1);
+	}
+
 	write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
 
 	if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
 		__el1_pnmi(regs, handler);
 	else
 		__el1_irq(regs, handler);
+
+	if (regs->pstate & DBG_SPSR_SS) {
+		write_sysreg(DAIF_PROCCTX_NOIRQ | PSR_D_BIT, daif);
+		write_sysreg(reg, mdscr_el1);
+	}
 }
 
 asmlinkage void noinstr el1h_64_irq_handler(struct pt_regs *regs)
-- 
2.25.1