[PATCH v3 3/5] microblaze: don't clobber r3/r4 restored by rt_sigreturn

[Ramin Moussavi]

ret_from_trap begins by storing the system call return values r3 and r4
back into the saved user pt_regs.  That is right for an ordinary system
call, but sys_rt_sigreturn() returns through the same path and has to be
transparent: restore_sigcontext() has just filled pt_regs from the signal
frame, and every register must reach userspace exactly as saved.  The two
stores overwrite the restored r3/r4 with whatever the C function left in
those registers.

Only r4 is actually lost, and that follows from how sigreturn is written
everywhere rather than from chance.  Architectures have sys_rt_sigreturn()
return the restored return-value register precisely so that this writeback
stores the restored value -- arm returns regs->ARM_r0, riscv and csky
regs->a0, arc regs->r0 -- and microblaze does the same through
*rval_p = regs->r3 in restore_sigcontext().  But the entry macros treat r3
and r4 as a pair, so ret_from_trap writes back two registers while a C
function has only one return value; nothing carries the restored r4.

Commit 791d0a169b91 ("microblaze: Fix sys_rt_sigreturn_wrapper") introduced
this in v2.6.37.  The old wrapper saved r3/r4 before the call and reloaded
them from pt_regs afterwards; switching brlid to brid removed the only
point where they were reloaded, and the stores have been overwriting them
ever since.

Fix it the way the sibling paths already behave: ret_from_irq and
ret_from_exc restore the full register set and do not perform these stores
at all.  Label the instruction after the stores ret_from_trap_no_rval and
enter there, biasing r15 by -8 so that the ABI return "rtsd r15, 8" lands
on it; naming the entry rather than computing ret_from_trap + 8 keeps it
correct if the number of stores ever changes.  Restoring the old
save/reload would work too, but costs four memory accesses and a branch to
undo damage that is better not done.

Any value the compiler keeps in r4 across a signal is lost.  The tightest
windows are the lwx/swx compare-and-swap retry loops gcc emits for atomics:
in uClibc-ng's libc.so alone, 50 of 364 such loops hold the address in r4.
This is not specific to one libc -- musl passes the address as "r"(p), and
glibc has no microblaze atomic-machine.h and so uses gcc's __atomic
builtins, which expand the same way.  Triggering it needs threads plus a
signal storm, which is why it survived 15 years.

Reproduced on qemu-system-microblazeel (petalogix-s3adsp1800) by running
the uClibc-ng NPTL test tst-eintr1 40 times against an unchanged userspace:
on v7.2 built with gcc 16.2.0, 23 of 40 iterations died with SIGSEGV
without this patch and 0 of 40 with it; on v7.0 built with gcc 12.5.0 the
same comparison gave 6 of 40 against 0 of 40.  The register dump of a
failing iteration shows the signature: a zero r4 and a fault at address 0,
while r3 came back intact.

Fixes: 791d0a169b91 ("microblaze: Fix sys_rt_sigreturn_wrapper")
Cc: stable@xxxxxxxxxxxxxxx
Assisted-by: Claude:claude-opus-5
Signed-off-by: Ramin Moussavi <ramin.moussavi@xxxxxxxxx>
---
 arch/microblaze/kernel/entry.S | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/microblaze/kernel/entry.S b/arch/microblaze/kernel/entry.S
index 582d7256d815..fea235f139d7 100644
--- a/arch/microblaze/kernel/entry.S
+++ b/arch/microblaze/kernel/entry.S
@@ -434,7 +434,13 @@ C_ENTRY(_user_exception):
 C_ENTRY(ret_from_trap):
 	swi	r3, r1, PT_R3
 	swi	r4, r1, PT_R4
-
+/*
+ * Entry point for returns that must not store r3/r4 back into pt_regs,
+ * i.e. rt_sigreturn, which has already restored them from the signal
+ * context.  Reached as "rtsd r15, 8" with r15 set to this label minus 8,
+ * so it stays correct if the number of stores above ever changes.
+ */
+ret_from_trap_no_rval:
 	lwi	r11, r1, PT_MODE;
 /* See if returning to kernel mode, if so, skip resched &c.  */
 	bnei	r11, 2f;
@@ -518,6 +524,15 @@ C_ENTRY(ret_from_kernel_thread):
 
 C_ENTRY(sys_rt_sigreturn_wrapper):
 	addik	r30, r0, 0		/* no restarts */
+	/*
+	 * rt_sigreturn restores the full register set from the signal
+	 * context, so it must skip the r3/r4 syscall-return stores at the
+	 * head of ret_from_trap which would otherwise overwrite the
+	 * just-restored r3/r4.  Every C function returns with "rtsd r15, 8"
+	 * -- the ABI return, where the 8 skips the caller's branch and its
+	 * delay slot -- so bias r15 by -8 to land on ret_from_trap_no_rval.
+	 */
+	addik	r15, r0, ret_from_trap_no_rval - 8
 	brid	sys_rt_sigreturn	/* Do real work */
 	addik	r5, r1, 0;		/* add user context as 1st arg */
 
-- 
2.53.0