linux-kernel - Re: [PATCH] x86_64: fix delayed signals

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87r69z2stg.fsf@basil.nowhere.org>
Date:	Sat, 12 Jul 2008 14:24:59 +0200
From:	Andi Kleen <andi@...stfloor.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Roland McGrath <roland@...hat.com>, Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86_64: fix delayed signals

Linus Torvalds <torvalds@...ux-foundation.org> 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@...ux.intel.com>

---
 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/