Subject: a REALLY, REALLY nasty bug

From: Jun Sun <[email-masked]>
Date: Wed, 25 Oct 2000 20:03:20 -0700

Nasty degree - 3 days of tracking.

The symptom was pthread cannot be created.  In the end the caller will
get a BUS error.

What exactly happened has to do with how registers are saved.  Below
attached is the beginning part of sys_sigsuspend() function.  It is easy
to see that s0 is saved into stack frame AFTER its modified.  Next time
when process returns to userland, the s0 reg will be wrong!

So the bug is either 

1) that we need to save s0 register in SAVE_SOME and not save it in
save_static; or that

2) we fix compiler so that it does not use s0 register in that case (it
does the same thing for sys_rt_sigsuspend)

I am sure Ralf will have something to say about it.  :-)  In any case, I
attached a patch for 1) fix.

Jun

------------

sys_sigsuspend(struct pt_regs regs)
{
    8008e280:   27bdffc0        addiu   $sp,$sp,-64
    8008e284:   afb00030        sw      $s0,48($sp)
        sigset_t *uset, saveset, newset;

        save_static(&regs);
    8008e288:   27b00040        addiu   $s0,$sp,64
    8008e28c:   afbf003c        sw      $ra,60($sp)
    8008e290:   afb20038        sw      $s2,56($sp)
    8008e294:   afb10034        sw      $s1,52($sp)
    8008e298:   afa40040        sw      $a0,64($sp)
    8008e29c:   afa50044        sw      $a1,68($sp)
    8008e2a0:   afa60048        sw      $a2,72($sp)
    8008e2a4:   afa7004c        sw      $a3,76($sp)
    8008e2a8:   ae100058        sw      $s0,88($s0)
    8008e2ac:   ae11005c        sw      $s1,92($s0)
    .....

diff -Nru linux/include/asm-mips/stackframe.h.orig linux/include/asm-mips/stackframe.h
--- linux/include/asm-mips/stackframe.h.orig    Wed Oct 25 19:45:50 2000
+++ linux/include/asm-mips/stackframe.h Wed Oct 25 19:47:54 2000
@@ -49,7 +49,6 @@
 
 #define save_static(frame)                               \
        __asm__ __volatile__(                            \
-               "sw\t$16,"__str(PT_R16)"(%0)\n\t"        \
                "sw\t$17,"__str(PT_R17)"(%0)\n\t"        \
                "sw\t$18,"__str(PT_R18)"(%0)\n\t"        \
                "sw\t$19,"__str(PT_R19)"(%0)\n\t"        \
@@ -90,6 +89,7 @@
                mfc0    v1, CP0_EPC;                     \
                sw      $7, PT_R7(sp);                   \
                sw      v1, PT_EPC(sp);                  \
+               sw      $16, PT_R16(sp);                 \
                sw      $25, PT_R25(sp);                 \
                sw      $28, PT_R28(sp);                 \
                sw      $31, PT_R31(sp);                 \

---

From: Ralf Baechle <[email-masked]>
Date: Fri, 27 Oct 2000 01:38:45 +0200

> attached a patch for 1) fix.

A fix that is less easily affected by compiler overoptmizations is contained
in 2.2; I'll merge it forward into 2.4.  Dunno how that didn't make it
into 2.4.

Time for a brown paper bag.

  Ralf

---

From: Jun Sun <[email-masked]>
Date: Thu, 26 Oct 2000 18:57:53 -0700

> in 2.2; I'll merge it forward into 2.4.  Dunno how that didn't make it

I took a look of what v2.2 does.  It does look better than my hack. 
Just verified that it worked on my machine.  I attached the patch - if
you want to save some time.
 
> Time for a brown paper bag.

You mean time for beer and pork feet?
 
Jun

diff -Nru linux/include/asm-mips/stackframe.h.orig linux/include/asm-mips/stackframe.h
--- linux/include/asm-mips/stackframe.h.orig	Wed Oct 25 19:45:50 2000
+++ linux/include/asm-mips/stackframe.h	Thu Oct 26 18:34:50 2000
@@ -47,19 +47,28 @@
 #define __str2(x) #x
 #define __str(x) __str2(x)
 
-#define save_static(frame)                               \
-	__asm__ __volatile__(                            \
-		"sw\t$16,"__str(PT_R16)"(%0)\n\t"        \
-		"sw\t$17,"__str(PT_R17)"(%0)\n\t"        \
-		"sw\t$18,"__str(PT_R18)"(%0)\n\t"        \
-		"sw\t$19,"__str(PT_R19)"(%0)\n\t"        \
-		"sw\t$20,"__str(PT_R20)"(%0)\n\t"        \
-		"sw\t$21,"__str(PT_R21)"(%0)\n\t"        \
-		"sw\t$22,"__str(PT_R22)"(%0)\n\t"        \
-		"sw\t$23,"__str(PT_R23)"(%0)\n\t"        \
-		"sw\t$30,"__str(PT_R30)"(%0)\n\t"        \
-		: /* No outputs */                       \
-		: "r" (frame))
+#define save_static_function(symbol)                                    \
+__asm__ (                                                               \
+        ".globl\t" #symbol "\n\t"                                       \
+        ".align\t2\n\t"                                                 \
+        ".type\t" #symbol ", @function\n\t"                             \
+        ".ent\t" #symbol ", 0\n"                                        \
+        #symbol":\n\t"                                                  \
+        ".frame\t$29, 0, $31\n\t"                                       \
+        "sw\t$16,"__str(PT_R16)"($29)\t\t\t# save_static_function\n\t"  \
+        "sw\t$17,"__str(PT_R17)"($29)\n\t"                              \
+        "sw\t$18,"__str(PT_R18)"($29)\n\t"                              \
+        "sw\t$19,"__str(PT_R19)"($29)\n\t"                              \
+        "sw\t$20,"__str(PT_R20)"($29)\n\t"                              \
+        "sw\t$21,"__str(PT_R21)"($29)\n\t"                              \
+        "sw\t$22,"__str(PT_R22)"($29)\n\t"                              \
+        "sw\t$23,"__str(PT_R23)"($29)\n\t"                              \
+        "sw\t$30,"__str(PT_R30)"($29)\n\t"                              \
+        ".end\t" #symbol "\n\t"                                         \
+        ".size\t" #symbol",. - " #symbol)
+
+/* Used in declaration of save_static functions.  */
+#define unused __attribute__((unused))
 
 #define SAVE_SOME                                        \
 		.set	push;                            \
@@ -90,6 +99,7 @@
 		mfc0	v1, CP0_EPC;                     \
 		sw	$7, PT_R7(sp);                   \
 		sw	v1, PT_EPC(sp);                  \
+		sw	$16, PT_R16(sp);                 \
 		sw	$25, PT_R25(sp);                 \
 		sw	$28, PT_R28(sp);                 \
 		sw	$31, PT_R31(sp);                 \
diff -Nru linux/arch/mips/kernel/signal.c.orig linux/arch/mips/kernel/signal.c
--- linux/arch/mips/kernel/signal.c.orig	Thu Oct 26 18:36:54 2000
+++ linux/arch/mips/kernel/signal.c	Thu Oct 26 18:42:01 2000
@@ -76,12 +76,12 @@
 /*
  * Atomically swap in the new signal mask, and wait for a signal.
  */
-asmlinkage inline int
-sys_sigsuspend(struct pt_regs regs)
+save_static_function(sys_sigsuspend);
+static unused int
+_sys_sigsuspend(struct pt_regs regs)
 {
 	sigset_t *uset, saveset, newset;
 
-	save_static(&regs);
 	uset = (sigset_t *) regs.regs[4];
 	if (copy_from_user(&newset, uset, sizeof(sigset_t)))
 		return -EFAULT;
@@ -103,13 +103,13 @@
 	}
 }
 
-asmlinkage int
-sys_rt_sigsuspend(struct pt_regs regs)
+
+save_static_function(sys_rt_sigsuspend);
+static unused int
+_sys_rt_sigsuspend(struct pt_regs regs)
 {
 	sigset_t *unewset, saveset, newset;
         size_t sigsetsize;
-
-	save_static(&regs);
 
 	/* XXX Don't preclude handling different sized sigset_t's.  */
 	sigsetsize = regs.regs[5];
diff -Nru linux/arch/mips/kernel/syscall.c.orig linux/arch/mips/kernel/syscall.c
--- linux/arch/mips/kernel/syscall.c.orig	Thu Oct 26 18:43:10 2000
+++ linux/arch/mips/kernel/syscall.c	Thu Oct 26 18:47:00 2000
@@ -92,22 +92,22 @@
 	return do_mmap2(addr, len, prot, flags, fd, pgoff);
 }
 
-asmlinkage int sys_fork(struct pt_regs regs)
+save_static_function(sys_fork);
+static unused int _sys_fork(struct pt_regs regs)
 {
 	int res;
 
-	save_static(&regs);
 	res = do_fork(SIGCHLD, regs.regs[29], &regs);
 	return res;
 }
 
-asmlinkage int sys_clone(struct pt_regs regs)
+save_static_function(sys_clone);
+static unused int _sys_clone(struct pt_regs regs)
 {
 	unsigned long clone_flags;
 	unsigned long newsp;
 	int res;
 
-	save_static(&regs);
 	clone_flags = regs.regs[4];
 	newsp = regs.regs[5];
 	if (!newsp)