Re: [PATCH] riscv: traps: handle uprobe event in software-check exception

[Deepak Gupta <debug@rivosinc.com>]

Hi Zong,

Thanks for taking the initiative for making cfi work with uprobe.
And sorry for not noticing the patch earlier.
Few comments inline.


On Fri, Mar 14, 2025 at 05:26:14PM +0800, Zong Li wrote:
>Handle the uprobe event first before handling the CFI violation in
>software-check exception handler. Because when the landing pad is
>activated, if the uprobe point is set at the lpad instruction at
>the beginning of a function, the system triggers a software-check
>exception instead of an ebreak exception due to the exception
>priority, then uprobe can't work successfully.
>
>Co-developed-by: Deepak Gupta <debug@rivosinc.com>
>Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>Signed-off-by: Zong Li <zong.li@sifive.com>
>---
>
>This patch is based on top of the following series
>[PATCH v11 00/27] riscv control-flow integrity for usermode
>
> arch/riscv/kernel/traps.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
>index 3f7709f4595a..ef5a92111ee1 100644
>--- a/arch/riscv/kernel/traps.c
>+++ b/arch/riscv/kernel/traps.c
>@@ -386,9 +386,12 @@ asmlinkage __visible __trap_section void do_trap_software_check(struct pt_regs *
> 	if (user_mode(regs)) {
> 		irqentry_enter_from_user_mode(regs);
>
>-		/* not a cfi violation, then merge into flow of unknown trap handler */
>-		if (!handle_user_cfi_violation(regs))
>-			do_trap_unknown(regs);
>+		/* handle uprobe event frist */
>+		if (!probe_breakpoint_handler(regs)) {

If task has uprobe enabled and there is a cfi violation due to mismatch in
return address on shadow stack and regular stack, then it would be a cfi
bypass, right?
Perhaps we should be doing this only when we match that sw check exception
is due to forward cfi violation?

Do you agree?

>+			/* not a cfi violation, then merge into flow of unknown trap handler */
>+			if (!handle_user_cfi_violation(regs))
>+				do_trap_unknown(regs);
>+		}
>
> 		irqentry_exit_to_user_mode(regs);
> 	} else {
>-- 
>2.17.1
>