Re: [PATCH v2 08/13] openrisc: pass pt_regs to audit_syscall_entry()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Syscall abortion via ptrace is ignored by the assembly fastpath, causing aborted syscalls to execute anyway.
- [High] Tracer-directed syscall number mutations are ignored by both the audit subsystem and the execution path.
--

commit b9f4a1989ccdc62e1dafc2931825c9e52d439b17
Author: Ricardo Robaina <rrobaina@redhat.com>

openrisc: pass pt_regs to audit_syscall_entry()

audit_syscall_entry() now takes a pointer to pt_regs and extracts
the syscall arguments itself via syscall_get_arguments(). Drop the
individual argument registers from the call and pass regs instead.

> diff --git a/arch/openrisc/kernel/ptrace.c b/arch/openrisc/kernel/ptrace.c
> index 287a9718f0c60..eca613b84ff87 100644
> --- a/arch/openrisc/kernel/ptrace.c
> +++ b/arch/openrisc/kernel/ptrace.c
> @@ -301,8 +301,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
>  		 */
>  		ret = -1L;
>  
> -	audit_syscall_entry(regs->gpr[11], regs->gpr[3], regs->gpr[4],
> -			    regs->gpr[5], regs->gpr[6]);
> +	audit_syscall_entry(regs->gpr[11], regs);

[Severity: High]
This is a pre-existing issue, but does hardcoding regs->gpr[11] here ignore
tracer-directed syscall number mutations?

The generic framework's syscall_set_nr() updates regs->orig_gpr11 with the new
syscall number. However, this code directly passes regs->gpr[11] to
audit_syscall_entry() instead of using syscall_get_nr(current, regs).

Additionally, the _syscall_call assembly fastpath appears to use the original
syscall number reloaded from PT_GPR11(r1). Since neither layer reads
orig_gpr11, standard PTRACE_SET_SYSCALL_INFO mutations seem to silently fail
to influence the kernel on OpenRISC.

>  
>  	return ret ? : regs->gpr[11];

[Severity: High]
This is also a pre-existing issue, but does this function successfully abort
syscalls when a tracer denies them?

When a tracer denies a syscall, ptrace_report_syscall_permit_entry() causes
ret to be set to -1L, which is then returned. However, looking at the
assembly fastpath in arch/openrisc/kernel/entry.S:_syscall_trace_enter:

	l.jal	do_syscall_trace_enter
	 l.addi	r3,r1,0

	/* Restore arguments ... */
	l.lwz	r11,PT_GPR11(r1)

Doesn't this instruction blindly reload the original unmodified syscall number
from the stack into r11?

Since do_syscall_trace_enter() never writes -1L back to regs->gpr[11] in
memory, it seems the abort signal is permanently lost when the assembly
restores registers, allowing the aborted syscall to execute anyway.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788351089.git.rrobaina@redhat.com?part=8