linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: jan.kratochvil@redhat.com, vda.linux@googlemail.com,
	linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, indan@nul.nu
Subject: Re: [PATCH 10/11] ptrace: move JOBCTL_TRAPPING wait to wait(2) and ptrace_check_attach()
Date: Thu, 12 May 2011 20:20:06 +0200	[thread overview]
Message-ID: <20110512182006.GA25850@redhat.com> (raw)
In-Reply-To: <20110512160709.GL1030@htj.dyndns.org>

On 05/12, Tejun Heo wrote:
>
> On Thu, May 12, 2011 at 05:59:10PM +0200, Oleg Nesterov wrote:
>
> > Also. _Perhaps_ we can rethink the SIGCONT trapping, and perhaps in
> > this case do_wait() won't need any changes. May be.
>
> But, if there's a better way, sure.

Unfortunately, I have no any particular ideas right now, and I doubt
I can invent something clean. But I'd like at least to try to think
a bit.


Now about GROUP_STOP_TRAPPING. To simplify the discussion, lets
forget about this series, please recall the previous
"ptrace: use GROUP_STOP_TRAPPING for PTRACE_DETACH too" change.

http://marc.info/?l=linux-kernel&m=130486589601593

In short, it changes __ptrace_unlink() to set _TRAPPING if needed,
and ptrace_attach() waits for !TRAPPING unconditionally.

Problem: TRAPPING can be set outside of do_signal_stop() paths, and
I think we should avoid this as much as possible.

(I am ignoring the problem this patch addresses temporary, I think
 we can fix it a bit differently).

As we already discussed, this patch is not right, we have the problems
with KILL/CONT. The proposed solution is to clear TRAPPING on kill,
but I think this is not enough.

One particular example. Note that de_thread() waits ->notify_count == 0
in TASK_UNINTERRUPTIBLE. Btw, this is not good, we need TASK_KILLABLE,
but this doesn't matter in this discussion. The only imporant thing is
that it is practically impossible to make this path restartable.

Note also we have PT_TRACE_EXIT, the sub-thread stops even if killed by
the execing thread. (to clarify, this depends on /dev/random and should
be fixed, and in fact it is debatable whether it should stop, please
ignore).

Now suppose we are tracing 2 threads, T1 execs and kills T2, T2 reports
PTRACE_EVENT_EXIT. Now, if the tracer waits for !(T1 & TRAPPING), it will
wait forever.


Say, the thread group was stopped, the tracer PTRACE_CONT's T1, it calls
sys_execve() and reports the trap from syscall_trace_enter().

The tracer does ptrace(T1, DETACH) + ptrace(T1, SEIZE) and hangs forever.


Once again, this is only one example. coredump, vfork, probably something
else. In short: I think that TRAPPING-outside-of-do_signal_stop is the
can of worms.

Oleg.


  reply	other threads:[~2011-05-12 18:21 UTC|newest]

Thread overview: 115+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-08 15:48 [PATCHSET ptrace] ptrace: implement PTRACE_SEIZE/INTERRUPT and group stop notification Tejun Heo
2011-05-08 15:48 ` [PATCH 01/11] job control: rename signal->group_stop and flags to jobctl and rearrange flags Tejun Heo
2011-05-08 15:48 ` [PATCH 02/11] ptrace: implement PTRACE_SEIZE Tejun Heo
2011-05-09 16:18   ` Oleg Nesterov
2011-05-10  9:46     ` Tejun Heo
2011-05-10 13:20       ` Oleg Nesterov
2011-05-10 13:47         ` Tejun Heo
2011-05-10 18:19           ` Oleg Nesterov
2011-05-15 15:56   ` PTRACE_SEIZE should not stop [Re: [PATCH 02/11] ptrace: implement PTRACE_SEIZE] Jan Kratochvil
2011-05-15 16:26     ` Tejun Heo
2011-05-15 17:15       ` Jan Kratochvil
2011-05-15 17:25         ` Tejun Heo
2011-05-15 19:48           ` Jan Kratochvil
2011-05-16  8:31             ` Tejun Heo
2011-05-16 12:26               ` Jan Kratochvil
2011-05-16 12:42                 ` Tejun Heo
2011-05-16 13:03                   ` Jan Kratochvil
2011-05-16 13:51                     ` Tejun Heo
2011-05-16 13:21               ` Jan Kratochvil
2011-05-16 13:45                 ` Tejun Heo
2011-05-16 13:48                   ` Jan Kratochvil
2011-05-16 13:54                     ` Tejun Heo
2011-05-08 15:48 ` [PATCH 03/11] ptrace: ptrace_check_attach(): rename @kill to @ignore_state and add comments Tejun Heo
2011-05-08 15:48 ` [PATCH 04/11] ptrace: implement PTRACE_INTERRUPT Tejun Heo
2011-05-08 21:58   ` Denys Vlasenko
2011-05-09 10:09     ` Tejun Heo
2011-05-09 10:55       ` Denys Vlasenko
2011-05-09 16:58   ` Oleg Nesterov
2011-05-10  9:50     ` Tejun Heo
2011-05-10 14:06       ` Oleg Nesterov
2011-05-10 14:20         ` Tejun Heo
2011-05-10 18:08           ` Oleg Nesterov
2011-05-11  8:29             ` Tejun Heo
2011-05-12 17:06               ` Oleg Nesterov
2011-05-12 17:21                 ` Tejun Heo
2011-05-10 21:59         ` Denys Vlasenko
2011-05-11  9:19           ` Tejun Heo
2011-05-11 12:23             ` Denys Vlasenko
2011-05-11 13:22               ` Tejun Heo
2011-05-11 16:20                 ` Bryan Donlan
2011-05-11 19:24                   ` Tejun Heo
2011-05-15 16:10             ` PTRACE_DETACH without stop [Re: [PATCH 04/11] ptrace: implement PTRACE_INTERRUPT] Jan Kratochvil
2011-05-15 16:35               ` Tejun Heo
2011-05-15 17:39                 ` Jan Kratochvil
2011-05-16  9:01                   ` Tejun Heo
2011-05-16 12:08                     ` Jan Kratochvil
2011-05-16 12:24                       ` Tejun Heo
2011-05-08 15:48 ` [PATCH 05/11] ptrace: restructure ptrace_getsiginfo() Tejun Heo
2011-05-08 15:49 ` [PATCH 06/11] ptrace: make group stop state visible via PTRACE_GETSIGINFO Tejun Heo
2011-05-10 16:55   ` Oleg Nesterov
2011-05-10 17:11     ` Oleg Nesterov
2011-05-11  8:08     ` Tejun Heo
2011-05-12 16:47       ` Oleg Nesterov
2011-05-12 17:15         ` Tejun Heo
2011-05-08 15:49 ` [PATCH 07/11] ptrace: add JOBCTL_TRAPPED Tejun Heo
2011-05-08 15:49 ` [PATCH 08/11] ptrace: move fallback JOBCTL_TRAPPING clearing to get_signal_to_deliver() Tejun Heo
2011-05-11 15:48   ` Oleg Nesterov
2011-05-11 19:17     ` Tejun Heo
2011-05-12 15:40       ` Oleg Nesterov
2011-05-08 15:49 ` [PATCH 09/11] job control: reorganize wait_task_stopped() Tejun Heo
2011-05-11 15:48   ` Oleg Nesterov
2011-05-11 19:29     ` Tejun Heo
2011-05-12 15:42       ` Oleg Nesterov
2011-05-12 16:02         ` Tejun Heo
2011-05-12 17:25           ` Oleg Nesterov
2011-05-12 17:32             ` Tejun Heo
2011-05-12 17:33               ` Tejun Heo
2011-05-12 18:33               ` Oleg Nesterov
2011-05-13  8:46                 ` Tejun Heo
2011-05-13 17:21                   ` Oleg Nesterov
2011-05-14 10:56                     ` Tejun Heo
2011-05-15 14:40               ` waitpid(WNOHANG) should report SIGCHLD-notified signals [Re: [PATCH 09/11] job control: reorganize wait_task_stopped()] Jan Kratochvil
2011-05-15 16:47                 ` Tejun Heo
2011-05-15 17:01                   ` Tejun Heo
2011-05-15 17:47                   ` Jan Kratochvil
2011-05-16  9:13                     ` Tejun Heo
2011-05-16 12:11                       ` Jan Kratochvil
2011-05-16 12:27                         ` Tejun Heo
2011-05-16 12:39                           ` Jan Kratochvil
2011-05-16 12:46                             ` Tejun Heo
2011-05-08 15:49 ` [PATCH 10/11] ptrace: move JOBCTL_TRAPPING wait to wait(2) and ptrace_check_attach() Tejun Heo
2011-05-11 16:49   ` Oleg Nesterov
2011-05-11 17:00     ` Oleg Nesterov
2011-05-11 19:45       ` Tejun Heo
2011-05-11 19:53     ` Tejun Heo
2011-05-12 10:23       ` Tejun Heo
2011-05-12 16:06         ` Oleg Nesterov
2011-05-12 15:59       ` Oleg Nesterov
2011-05-12 16:07         ` Tejun Heo
2011-05-12 18:20           ` Oleg Nesterov [this message]
2011-05-13  9:13             ` Tejun Heo
2011-05-13 18:34               ` Oleg Nesterov
2011-05-08 15:49 ` [PATCH 11/11] ptrace: implement group stop notification for ptracer Tejun Heo
2011-05-08 22:42   ` Denys Vlasenko
2011-05-09 10:10     ` Tejun Heo
2011-05-10 22:37   ` Denys Vlasenko
2011-05-11  9:05     ` Tejun Heo
2011-05-11 12:01       ` Denys Vlasenko
2011-05-11 13:13         ` Tejun Heo
2011-05-11 19:58   ` Oleg Nesterov
2011-05-11 20:18     ` Tejun Heo
2011-05-11 20:21       ` Tejun Heo
2011-05-12 10:24         ` Tejun Heo
2011-05-15 14:02   ` getter PTRACE_GETSIGINFO should not modify anything [Re: [PATCH 11/11] ptrace: implement group stop notification for ptracer] Jan Kratochvil
2011-05-15 14:28     ` Tejun Heo
2011-05-15 17:17       ` Jan Kratochvil
2011-05-15 17:28         ` Tejun Heo
2011-05-15 20:06           ` Jan Kratochvil
2011-05-16  8:43             ` Tejun Heo
2011-05-16 12:17               ` Jan Kratochvil
2011-05-16 12:56                 ` Tejun Heo
2011-05-16 13:00                   ` Ingo Molnar
2011-05-08 22:27 ` [PATCHSET ptrace] ptrace: implement PTRACE_SEIZE/INTERRUPT and group stop notification Denys Vlasenko
2011-05-09  9:48   ` Tejun Heo
2011-05-15 13:55   ` ptrace-testsuite status [Re: [PATCHSET ptrace] ptrace: implement PTRACE_SEIZE/INTERRUPT and group stop notification] Jan Kratochvil

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=20110512182006.GA25850@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=indan@nul.nu \
    --cc=jan.kratochvil@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.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 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).