Re: [PATCH] x86_64: fix delayed signals

[Andi Kleen]

Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
>
> That said, seeing the full history of this same thing on the x86-32 side 
> actually makes me _much_ happier about the patch. Because now I can tell 
> when we did it, and what problems it seems to have caused (answer: 
> "apparently none that are possibly relevant on x86-64").

FWIW I agree with Roland that the 64bit behaviour has a race window
that can lead to delayed signals. I had actually fixed this some
months ago myself in my tree because this was noticed by David (who
was comparing to sparc I thin) during code review, but never pushed it
out because it didn't get enough testing (it only ran on my
workstation for some time)

I think his patch is actually doing way too much work though, especially
it is not needed to repeat all checks for the full work mask. I'm
appending the patch I was using, which only rechecks signals.

Obviously it's still not something that should be used without a lot 
of testing.

But in theory it's ok.

> So now I'm considering just putting it in before the 2.6.26 release after 
> all ;)

That would seem rather hazardous.

-Andi

---

Fix lost signal race on x86-64

Originally noticed by Dave Miller during code review

Signed-off-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>

---
 arch/x86/kernel/entry_64.S |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux/arch/x86/kernel/entry_64.S
===================================================================
--- linux.orig/arch/x86/kernel/entry_64.S
+++ linux/arch/x86/kernel/entry_64.S
@@ -305,7 +305,7 @@ sysret_signal:
 	leaq -ARGOFFSET(%rsp),%rdi # &pt_regs -> arg1
 	xorl %esi,%esi # oldset -> arg2
 	call ptregscall_common
-1:	movl $_TIF_NEED_RESCHED,%edi
+1:	movl $_TIF_NEED_RESCHED|_TIF_SIGPENDING,%edi
 	/* Use IRET because user could have changed frame. This
 	   works because ptregscall_common has called FIXUP_TOP_OF_STACK. */
 	DISABLE_INTERRUPTS(CLBR_NONE)
@@ -393,7 +393,7 @@ int_signal:
 	movq %rsp,%rdi		# &ptregs -> arg1
 	xorl %esi,%esi		# oldset -> arg2
 	call do_notify_resume
-1:	movl $_TIF_NEED_RESCHED,%edi	
+1:	movl $_TIF_NEED_RESCHED|_TIF_SIGPENDING,%edi
 int_restore_rest:
 	RESTORE_REST
 	DISABLE_INTERRUPTS(CLBR_NONE)
@@ -647,7 +647,7 @@ retint_signal:
 	RESTORE_REST
 	DISABLE_INTERRUPTS(CLBR_NONE)
 	TRACE_IRQS_OFF
-	movl $_TIF_NEED_RESCHED,%edi
+	movl $_TIF_NEED_RESCHED|_TIF_SIGPENDING,%edi
 	GET_THREAD_INFO(%rcx)
 	jmp retint_check
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/