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

Ramin Moussavi <[email protected]>
Newsgroups org.kernel.vger.linux-arch,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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: [email protected]
Assisted-by: Claude:claude-opus-5
Signed-off-by: Ramin Moussavi <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.