From: Oleg Nesterov <oleg@redhat.com>
To: Tejun Heo <tj@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>
Cc: vda.linux@googlemail.com, jan.kratochvil@redhat.com,
pedro@codesourcery.com, indan@nul.nu, bdonlan@gmail.com,
linux-kernel@vger.kernel.org
Subject: [PATCH 1/1] ptrace: fix ptrace_signal() && STOP_DEQUEUED interaction
Date: Thu, 7 Jul 2011 21:03:24 +0200 [thread overview]
Message-ID: <20110707190324.GB25332@redhat.com> (raw)
In-Reply-To: <20110707190301.GA25332@redhat.com>
Simple test-case,
int main(void)
{
int pid, status;
pid = fork();
if (!pid) {
pause();
assert(0);
return 0x23;
}
assert(ptrace(PTRACE_ATTACH, pid, 0,0) == 0);
assert(wait(&status) == pid);
assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
kill(pid, SIGCONT); // <--- also clears STOP_DEQUEUD
assert(ptrace(PTRACE_CONT, pid, 0,0) == 0);
assert(wait(&status) == pid);
assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGCONT);
assert(ptrace(PTRACE_CONT, pid, 0, SIGSTOP) == 0);
assert(wait(&status) == pid);
assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP);
kill(pid, SIGKILL);
return 0;
}
Without the patch it hangs. After the patch SIGSTOP "injected" by the
tracer is not ignored and stops the tracee.
Note also that if this test-case uses, say, SIGWINCH instead of SIGCONT,
everything works without the patch. This can't be right, and this is
confusing.
The problem is that SIGSTOP (or any other sig_kernel_stop() signal) has
no effect without JOBCTL_STOP_DEQUEUED. This means it is simply ignored
after PTRACE_CONT unless JOBCTL_STOP_DEQUEUED was set "by accident", say
it wasn't cleared after initial SIGSTOP sent by PTRACE_ATTACH.
At first glance we could change ptrace_signal() to add STOP_DEQUEUED
after return from ptrace_stop(), but this is not right in case when the
tracer does not change the reported SIGSTOP and SIGCONT comes in between.
This is even more wrong with PT_SEIZED, SIGCONT adds JOBCTL_TRAP_NOTIFY
which will be "lost" during the TRAP_STOP | TRAP_NOTIFY report.
So lets add STOP_DEQUEUED _before_ we report the signal. It has no effect
unless sig_kernel_stop() == T after the tracer resumes us, and in the
latter case the pending STOP_DEQUEUED means no SIGCONT in between, we
should stop.
Note also that if SIGCONT was sent, PT_SEIZED tracee will correctly
report PTRACE_EVENT_STOP/SIGTRAP and thus the tracer can notice the fact
SIGSTOP was cancelled.
Also, move the current->ptrace check from ptrace_signal() to its caller,
get_signal_to_deliver(), this looks more natural.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/signal.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
--- ptrace/kernel/signal.c~1_ptrace_signal_stop_dequeued 2011-07-07 18:34:44.000000000 +0200
+++ ptrace/kernel/signal.c 2011-07-07 20:34:41.000000000 +0200
@@ -2084,12 +2084,13 @@ static void do_jobctl_trap(void)
static int ptrace_signal(int signr, siginfo_t *info,
struct pt_regs *regs, void *cookie)
{
- if (!current->ptrace)
- return signr;
-
ptrace_signal_deliver(regs, cookie);
-
- /* Let the debugger run. */
+ /*
+ * Debugger can change sig_kernel_stop(signr) from F to T,
+ * in this case we should stop iff no SIGCONT in between.
+ * Otherwise this JOBCTL_STOP_DEQUEUED has no effect.
+ */
+ current->jobctl |= JOBCTL_STOP_DEQUEUED;
ptrace_stop(signr, CLD_TRAPPED, 0, info);
/* We're back. Did the debugger cancel the sig? */
@@ -2193,7 +2194,7 @@ relock:
if (!signr)
break; /* will return 0 */
- if (signr != SIGKILL) {
+ if (unlikely(current->ptrace) && signr != SIGKILL) {
signr = ptrace_signal(signr, info,
regs, cookie);
if (!signr)
next prev parent reply other threads:[~2011-07-07 19:06 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-07 19:03 [PATCH 0/1] ptrace: fix ptrace_signal() && STOP_DEQUEUED interaction Oleg Nesterov
2011-07-07 19:03 ` Oleg Nesterov [this message]
2011-07-13 10:04 ` [PATCH 1/1] " Tejun Heo
2011-07-13 18:33 ` Oleg Nesterov
2011-07-14 6:45 ` Tejun Heo
2011-07-14 19:12 ` Oleg Nesterov
2011-07-17 19:03 ` Oleg Nesterov
2011-07-20 16:44 ` Tejun Heo
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=20110707190324.GB25332@redhat.com \
--to=oleg@redhat.com \
--cc=bdonlan@gmail.com \
--cc=indan@nul.nu \
--cc=jan.kratochvil@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pedro@codesourcery.com \
--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 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.