* [patch] pi-futex: futex_wake() lockup fix
@ 2006-07-01 7:07 Ingo Molnar
2006-07-01 7:23 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Ingo Molnar @ 2006-07-01 7:07 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: linux-kernel, Dave Jones, Thomas Gleixner, Ulrich Drepper
Subject: pi-futex: futex_wake() lockup fix
From: Ingo Molnar <mingo@elte.hu>
fix futex_wake() exit condition bug when handling the robust-list with
PI futexes on them.
(reported by Ulrich Drepper, debugged by the lock validator.)
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/futex.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Index: linux/kernel/futex.c
===================================================================
--- linux.orig/kernel/futex.c
+++ linux/kernel/futex.c
@@ -646,8 +646,10 @@ static int futex_wake(u32 __user *uaddr,
list_for_each_entry_safe(this, next, head, list) {
if (match_futex (&this->key, &key)) {
- if (this->pi_state)
- return -EINVAL;
+ if (this->pi_state) {
+ ret = -EINVAL;
+ break;
+ }
wake_futex(this);
if (++ret >= nr_wake)
break;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] pi-futex: futex_wake() lockup fix
2006-07-01 7:07 [patch] pi-futex: futex_wake() lockup fix Ingo Molnar
@ 2006-07-01 7:23 ` Andrew Morton
2006-07-01 7:25 ` Ingo Molnar
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2006-07-01 7:23 UTC (permalink / raw)
To: Ingo Molnar; +Cc: torvalds, linux-kernel, davej, tglx, drepper
On Sat, 1 Jul 2006 09:07:46 +0200
Ingo Molnar <mingo@elte.hu> wrote:
> Subject: pi-futex: futex_wake() lockup fix
> From: Ingo Molnar <mingo@elte.hu>
>
> fix futex_wake() exit condition bug when handling the robust-list with
> PI futexes on them.
>
> (reported by Ulrich Drepper, debugged by the lock validator.)
>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
> kernel/futex.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Index: linux/kernel/futex.c
> ===================================================================
> --- linux.orig/kernel/futex.c
> +++ linux/kernel/futex.c
> @@ -646,8 +646,10 @@ static int futex_wake(u32 __user *uaddr,
>
> list_for_each_entry_safe(this, next, head, list) {
> if (match_futex (&this->key, &key)) {
> - if (this->pi_state)
> - return -EINVAL;
> + if (this->pi_state) {
> + ret = -EINVAL;
> + break;
> + }
> wake_futex(this);
> if (++ret >= nr_wake)
> break;
Well that was rather a howler.
How did the lock validator help in identifying this?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] pi-futex: futex_wake() lockup fix
2006-07-01 7:23 ` Andrew Morton
@ 2006-07-01 7:25 ` Ingo Molnar
0 siblings, 0 replies; 3+ messages in thread
From: Ingo Molnar @ 2006-07-01 7:25 UTC (permalink / raw)
To: Andrew Morton; +Cc: torvalds, linux-kernel, davej, tglx, drepper
* Andrew Morton <akpm@osdl.org> wrote:
> > if (match_futex (&this->key, &key)) {
> > - if (this->pi_state)
> > - return -EINVAL;
> > + if (this->pi_state) {
> > + ret = -EINVAL;
> > + break;
> > + }
> > wake_futex(this);
> > if (++ret >= nr_wake)
> > break;
>
> Well that was rather a howler.
yeah :-| This bug was introduced relatively late, as part of another
bugfix.
> How did the lock validator help in identifying this?
i've got an extension for it that moans if we exit a syscall with held
locks. (i dont want to risk the current queue with it, it's below - just
for review)
(but it's not a strict lockdep thing - it should probably be done as
part of DEBUG_LOCK_ALLOC, not LOCKDEP)
anyway, this is for later - it just shows that we can certainly do a
number of nice things with lockdep's lock-stack. [as DEBUG_LOCK_ALLOC
itself already demonstrates]
Ingo
NOT-YET-Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/i386/kernel/entry.S | 15 ++++++++++++
arch/x86_64/ia32/ia32entry.S | 6 +++++
arch/x86_64/kernel/entry.S | 4 +++
include/asm-x86_64/calling.h | 50 +++++++++++++++++++++++++++++++++++++++++++
kernel/lockdep.c | 20 +++++++++++++++++
lib/Kconfig.debug | 4 +++
6 files changed, 99 insertions(+)
Index: linux/arch/i386/kernel/entry.S
===================================================================
--- linux.orig/arch/i386/kernel/entry.S
+++ linux/arch/i386/kernel/entry.S
@@ -308,6 +308,11 @@ sysenter_past_esp:
pushl %eax
CFI_ADJUST_CFA_OFFSET 4
SAVE_ALL
+#ifdef CONFIG_SYSCALL_TRACE
+ pushl %edx; pushl %ecx; pushl %ebx; pushl %eax
+ call sys_call
+ popl %eax; popl %ebx; popl %ecx; popl %edx
+#endif
GET_THREAD_INFO(%ebp)
/* Note, _TIF_SECCOMP is bit number 8, and so it needs testw and not testb */
@@ -322,6 +327,11 @@ sysenter_past_esp:
movl TI_flags(%ebp), %ecx
testw $_TIF_ALLWORK_MASK, %cx
jne syscall_exit_work
+#ifdef CONFIG_SYSCALL_TRACE
+ pushl %eax
+ call sys_ret
+ popl %eax
+#endif
/* if something modifies registers it must also disable sysexit */
movl EIP(%esp), %edx
movl OLDESP(%esp), %ecx
@@ -338,6 +348,11 @@ ENTRY(system_call)
pushl %eax # save orig_eax
CFI_ADJUST_CFA_OFFSET 4
SAVE_ALL
+#ifdef CONFIG_SYSCALL_TRACE
+ pushl %edx; pushl %ecx; pushl %ebx; pushl %eax
+ call sys_call
+ popl %eax; popl %ebx; popl %ecx; popl %edx
+#endif
GET_THREAD_INFO(%ebp)
testl $TF_MASK,EFLAGS(%esp)
jz no_singlestep
Index: linux/arch/x86_64/ia32/ia32entry.S
===================================================================
--- linux.orig/arch/x86_64/ia32/ia32entry.S
+++ linux/arch/x86_64/ia32/ia32entry.S
@@ -119,7 +119,9 @@ sysenter_do_call:
cmpl $(IA32_NR_syscalls-1),%eax
ja ia32_badsys
IA32_ARG_FIXUP 1
+ TRACE_SYS_IA32_CALL
call *ia32_sys_call_table(,%rax,8)
+ TRACE_SYS_RET
movq %rax,RAX-ARGOFFSET(%rsp)
GET_THREAD_INFO(%r10)
cli
@@ -227,7 +229,9 @@ cstar_do_call:
cmpl $IA32_NR_syscalls-1,%eax
ja ia32_badsys
IA32_ARG_FIXUP 1
+ TRACE_SYS_IA32_CALL
call *ia32_sys_call_table(,%rax,8)
+ TRACE_SYS_RET
movq %rax,RAX-ARGOFFSET(%rsp)
GET_THREAD_INFO(%r10)
cli
@@ -320,8 +324,10 @@ ia32_do_syscall:
cmpl $(IA32_NR_syscalls-1),%eax
ja ia32_badsys
IA32_ARG_FIXUP
+ TRACE_SYS_IA32_CALL
call *ia32_sys_call_table(,%rax,8) # xxx: rip relative
ia32_sysret:
+ TRACE_SYS_RET
movq %rax,RAX-ARGOFFSET(%rsp)
jmp int_ret_from_sys_call
Index: linux/arch/x86_64/kernel/entry.S
===================================================================
--- linux.orig/arch/x86_64/kernel/entry.S
+++ linux/arch/x86_64/kernel/entry.S
@@ -222,7 +222,9 @@ ENTRY(system_call)
cmpq $__NR_syscall_max,%rax
ja badsys
movq %r10,%rcx
+ TRACE_SYS_CALL
call *sys_call_table(,%rax,8) # XXX: rip relative
+ TRACE_SYS_RET
movq %rax,RAX-ARGOFFSET(%rsp)
/*
* Syscall return path ending with SYSRET (fast path)
@@ -304,7 +306,9 @@ tracesys:
cmpq $__NR_syscall_max,%rax
ja 1f
movq %r10,%rcx /* fixup for C */
+ TRACE_SYS_CALL
call *sys_call_table(,%rax,8)
+ TRACE_SYS_RET
1: movq %rax,RAX-ARGOFFSET(%rsp)
/* Use IRET because user could have changed frame */
jmp int_ret_from_sys_call
Index: linux/include/asm-x86_64/calling.h
===================================================================
--- linux.orig/include/asm-x86_64/calling.h
+++ linux/include/asm-x86_64/calling.h
@@ -160,3 +160,53 @@
.macro icebp
.byte 0xf1
.endm
+
+/*
+ * syscall-tracing helpers:
+ */
+
+ .macro TRACE_SYS_CALL
+
+#ifdef CONFIG_SYSCALL_TRACE
+ SAVE_ARGS
+
+ mov %rdx, %rcx
+ mov %rsi, %rdx
+ mov %rdi, %rsi
+ mov %rax, %rdi
+
+ call sys_call
+
+ RESTORE_ARGS
+#endif
+ .endm
+
+
+ .macro TRACE_SYS_IA32_CALL
+
+#ifdef CONFIG_SYSCALL_TRACE
+ SAVE_ARGS
+
+ mov %rdx, %rcx
+ mov %rsi, %rdx
+ mov %rdi, %rsi
+ mov %rax, %rdi
+
+ call sys_ia32_call
+
+ RESTORE_ARGS
+#endif
+ .endm
+
+ .macro TRACE_SYS_RET
+
+#ifdef CONFIG_SYSCALL_TRACE
+ SAVE_ARGS
+
+ mov %rax, %rdi
+
+ call sys_ret
+
+ RESTORE_ARGS
+#endif
+ .endm
Index: linux/kernel/lockdep.c
===================================================================
--- linux.orig/kernel/lockdep.c
+++ linux/kernel/lockdep.c
@@ -2701,3 +2701,23 @@ void debug_show_held_locks(struct task_s
EXPORT_SYMBOL_GPL(debug_show_held_locks);
+void
+sys_call(unsigned long nr, unsigned long p1, unsigned long p2, unsigned long p3)
+{
+ DEBUG_LOCKS_WARN_ON(current->lockdep_depth);
+}
+
+#if defined(CONFIG_COMPAT) && defined(CONFIG_X86)
+
+void sys_ia32_call(unsigned long nr, unsigned long p1, unsigned long p2,
+ unsigned long p3)
+{
+ DEBUG_LOCKS_WARN_ON(current->lockdep_depth);
+}
+
+#endif
+
+void sys_ret(unsigned long ret)
+{
+ DEBUG_LOCKS_WARN_ON(current->lockdep_depth);
+}
Index: linux/lib/Kconfig.debug
===================================================================
--- linux.orig/lib/Kconfig.debug
+++ linux/lib/Kconfig.debug
@@ -251,6 +251,10 @@ config LOCKDEP
select FRAME_POINTER
select KALLSYMS
select KALLSYMS_ALL
+ select SYSCALL_TRACE
+
+config SYSCALL_TRACE
+ bool
config DEBUG_LOCKDEP
bool "Lock dependency engine debugging"
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2006-07-01 7:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-07-01 7:07 [patch] pi-futex: futex_wake() lockup fix Ingo Molnar
2006-07-01 7:23 ` Andrew Morton
2006-07-01 7:25 ` Ingo Molnar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox