linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Holmes - Sun Microsystems <David.Holmes@Sun.COM>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@elte.hu>,
	Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH -v2] fix for futex_wait signal stack corruption
Date: Wed, 05 Dec 2007 16:14:46 +1000	[thread overview]
Message-ID: <475641D6.1060409@sun.com> (raw)
In-Reply-To: <alpine.LFD.0.9999.0712042151250.13796@woody.linux-foundation.org>

Thanks for clarifying that Linus.

Regards,
David Holmes

Linus Torvalds said the following on  5/12/07 04:06 PM:
> 
> On Wed, 5 Dec 2007, David Holmes - Sun Microsystems wrote:
>> While this was observed with process control signals, my concern was that
>> other signals might cause pthread_cond_timedwait to return immediately in the
>> same way. The test program allows for SIGUSR1 and SIGRTMIN testing as well,
>> but these other signals did not cause the immediate return. But it would seem
>> from Steven's analysis that this is just a fortuitous result. If I understand
>> things correctly, any interruption of pthread_cond_timedwait by a signal,
>> could result in waiting until an arbitrary time - depending on how the stack
>> value was corrupted. Is that correct?
> 
> No, very few things can actually cause the restart_block path to be taken. 
> An actual signal execution would turn that into an EINTR, the only case 
> that should ever trigger this is a signal that causes some kernel action 
> (ie the system call *is* interrupted), but does not actually result in any 
> user-visible state changes.
> 
> The classic case is ^Z + bg, but iirc you can trigger it with ptrace too. 
> And I think two threads racing to pick up the same signal can cause it 
> too, for that matter (ie one thread takes the signal, the other one got 
> interrupted but there's nothing there, so it just causes a system call 
> restart).
> 
> There's basically two different system call restart mechanisms in the 
> kernel:
> 
>  - returning -ERESTARTNOHAND will cause the system call to be restarted 
>    with the *original* arguments if no signal handler was actually 
>    invoked. This has been around for a long time, and is used by a lot of 
>    system calls. It's fine for things that are idempotent, ie the argument 
>    meaning doesn't change over time (things like a "read()" system call, 
>    for example)
> 
>  - the "restart_block" model that returns -ERESTARTBLOCK, which will cause 
>    the system call to be restarted with the arguments specified in the 
>    system call restart block. This is for system calls that are *not* 
>    idempotent, ie the argument might be a relative timeout or something 
>    like that, where we need to actually behave *differently* when 
>    restarting.
> 
> The latter case is "new" (it's been around for a while, but relative to 
> the ERESTARTNOHAND one), and it relies on the system call itself setting 
> up its restart point and the argument save area. And each such system call 
> can obviously screw it up by saving/restoring the arguments with the 
> incorrect semantics.
> 
> So this bug was really (a) specific to that particular futex restart 
> mechanism, and (b) only triggers for the (rather unusual) case where the 
> system call gets interrupted by a signal, but no signal handler actually 
> happens. In practice, ^Z is the most common case by far (other signals are 
> either ignored and don't even cause an interrupt event in the first place, 
> or they are "real" signals, and cause a signal handler to be invoked).
> 
> 			Linus

  reply	other threads:[~2007-12-05  6:14 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-04 20:57 [PATCH] fix for futex_wait signal stack corruption Steven Rostedt
2007-12-04 21:09 ` Linus Torvalds
2007-12-04 21:39   ` Steven Rostedt
2007-12-04 22:43     ` Linus Torvalds
2007-12-05  1:23       ` Steven Rostedt
2007-12-05  1:46         ` Linus Torvalds
2007-12-05  3:17           ` [PATCH -v2] " Steven Rostedt
2007-12-05  3:41             ` Linus Torvalds
2007-12-05  3:47               ` Randy Dunlap
2007-12-05  3:53                 ` Steven Rostedt
2007-12-05  5:33               ` David Holmes - Sun Microsystems
2007-12-05  6:06                 ` Linus Torvalds
2007-12-05  6:14                   ` David Holmes - Sun Microsystems [this message]
2007-12-05  5:54               ` Thomas Gleixner
2007-12-05 10:31                 ` Ingo Molnar

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=475641D6.1060409@sun.com \
    --to=david.holmes@sun.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).