All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Roland McGrath <roland@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Jerome Marchand <jmarchan@redhat.com>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] ptrace_detach: the wrong wakeup breaks the ERESTARTxxx logic
Date: Mon, 9 Feb 2009 03:05:15 +0100	[thread overview]
Message-ID: <20090209020515.GA28280@redhat.com> (raw)
In-Reply-To: <20090209013455.CB996FC330@magilla.sf.frob.com>

On 02/08, Roland McGrath wrote:
>
> > This is because ptrace_detach does:
> >
> > 	if (!child->exit_state)
> > 		wake_up_process(child);
>
> I'm pretty sure that all these uses of wake_up_process were just blindly
> copied from an original use in ptrace code (what's now ptrace_resume).
> That original use just dates from the beforetime, the long long ago.
> (I don't think it indicates any coherent original intent.)
>
> It's many kinds of wrong.  It's also always been wrong in case of a
> simultaneous SIGKILL that already woke the child, which has then blocked on
> some mutex or semaphore or whatnot.  I don't know what the stated general
> policy about spurious wakeups from schedule() is supposed to be.  Perhaps
> to be pedantic, the sys_pause() code has been wrong to return without
> checking signal_pending().

Yes, I thought about fixing sys_pause() too, but I'm afraid we can have
the similar code.

> Frankly, I've always been afraid of strange cruft that might unexpectedly
> turn out to rely on this "wrong" (unconditional) wake-up.  Probably the
> things like that historically were all just to do with the stopped/traced
> bookkeeping and would be covered by explicitly dealing with PTRACE_CONT vs
> group stop et al.  But FWIW my reaction to fiddling the wake_up_process
> bogons in the past has been, "Be afraid."

Yes, I am afraid, seriously ;)

This can reveal other subtle problems, of course. But there is another reason
why this wakeup is wrong. It clearly breaks the SIGNAL_GROUP_STOPPED logic
in ptrace_untrace().

Oleg.


      reply	other threads:[~2009-02-09  2:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-08 18:47 [PATCH 1/3] ptrace_detach: the wrong wakeup breaks the ERESTARTxxx logic Oleg Nesterov
2009-02-09  1:34 ` Roland McGrath
2009-02-09  2:05   ` Oleg Nesterov [this message]

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=20090209020515.GA28280@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dvlasenk@redhat.com \
    --cc=jmarchan@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roland@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.