Re: [PATCH 2/3] MIPS: perf: Add SB1 userspace callchain unwinding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] MIPS: perf: perf_callchain_user() clobbers
  current->thread.cp0_baduaddr
--

commit d44d716d25b2c4b55ac95b94fe0bbc43afc39a94
Author: Florian Fainelli <f.fainelli@gmail.com>

MIPS: perf: Add SB1 userspace callchain unwinding

This commit adds in-kernel userspace callchain unwinding for SB1 and SB1A
CPUs by scanning function prologues prior to the sampled PC.

> diff --git a/arch/mips/kernel/perf_event.c b/arch/mips/kernel/perf_event.c
> index 5d7a9c03903b5..811ac8f03cabd 100644
> --- a/arch/mips/kernel/perf_event.c
> +++ b/arch/mips/kernel/perf_event.c
> @@ -13,15 +13,113 @@

[ ... ]

> +/* Best-effort prologue unwinding, as provided by the former MIPS OProfile. */
> +static bool sb1_unwind_user(struct sb1_user_frame *frame)
> +{
> +	unsigned long pc = frame->pc, caller = frame->ra;
> +	unsigned int frame_size = 0, ra_size = 0, ra_offset = 0, n;
> +	union mips_instruction insn;
> +
> +	/* SB1 has neither MIPS16 nor microMIPS instructions. */
> +	if ((pc & 3) || (frame->sp & 3))
> +		return false;
> +
> +	for (n = 0; n < 128 && pc >= sizeof(insn); n++) {
> +		pc -= sizeof(insn);
> +		if (copy_from_user_nofault(&insn, (void __user *)pc, sizeof(insn)))
> +			break;

[ ... ]

> +void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
> +			 struct pt_regs *regs)
> +{
> +	struct sb1_user_frame frame = {
> +		.pc = sb1_user_address(regs->cp0_epc),
> +		.sp = sb1_user_address(regs->regs[29]),
> +		.ra = sb1_user_address(regs->regs[31]),
> +	};
> +
> +	if (current_cpu_type() != CPU_SB1 && current_cpu_type() != CPU_SB1A)
> +		return;
> +	do {
> +		if (perf_callchain_store(entry, frame.pc))
> +			break;
> +	} while (sb1_unwind_user(&frame));
> +}

[Severity: Medium]
Can perf_callchain_user() clobber current->thread.cp0_baduaddr while an
interrupted process-context usercopy is recovering from a fault?

On MIPS, raw_copy_from_user(), raw_copy_to_user() and __clear_user() run
with hardware interrupts enabled.  When a kernel-mode user access faults,
__do_page_fault() (or do_ade()) runs fixup_exception() and stores the
faulting address in current->thread.cp0_baduaddr:

arch/mips/mm/fault.c:__do_page_fault() {
    ...
no_context:
	/* Are we prepared to handle this kernel fault?  */
	if (fixup_exception(regs)) {
		current->thread.cp0_baduaddr = address;
		return;
	}
    ...
}

If a perf interrupt fires before the usercopy fixup handler reads
THREAD_BUADDR (current->thread.cp0_baduaddr) and get_perf_callchain()
walks the user stack of the interrupted user task, a fault in
copy_from_user_nofault() inside sb1_unwind_user() overwrites
cp0_baduaddr:

raw_copy_from_user()
  <page fault on user address>
  __do_page_fault()
    fixup_exception(regs)
    current->thread.cp0_baduaddr = address;
  <hardirq>
    ...
      get_perf_callchain()
        perf_callchain_user()
          sb1_unwind_user()
            copy_from_user_nofault()
              <page fault on unmapped user page>
              __do_page_fault()
                fixup_exception(regs)
                current->thread.cp0_baduaddr = address;
  .Ll_exc:
    LOADK t0, THREAD_BUADDR(t0)  <-- reads clobbered cp0_baduaddr
    SUB len, AT, t0

In _inline_copy_from_user(), the bogus residual byte count returned by
raw_copy_from_user() is then passed to memset(to + (n - res), 0, res),
corrupting kernel memory out of bounds or leaving uncopied bytes
unzeroed.

Saving and restoring current->thread.cp0_baduaddr across
perf_callchain_user() would avoid corrupting an in-flight usercopy fixup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112190916.3725141-1-f.fainelli@gmail.com?part=2