linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: David Holmes - Sun Microsystems <David.Holmes@Sun.COM>
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: Tue, 4 Dec 2007 22:06:24 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.0.9999.0712042151250.13796@woody.linux-foundation.org> (raw)
In-Reply-To: <4756382D.4040904@sun.com>



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:07 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 [this message]
2007-12-05  6:14                   ` David Holmes - Sun Microsystems
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=alpine.LFD.0.9999.0712042151250.13796@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=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 \
    /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).