public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Roland McGrath <roland@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>,
	Oleg Nesterov <oleg@tv-sign.ru>
Subject: Re: [BUG] ptraced process waiting on syscall may return kernel internal errnos
Date: Thu, 07 Jun 2007 13:20:18 +1000	[thread overview]
Message-ID: <1181186418.14818.5.camel@localhost.localdomain> (raw)
In-Reply-To: <20070606105900.DE5E94D0592@magilla.localdomain>

On Wed, 2007-06-06 at 03:59 -0700, Roland McGrath wrote:
> Oleg and I were just discussing this issue in relation to other problems.
> We established that it is never safe to clear TIF_SIGPENDING on another
> thread.  But I hadn't really thought through that it's sometimes not safe
> to clear your own TIF_SIGPENDING either.  That is, any time you are not
> positive you cannot be in a syscall that will return a -ERESTART* code.
> (I had the ptrace_stop case lurking in the back of my mind but hadn't
> considered how it would really come up.)

Ah, I missed that ptrace_stop() case indeed.

> I have a general recollection of thinking that dequeue_signal could only
> be called on current and that it mattered somehow.

I matter with that, and with the notifier thingy. It might matter for
other things as well, I don't know for sure yet though signalfd will
definitely cause it to be called for !current, though with my patch only
for shared signals.

>   But aside from
> avoiding recalc_sigpending, and kernel threads with notifier_mask set, I
> can't see off hand what it is.  I won't testify that I think signalfd is
> necessarily on safe ground, though.

Yeah, I'm a little worried too, the whole thing seems a bit fragile to
me.

I'm tempted to in fact make dequeue_signal() be the only one to every
clear TIF_SIGPENDING and only when tsk==current, but then, that does
mean there are a few cases, like when explicitely masking signals, where
we might end up with it spurriously set... thus causing spurrious -EINTR
returns for blocked signals.

I must admit at this point, it's becoming almost tempting to split it
into two different flags. TIF_SIGPENDING would then be the exact logic
"there is currently a non blocked signal pending" and could be cleared
at any time provided the siglock is help (which btw, is another source
of headaches if you grep ... people maniupate these things here or there
without the lock). And another one, TIF_SIGNALED, which would be set
always if TIG_SIGPENDING is set, and only ever cleared by
dequeue_signal, and causes the exception path to get into do_signal.

That would mean occasional spurrious trips in do_signal but that's
totally harmless and it wouldn't, I beleive, happen often enough to
impact performances significantly.

Cheers,
Ben.
 


  parent reply	other threads:[~2007-06-07  3:21 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-06  5:47 [BUG] ptraced process waiting on syscall may return kernel internal errnos Satoru Takeuchi
2007-06-06 10:59 ` Roland McGrath
2007-06-06 15:35   ` Linus Torvalds
2007-06-06 23:07     ` Paul Mackerras
2007-06-07  3:25     ` Benjamin Herrenschmidt
2007-06-07  3:27     ` Benjamin Herrenschmidt
2007-06-07 11:33     ` Satoru Takeuchi
2007-06-07 15:54       ` Linus Torvalds
2007-06-07 22:24         ` Benjamin Herrenschmidt
2007-06-08  3:18           ` Linus Torvalds
2007-06-08  5:30             ` Benjamin Herrenschmidt
2007-06-11 22:16             ` Benjamin Herrenschmidt
2007-06-08  3:07         ` Satoru Takeuchi
2007-06-13 22:06     ` Roland McGrath
2007-06-07  3:20   ` Benjamin Herrenschmidt [this message]
2007-06-13 15:15   ` Oleg Nesterov
2007-06-13 22:36     ` Benjamin Herrenschmidt
2007-06-13 23:01       ` Roland McGrath
2007-06-13 23:18         ` Benjamin Herrenschmidt
2007-06-14  0:02           ` Roland McGrath
2007-06-13 22:53     ` Roland McGrath
2007-06-14 12:26     ` Rafael J. Wysocki
2007-06-14 12:58       ` Oleg Nesterov
2007-06-14 23:35         ` Rafael J. Wysocki
2007-06-15 11:31           ` Oleg Nesterov
2007-06-15 21:48             ` Rafael J. Wysocki
2007-06-15  0:06         ` Benjamin Herrenschmidt

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=1181186418.14818.5.camel@localhost.localdomain \
    --to=benh@kernel.crashing.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@tv-sign.ru \
    --cc=roland@redhat.com \
    --cc=takeuchi_satoru@jp.fujitsu.com \
    --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