All of lore.kernel.org
 help / color / mirror / Atom feed
From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
To: linux-sh@vger.kernel.org
Subject: sh:fixed issue in xchg_u32 function when val==r15.
Date: Thu, 07 Apr 2011 12:57:09 +0000	[thread overview]
Message-ID: <4D9DB4A5.4010002@st.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]

Hi All,

Recently we have discovered a bug in xchg_u32 function of GUSA_RB feature.
This function breaks if one of the input parameter 'val' is r15.

Kernel version: Linux 2.6.39-rc2 and previous versions too.

Here is the part of disassembly code of exit_mm function in 
kernel/exit.c which calls xchg_u32 and looks
like: (generated by 'sh4-linux-objdump -d kernel/exit.o' )

     168:    02 c7           mova    174 <exit_mm+0x54>,r0
     16a:    09 00           nop   
     16c:    f3 61           mov    r15,r1
     16e:    fc ef           mov    #-4,r15
     170:    22 67           mov.l    @r2,r7
     172:    f2 22           mov.l    r15,@r2
     174:    13 6f           mov    r1,r15
     176:    01 e3           mov    #1,r3
     178:    7f 1b           mov.l    r7,@(60,r11)
     17a:    d3 62           mov    r13,r2
     17c:    02 c7           mova    188 <exit_mm+0x68>,r0

in '16e' we can see that r15 was changed to -4 and at '172' we can see 
it gets assigned to @r2.

Originally this bug was discovered as part of stlinux bugzilla triage.
This issue can be easily reproduced with a simple multi-threaded 
test-app attached, the kernel crashes while cleaning up its memory 
descriptor.

Fix is making val an input/output constraint so the compiler cannot pass 
r15 directly and must use a temporary register.

After applying the patch the code in exit_mm() looks like:

    168:    02 c7           mova    174 <exit_mm+0x54>,r0
     16a:    09 00           nop   
     16c:    f3 61           mov    r15,r1
     16e:    fc ef           mov    #-4,r15
     170:    22 67           mov.l    @r2,r7
     172:    32 22           mov.l    r3,@r2
     174:    13 6f           mov    r1,r15
     176:    01 e3           mov    #1,r3
     178:    7f 1b           mov.l    r7,@(60,r11)
     17a:    e3 62           mov    r14,r2
     17c:    02 c7           mova    188 <exit_mm+0x68>,r0

Patch is attached

Thanks,
srini

[-- Attachment #2: main.c --]
[-- Type: text/x-csrc, Size: 708 bytes --]

#include <pthread.h>
#include <stdio.h>
#define NTHREADS 40
 
void *dodump(void *threadid)
{
	char *ptr = 0x1212121;
	printf("DUmp... %s \n", ptr);
}
 
void *dowork(void *threadid)
{
	printf("\nThread %d started \n", (int )threadid);
	while(1);
}
 
int main(int argc, char *argv[])
{
   pthread_t threads[NTHREADS];
   int rc;
   long t;
 
   for(t=0; t<NTHREADS; t++){
	if(t==25)
	      	rc = pthread_create(&threads[t], NULL, dodump, (void *)t);
	else
	      rc = pthread_create(&threads[t], NULL, dowork, (void *)t);
	
      if (rc){
         printf("ERROR; return code from pthread_create() is %d\n", rc);
         exit(-1);
      }
   }
   printf("Created %ld threads.\n", t);
   pthread_exit(NULL);
}


[-- Attachment #3: 0001-sh-fixed-issue-in-xchg_u32-function.patch --]
[-- Type: text/x-patch, Size: 1632 bytes --]

From da8a5909fed9bb801b7d9b175330f4e205b9dd61 Mon Sep 17 00:00:00 2001
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Date: Thu, 7 Apr 2011 13:34:43 +0100
Subject: [PATCH sh-2.6.32.y] sh: fixed issue in xchg_u32 function.

This patch addresses a use case where input parameter (val) to xchg_u32
inline asm function is equal to r15. If val == r15 then xchg_u32 always
sets m to -4(0xfffffffc). In particular code in kernel/exit.c exit_mm()
will hit this bug.

This patch makes adds val to input/output constraint so that compiler
cannot pass r15 directly and must use a temporary register instead.

Without this patch a SEGFAULT in multithreaded program will crash the
kernel.

Originally this bug was discovered as part of stlinux bugzilla##11229
triage.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Reviewed-by: Stuart Menefy <stuart.menefy@st.com>
---
 arch/sh/include/asm/cmpxchg-grb.h |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/sh/include/asm/cmpxchg-grb.h b/arch/sh/include/asm/cmpxchg-grb.h
index 4676bf5..fc3deb6 100644
--- a/arch/sh/include/asm/cmpxchg-grb.h
+++ b/arch/sh/include/asm/cmpxchg-grb.h
@@ -15,8 +15,10 @@ static inline unsigned long xchg_u32(volatile u32 *m, unsigned long val)
 		"   mov.l   %2,   @%1     \n\t" /* store new value */
 		"1: mov     r1,   r15     \n\t" /* LOGOUT */
 		: "=&r" (retval),
-		  "+r"  (m)
-		: "r"   (val)
+		  "+r"  (m),
+		  "+r"  (val)	/* when val == r15 this function will not work as expected.
+				 * So val is added to output constriants */
+		:
 		: "memory", "r0", "r1");
 
 	return retval;
-- 
1.6.3.3


             reply	other threads:[~2011-04-07 12:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-07 12:57 Srinivas KANDAGATLA [this message]
2011-04-07 16:53 ` sh:fixed issue in xchg_u32 function when val==r15 Paul Mundt
2011-04-19 16:28 ` Srinivas KANDAGATLA
2011-06-08  6:51 ` Paul Mundt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D9DB4A5.4010002@st.com \
    --to=srinivas.kandagatla@st.com \
    --cc=linux-sh@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.