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

[Deepak Gupta <debug@rivosinc.com>]

On Tue, Jun 03, 2025 at 09:48:08AM +0800, Zong Li wrote:
>On Tue, Jun 3, 2025 at 12:50 AM Deepak Gupta <debug@rivosinc.com> wrote:
>>
>> 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?
>
>Yes, let me add a condition for forward cfi violation here. Thanks for
>pointing it out.

Cool, I'll send out another revision for my cfi series this week.
If you send out your uprobe fix, I can include it in my patchset.

>
>>
>> >+                      /* 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
>> >
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv