[PATCH] powerpc: Fix error path in kernel_thread function

[PATCH] powerpc: Fix error path in kernel_thread function

[Josh Boyer]

From: Josh Poimboeuf <jpoimboe@us.ibm.com>

The powerpc 32-bit and 64-bit kernel_thread functions don't properly
propagate errors being returned by the clone syscall.  (In the case of
error, the syscall exit code returns a positive errno in r3 and sets
the CR0[SO] bit.)

This patch fixes that by negating r3 if CR0[SO] is set after the syscall.

Signed-off-by: Josh Poimboeuf <jpoimboe@us.ibm.com>
Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/misc_32.S |    8 +++++---
 arch/powerpc/kernel/misc_64.S |    8 +++++---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index 7a6dfbc..1fe9f62 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -846,8 +846,10 @@ _GLOBAL(kernel_thread)
 	li	r4,0		/* new sp (unused) */
 	li	r0,__NR_clone
 	sc
-	cmpwi	0,r3,0		/* parent or child? */
-	bne	1f		/* return if parent */
+	bns+	1f		/* did system call indicate error? */
+	neg	r3,r3		/* if so, make return code negative */
+1:	cmpwi	0,r3,0		/* parent or child? */
+	bne	2f		/* return if parent */
 	li	r0,0		/* make top-level stack frame */
 	stwu	r0,-16(r1)
 	mtlr	r30		/* fn addr in lr */
@@ -857,7 +859,7 @@ _GLOBAL(kernel_thread)
 	li	r0,__NR_exit	/* exit if function returns */
 	li	r3,0
 	sc
-1:	lwz	r30,8(r1)
+2:	lwz	r30,8(r1)
 	lwz	r31,12(r1)
 	addi	r1,r1,16
 	blr
diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index 4dd70cf..3053fe5 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -426,8 +426,10 @@ _GLOBAL(kernel_thread)
 	li	r4,0		/* new sp (unused) */
 	li	r0,__NR_clone
 	sc
-	cmpdi	0,r3,0		/* parent or child? */
-	bne	1f		/* return if parent */
+	bns+	1f		/* did system call indicate error? */
+	neg	r3,r3		/* if so, make return code negative */
+1:	cmpdi	0,r3,0		/* parent or child? */
+	bne	2f		/* return if parent */
 	li	r0,0
 	stdu	r0,-STACK_FRAME_OVERHEAD(r1)
 	ld	r2,8(r29)
@@ -438,7 +440,7 @@ _GLOBAL(kernel_thread)
 	li	r0,__NR_exit	/* exit after child exits */
         li	r3,0
 	sc
-1:	addi	r1,r1,STACK_FRAME_OVERHEAD	
+2:	addi	r1,r1,STACK_FRAME_OVERHEAD
 	ld	r29,-24(r1)
 	ld	r30,-16(r1)
 	blr
-- 
1.5.5.1

Re: [PATCH] powerpc: Fix error path in kernel_thread function

[Josh Boyer]

On Tue, Oct 07, 2008 at 12:10:03PM -0400, Josh Boyer wrote:
>From: Josh Poimboeuf <jpoimboe@us.ibm.com>
>
>The powerpc 32-bit and 64-bit kernel_thread functions don't properly
>propagate errors being returned by the clone syscall.  (In the case of
>error, the syscall exit code returns a positive errno in r3 and sets
>the CR0[SO] bit.)
>
>This patch fixes that by negating r3 if CR0[SO] is set after the syscall.
>
>Signed-off-by: Josh Poimboeuf <jpoimboe@us.ibm.com>
>Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

FYI, I boot tested this on a G5 this morning.  A variant for an older
kernel was also tested on a 440-based board.

Not what I would call exhaustive testing, but at least it didn't
crash and burn.

josh

Re: [PATCH] powerpc: Fix error path in kernel_thread function

[Segher Boessenkool]

> -	cmpwi	0,r3,0		/* parent or child? */
> -	bne	1f		/* return if parent */
> +	bns+	1f		/* did system call indicate error? */
> +	neg	r3,r3		/* if so, make return code negative */
> +1:	cmpwi	0,r3,0		/* parent or child? */
> +	bne	2f		/* return if parent */
>  	li	r0,0		/* make top-level stack frame */
>  	stwu	r0,-16(r1)
>  	mtlr	r30		/* fn addr in lr */
> @@ -857,7 +859,7 @@ _GLOBAL(kernel_thread)
>  	li	r0,__NR_exit	/* exit if function returns */
>  	li	r3,0
>  	sc
> -1:	lwz	r30,8(r1)
> +2:	lwz	r30,8(r1)

You don't need to renumber the labels, FWIW.  Some people might
find it more readable this way of course (but then, you could
use actual label _names_ -- what a novel idea! ;-) )


Segher

Re: [PATCH] powerpc: Fix error path in kernel_thread function

[Paul Mackerras]

Josh Boyer writes:

> From: Josh Poimboeuf <jpoimboe@us.ibm.com>
> 
> The powerpc 32-bit and 64-bit kernel_thread functions don't properly
> propagate errors being returned by the clone syscall.  (In the case of
> error, the syscall exit code returns a positive errno in r3 and sets
> the CR0[SO] bit.)
> 
> This patch fixes that by negating r3 if CR0[SO] is set after the syscall.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@us.ibm.com>
> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

Acked-by: Paul Mackerras <paulus@samba.org>

Re: [PATCH] powerpc: Fix error path in kernel_thread function

[Josh Boyer]

On Thu, Oct 09, 2008 at 01:57:37PM +1100, Paul Mackerras wrote:
>Josh Boyer writes:
>
>> From: Josh Poimboeuf <jpoimboe@us.ibm.com>
>> 
>> The powerpc 32-bit and 64-bit kernel_thread functions don't properly
>> propagate errors being returned by the clone syscall.  (In the case of
>> error, the syscall exit code returns a positive errno in r3 and sets
>> the CR0[SO] bit.)
>> 
>> This patch fixes that by negating r3 if CR0[SO] is set after the syscall.
>> 
>> Signed-off-by: Josh Poimboeuf <jpoimboe@us.ibm.com>
>> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
>
>Acked-by: Paul Mackerras <paulus@samba.org>

Should we send this patch to -stable for 2.6.27?  Seems valid enough to
do so.

josh