Revision e91467ecd1ef381377fd327c0ded922835ec52ab authored by Christian Borntraeger on 05 August 2006, 19:13:52 UTC, committed by Linus Torvalds on 06 August 2006, 15:57:46 UTC
This patch adds a barrier() in futex unqueue_me to avoid aliasing of two
pointers.
On my s390x system I saw the following oops:
Unable to handle kernel pointer dereference at virtual kernel address
0000000000000000
Oops: 0004 [#1]
CPU: 0 Not tainted
Process mytool (pid: 13613, task: 000000003ecb6ac0, ksp: 00000000366bdbd8)
Krnl PSW : 0704d00180000000 00000000003c9ac2 (_spin_lock+0xe/0x30)
Krnl GPRS: 00000000ffffffff 000000003ecb6ac0 0000000000000000 0700000000000000
0000000000000000 0000000000000000 000001fe00002028 00000000000c091f
000001fe00002054 000001fe00002054 0000000000000000 00000000366bddc0
00000000005ef8c0 00000000003d00e8 0000000000144f91 00000000366bdcb8
Krnl Code: ba 4e 20 00 12 44 b9 16 00 3e a7 84 00 08 e3 e0 f0 88 00 04
Call Trace:
([<0000000000144f90>] unqueue_me+0x40/0xe4)
[<0000000000145a0c>] do_futex+0x33c/0xc40
[<000000000014643e>] sys_futex+0x12e/0x144
[<000000000010bb00>] sysc_noemu+0x10/0x16
[<000002000003741c>] 0x2000003741c
The code in question is:
static int unqueue_me(struct futex_q *q)
{
int ret = 0;
spinlock_t *lock_ptr;
/* In the common case we don't take the spinlock, which is nice. */
retry:
lock_ptr = q->lock_ptr;
if (lock_ptr != 0) {
spin_lock(lock_ptr);
/*
* q->lock_ptr can change between reading it and
* spin_lock(), causing us to take the wrong lock. This
* corrects the race condition.
[...]
and my compiler (gcc 4.1.0) makes the following out of it:
00000000000003c8 <unqueue_me>:
3c8: eb bf f0 70 00 24 stmg %r11,%r15,112(%r15)
3ce: c0 d0 00 00 00 00 larl %r13,3ce <unqueue_me+0x6>
3d0: R_390_PC32DBL .rodata+0x2a
3d4: a7 f1 1e 00 tml %r15,7680
3d8: a7 84 00 01 je 3da <unqueue_me+0x12>
3dc: b9 04 00 ef lgr %r14,%r15
3e0: a7 fb ff d0 aghi %r15,-48
3e4: b9 04 00 b2 lgr %r11,%r2
3e8: e3 e0 f0 98 00 24 stg %r14,152(%r15)
3ee: e3 c0 b0 28 00 04 lg %r12,40(%r11)
/* write q->lock_ptr in r12 */
3f4: b9 02 00 cc ltgr %r12,%r12
3f8: a7 84 00 4b je 48e <unqueue_me+0xc6>
/* if r12 is zero then jump over the code.... */
3fc: e3 20 b0 28 00 04 lg %r2,40(%r11)
/* write q->lock_ptr in r2 */
402: c0 e5 00 00 00 00 brasl %r14,402 <unqueue_me+0x3a>
404: R_390_PC32DBL _spin_lock+0x2
/* use r2 as parameter for spin_lock */
So the code becomes more or less:
if (q->lock_ptr != 0) spin_lock(q->lock_ptr)
instead of
if (lock_ptr != 0) spin_lock(lock_ptr)
Which caused the oops from above.
After adding a barrier gcc creates code without this problem:
[...] (the same)
3ee: e3 c0 b0 28 00 04 lg %r12,40(%r11)
3f4: b9 02 00 cc ltgr %r12,%r12
3f8: b9 04 00 2c lgr %r2,%r12
3fc: a7 84 00 48 je 48c <unqueue_me+0xc4>
400: c0 e5 00 00 00 00 brasl %r14,400 <unqueue_me+0x38>
402: R_390_PC32DBL _spin_lock+0x2
As a general note, this code of unqueue_me seems a bit fishy. The retry logic
of unqueue_me only works if we can guarantee, that the original value of
q->lock_ptr is always a spinlock (Otherwise we overwrite kernel memory). We
know that q->lock_ptr can change. I dont know what happens with the original
spinlock, as I am not an expert with the futex code.
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Acked-by: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@timesys.com>
Signed-off-by: Christian Borntraeger <borntrae@de.ibm.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
1 parent 72f0b4e
termbits.h
#ifndef _ALPHA_TERMBITS_H
#define _ALPHA_TERMBITS_H
#include <linux/posix_types.h>
typedef unsigned char cc_t;
typedef unsigned int speed_t;
typedef unsigned int tcflag_t;
/*
* termios type and macro definitions. Be careful about adding stuff
* to this file since it's used in GNU libc and there are strict rules
* concerning namespace pollution.
*/
#define NCCS 19
struct termios {
tcflag_t c_iflag; /* input mode flags */
tcflag_t c_oflag; /* output mode flags */
tcflag_t c_cflag; /* control mode flags */
tcflag_t c_lflag; /* local mode flags */
cc_t c_cc[NCCS]; /* control characters */
cc_t c_line; /* line discipline (== c_cc[19]) */
speed_t c_ispeed; /* input speed */
speed_t c_ospeed; /* output speed */
};
/* c_cc characters */
#define VEOF 0
#define VEOL 1
#define VEOL2 2
#define VERASE 3
#define VWERASE 4
#define VKILL 5
#define VREPRINT 6
#define VSWTC 7
#define VINTR 8
#define VQUIT 9
#define VSUSP 10
#define VSTART 12
#define VSTOP 13
#define VLNEXT 14
#define VDISCARD 15
#define VMIN 16
#define VTIME 17
/* c_iflag bits */
#define IGNBRK 0000001
#define BRKINT 0000002
#define IGNPAR 0000004
#define PARMRK 0000010
#define INPCK 0000020
#define ISTRIP 0000040
#define INLCR 0000100
#define IGNCR 0000200
#define ICRNL 0000400
#define IXON 0001000
#define IXOFF 0002000
#define IXANY 0004000
#define IUCLC 0010000
#define IMAXBEL 0020000
#define IUTF8 0040000
/* c_oflag bits */
#define OPOST 0000001
#define ONLCR 0000002
#define OLCUC 0000004
#define OCRNL 0000010
#define ONOCR 0000020
#define ONLRET 0000040
#define OFILL 00000100
#define OFDEL 00000200
#define NLDLY 00001400
#define NL0 00000000
#define NL1 00000400
#define NL2 00001000
#define NL3 00001400
#define TABDLY 00006000
#define TAB0 00000000
#define TAB1 00002000
#define TAB2 00004000
#define TAB3 00006000
#define CRDLY 00030000
#define CR0 00000000
#define CR1 00010000
#define CR2 00020000
#define CR3 00030000
#define FFDLY 00040000
#define FF0 00000000
#define FF1 00040000
#define BSDLY 00100000
#define BS0 00000000
#define BS1 00100000
#define VTDLY 00200000
#define VT0 00000000
#define VT1 00200000
#define XTABS 01000000 /* Hmm.. Linux/i386 considers this part of TABDLY.. */
/* c_cflag bit meaning */
#define CBAUD 0000037
#define B0 0000000 /* hang up */
#define B50 0000001
#define B75 0000002
#define B110 0000003
#define B134 0000004
#define B150 0000005
#define B200 0000006
#define B300 0000007
#define B600 0000010
#define B1200 0000011
#define B1800 0000012
#define B2400 0000013
#define B4800 0000014
#define B9600 0000015
#define B19200 0000016
#define B38400 0000017
#define EXTA B19200
#define EXTB B38400
#define CBAUDEX 0000000
#define B57600 00020
#define B115200 00021
#define B230400 00022
#define B460800 00023
#define B500000 00024
#define B576000 00025
#define B921600 00026
#define B1000000 00027
#define B1152000 00030
#define B1500000 00031
#define B2000000 00032
#define B2500000 00033
#define B3000000 00034
#define B3500000 00035
#define B4000000 00036
#define CSIZE 00001400
#define CS5 00000000
#define CS6 00000400
#define CS7 00001000
#define CS8 00001400
#define CSTOPB 00002000
#define CREAD 00004000
#define PARENB 00010000
#define PARODD 00020000
#define HUPCL 00040000
#define CLOCAL 00100000
#define CMSPAR 010000000000 /* mark or space (stick) parity */
#define CRTSCTS 020000000000 /* flow control */
/* c_lflag bits */
#define ISIG 0x00000080
#define ICANON 0x00000100
#define XCASE 0x00004000
#define ECHO 0x00000008
#define ECHOE 0x00000002
#define ECHOK 0x00000004
#define ECHONL 0x00000010
#define NOFLSH 0x80000000
#define TOSTOP 0x00400000
#define ECHOCTL 0x00000040
#define ECHOPRT 0x00000020
#define ECHOKE 0x00000001
#define FLUSHO 0x00800000
#define PENDIN 0x20000000
#define IEXTEN 0x00000400
/* Values for the ACTION argument to `tcflow'. */
#define TCOOFF 0
#define TCOON 1
#define TCIOFF 2
#define TCION 3
/* Values for the QUEUE_SELECTOR argument to `tcflush'. */
#define TCIFLUSH 0
#define TCOFLUSH 1
#define TCIOFLUSH 2
/* Values for the OPTIONAL_ACTIONS argument to `tcsetattr'. */
#define TCSANOW 0
#define TCSADRAIN 1
#define TCSAFLUSH 2
#endif /* _ALPHA_TERMBITS_H */
Computing file changes ...
