NetBSD Problem Report #52117
From www@NetBSD.org Tue Mar 28 02:08:09 2017
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 5A5E67A26D
for <gnats-bugs@gnats.NetBSD.org>; Tue, 28 Mar 2017 02:08:09 +0000 (UTC)
Message-Id: <20170328020808.795E47A26D@mollari.NetBSD.org>
Date: Tue, 28 Mar 2017 02:08:08 +0000 (UTC)
From: n54@gmx.com
Reply-To: n54@gmx.com
To: gnats-bugs@NetBSD.org
Subject: ptrace(2) PTRACE_FORK fails on some platforms
X-Send-Pr-Version: www-1.0
>Number: 52117
>Category: kern
>Synopsis: ptrace(2) PTRACE_FORK fails on some platforms
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: kern-bug-people
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Tue Mar 28 02:10:00 +0000 2017
>Closed-Date: Mon Apr 10 16:41:16 +0000 2017
>Last-Modified: Mon Apr 10 16:50:00 +0000 2017
>Originator: Kamil Rytarowski
>Release: NetBSD 7.99.66 amd64
>Organization:
TNF
>Environment:
NetBSD chieftec 7.99.66 NetBSD 7.99.66 (GENERIC) #3: Tue Mar 28 02:01:26 CEST 2017 root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64
>Description:
ptrace(2) PTRACE_FORK events don't work on some platforms, including: alpha, sparc, sparc64, evbarm.
It works on amd64, i386 and xen.
>How-To-Repeat:
tests/kernel/t_ptrace_wait tests siginfo5 and fork1
>Fix:
N/A
>Release-Note:
>Audit-Trail:
From: "Kamil Rytarowski" <kamil@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/52117 CVS commit: src/tests/kernel
Date: Tue, 28 Mar 2017 03:03:15 +0000
Module Name: src
Committed By: kamil
Date: Tue Mar 28 03:03:15 UTC 2017
Modified Files:
src/tests/kernel: t_ptrace_wait.c
Log Message:
Mark fork1 and siginfo5 as broken on sparc, sparc64, evbarm and alpha
PR kern/52117 ptrace(2) PTRACE_FORK fails on some platforms
These tests work on amd64, i386 and xen.
They are part of t_ptrace_wait*.
Sponsored by <The NetBSD Foundation>
To generate a diff of this commit:
cvs rdiff -u -r1.80 -r1.81 src/tests/kernel/t_ptrace_wait.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
Date: Tue, 28 Mar 2017 10:04:40 +0200
Can you please describe this better?
What are the steps to reproduce the issue? Which part of the atf test
output is relevant to this PR?
After digging through the source I guess you have to do:
cd /usr/tests/kernel
atf-run t_ptrace_waitpid
and watch :
tc-start: 1490688009.793401, signal6
tc-so:Before forking process PID=2969
tc-so:Before calling PT_TRACE_ME from child 2722
tc-so:Parent process PID=2969, child's PID=2722
tc-so:Before calling waitpid() for the child
tc-so:Before raising Stopped (signal) from child
tc-so:Enable PTRACE_FORK in EVENT_MASK for the child 2722
tc-so:Before resuming the child process where it left off and without signal to be sent
tc-so:Before calling waitpid() for the child
[.. long pause untill timeout..]
and a fixed kernel would not pause here?
Martin
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
Date: Tue, 28 Mar 2017 10:19:26 +0200
On Tue, Mar 28, 2017 at 10:04:40AM +0200, Martin Husemann wrote:
> tc-so:Before resuming the child process where it left off and without signal to be sent
> tc-so:Before calling waitpid() for the child
Can you please comment the tests some more as well?
There are multiple forks, but when the waitpid() happens only one of them
is still alive, like:
tc-start: 1490688991.212638, signal6
tc-so:Before forking process PID=3246
tc-so:Before calling PT_TRACE_ME from child 3957
tc-so:Before raising Stopped (signal) from child
tc-so:Parent process PID=3246, child's PID=3957
tc-so:Before calling waitpid() for the child
tc-so:Enable PTRACE_FORK in EVENT_MASK for the child 3957
tc-so:Before resuming the child process where it left off and without signal to be sent
tc-so:Before calling waitpid() for the child
PID TTY STAT TIME COMMAND
3246 pts/7 I 0:00.01 t_ptrace_waitpid -r/tmp/atf-run.IcYGNS/tcr -s/usr/tests/kernel -vunprivileged-user signal6:body
3465 pts/7 ZX 0:00.00 (t_ptrace_waitpid)
3957 pts/7 IX 0:00.00 t_ptrace_waitpid -r/tmp/atf-run.IcYGNS/tcr -s/usr/tests/kernel -vunprivileged-user signal6:body
so it would be helpfull if the "Before calling waitpid()" message would
mention the pid, and some comment in the test explain what the other child
is about and what its state should be now (and why you double fork at all).
Martin
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
Date: Tue, 28 Mar 2017 13:39:41 +0200
OK, you have confused me big time.
If not blocking SIGTRAP, everything seems to work as designed on
sparc64. What is this PR about?
Martin
From: "Kamil Rytarowski" <n54@gmx.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
Date: Tue, 28 Mar 2017 14:56:53 +0200
Example taken from evbarm:
Test case: kernel/t_ptrace_wait4/fork1
Duration: 0.030507 seconds
Termination reason
FAILED: /usr/src/tests/kernel/t_ptrace_wait.h:295: Reported exited process
Standard output stream
Before forking process PID=12298
Parent process PID=12298, child's PID=19554
Before calling PT_TRACE_ME from child 19554
Before calling wait4() for the child
Before raising Stopped (signal) from child
Enable PTRACE_FORK in EVENT_MASK for the child 19554
Before resuming the child process where it left off and without signal to be sent
Before calling wait4() for the child 19554
Reported PTRACE_FORK event with forkee 11702
Before calling wait4() for the forkee 11702 of the child 19554
It means that fork(2) must generate two SIGTAP events:
- first for child (traced parent) with TRAP_CHLD, pe_report_event=PTRACE_FORK, pe_other_pid=child2
- second for child2 (traced child of child) with TRAP_CHLD, pe_report_event=PTRACE_FORK, pe_other_pid=child
The test waits infinitely for the second SIGRAP.
There is a hand-shake for forking.
Debuggers calls wait*(2) twice, one after another, without operation of resuming
its traced processes between the wait*(2) calls.
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
Date: Tue, 28 Mar 2017 15:13:36 +0200
On Tue, Mar 28, 2017 at 01:00:01PM +0000, Kamil Rytarowski wrote:
> Before calling wait4() for the child 19554
> Reported PTRACE_FORK event with forkee 11702
> Before calling wait4() for the forkee 11702 of the child 19554
What happens here is (from my local run):
5603 1 t_ptrace_wait4 GIO fd 1 wrote 44 bytes
"Reported PTRACE_FORK event with forkee 5251\n"
5603 1 t_ptrace_wait4 RET write 44/0x2c
5603 1 t_ptrace_wait4 CALL write(1,0xffffffffffffcbf8,0x3d)
5603 1 t_ptrace_wait4 GIO fd 1 wrote 61 bytes
"Before calling wait4() for the forkee 5251 of the child 6264\n"
5603 1 t_ptrace_wait4 RET write 61/0x3d
5603 1 t_ptrace_wait4 CALL __wait450(0x1483,0xffffffffffffd240,0,0)
5251 1 t_ptrace_wait4 EMUL "netbsd"
5251 1 t_ptrace_wait4 RET fork 0
5251 1 t_ptrace_wait4 CALL exit(0xf)
5603 1 t_ptrace_wait4 RET __wait450 5251/0x1483
5603 1 t_ptrace_wait4 CALL writev(1,0xffffffffffffcc70,4)
5603 1 t_ptrace_wait4 GIO fd 1 wrote 75 bytes
"failed: /ssd/src/tests/kernel/t_ptrace_wait.h:295: Reported exited pro\
cess\n"
Now note that wait4 is called on the childs forkee (5251) which does
nothing but exit. So WIFEXITED() is correct, but you probably want
to wait for the child (6264) instead?
Martin
From: "Kamil Rytarowski" <kamil@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/52117 CVS commit: src/tests/kernel
Date: Tue, 28 Mar 2017 13:16:30 +0000
Module Name: src
Committed By: kamil
Date: Tue Mar 28 13:16:30 UTC 2017
Modified Files:
src/tests/kernel: t_ptrace_wait.c
Log Message:
Explain expected behavior for PTRACE_FORK events in t_ptrace_wait*
PR kern/52117 ptrace(2) PTRACE_FORK fails on some platforms
Add message similar to:
We expect two SIGTRAP events, for child 22199 (TRAP_CHLD,
pe_report_event=PTRACE_FORK, state.pe_other_pid=child2) and for child2
(TRAP_CHLD, pe_report_event=PTRACE_FORK, state.pe_other_pid=child)
This should make the expected behavior more clear.
Sponsored by <The NetBSD Foundation>
To generate a diff of this commit:
cvs rdiff -u -r1.83 -r1.84 src/tests/kernel/t_ptrace_wait.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/52117 CVS commit: src/tests/kernel
Date: Tue, 28 Mar 2017 15:37:35 +0200
On Tue, Mar 28, 2017 at 01:20:01PM +0000, Kamil Rytarowski wrote:
> Add message similar to:
> We expect two SIGTRAP events, for child 22199 (TRAP_CHLD,
> pe_report_event=PTRACE_FORK, state.pe_other_pid=child2) and for child2
> (TRAP_CHLD, pe_report_event=PTRACE_FORK, state.pe_other_pid=child)
Where is the kernel code hiding that causes this for the forkee?
I see the code for the process calling fork() at the end of
kern_fork.c, but the "child2" process does not run that code, and I
don't understand how it works on amd64. nxr does not find other kernel
referencs for TRAP_CHLD (and this one works, the forker properly gets
signaled).
Martin
From: "Kamil Rytarowski" <n54@gmx.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/52117 CVS commit: src/tests/kernel
Date: Tue, 28 Mar 2017 15:57:57 +0200
> Sent: Tuesday, March 28, 2017 at 3:40 PM
> From: "Martin Husemann" <martin@duskware.de>
> To: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, n54@gmx.com
> Subject: Re: PR/52117 CVS commit: src/tests/kernel
>
> The following reply was made to PR kern/52117; it has been noted by GNATS.
>
> From: Martin Husemann <martin@duskware.de>
> To: gnats-bugs@NetBSD.org
> Cc:
> Subject: Re: PR/52117 CVS commit: src/tests/kernel
> Date: Tue, 28 Mar 2017 15:37:35 +0200
>
> On Tue, Mar 28, 2017 at 01:20:01PM +0000, Kamil Rytarowski wrote:
> > Add message similar to:
> > We expect two SIGTRAP events, for child 22199 (TRAP_CHLD,
> > pe_report_event=PTRACE_FORK, state.pe_other_pid=child2) and for child2
> > (TRAP_CHLD, pe_report_event=PTRACE_FORK, state.pe_other_pid=child)
>
> Where is the kernel code hiding that causes this for the forkee?
>
> I see the code for the process calling fork() at the end of
> kern_fork.c, but the "child2" process does not run that code, and I
> don't understand how it works on amd64. nxr does not find other kernel
> referencs for TRAP_CHLD (and this one works, the forker properly gets
> signaled).
>
> Martin
>
>
It's a bug in the kernel. It sits hardcoded in x86 specific code:
void
child_return(void *arg)
{
struct lwp *l = arg;
struct trapframe *tf = l->l_md.md_regs;
struct proc *p = l->l_proc;
if (p->p_slflag & PSL_TRACED) {
ksiginfo_t ksi;
mutex_enter(proc_lock);
KSI_INIT_EMPTY(&ksi);
ksi.ksi_signo = SIGTRAP;
ksi.ksi_lid = l->l_lid;
kpsignal(p, &ksi, NULL);
mutex_exit(proc_lock);
}
X86_TF_RAX(tf) = 0;
X86_TF_RFLAGS(tf) &= ~PSL_C;
userret(l);
ktrsysret(SYS_fork, 0, 0);
}
I've verified that this fires SIGTRAP for fork(2).
From: "Kamil Rytarowski" <n54@gmx.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
Date: Tue, 28 Mar 2017 16:00:21 +0200
> Sent: Tuesday, March 28, 2017 at 3:15 PM
> From: "Martin Husemann" <martin@duskware.de>
> To: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, n54@gmx.com
> Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
>
> The following reply was made to PR kern/52117; it has been noted by GNATS.
>
> From: Martin Husemann <martin@duskware.de>
> To: gnats-bugs@NetBSD.org
> Cc:
> Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
> Date: Tue, 28 Mar 2017 15:13:36 +0200
>
> On Tue, Mar 28, 2017 at 01:00:01PM +0000, Kamil Rytarowski wrote:
> > Before calling wait4() for the child 19554
> > Reported PTRACE_FORK event with forkee 11702
> > Before calling wait4() for the forkee 11702 of the child 19554
>
> What happens here is (from my local run):
>
> 5603 1 t_ptrace_wait4 GIO fd 1 wrote 44 bytes
> "Reported PTRACE_FORK event with forkee 5251\n"
> 5603 1 t_ptrace_wait4 RET write 44/0x2c
> 5603 1 t_ptrace_wait4 CALL write(1,0xffffffffffffcbf8,0x3d)
> 5603 1 t_ptrace_wait4 GIO fd 1 wrote 61 bytes
> "Before calling wait4() for the forkee 5251 of the child 6264\n"
> 5603 1 t_ptrace_wait4 RET write 61/0x3d
> 5603 1 t_ptrace_wait4 CALL __wait450(0x1483,0xffffffffffffd240,0,0)
> 5251 1 t_ptrace_wait4 EMUL "netbsd"
> 5251 1 t_ptrace_wait4 RET fork 0
> 5251 1 t_ptrace_wait4 CALL exit(0xf)
> 5603 1 t_ptrace_wait4 RET __wait450 5251/0x1483
> 5603 1 t_ptrace_wait4 CALL writev(1,0xffffffffffffcc70,4)
> 5603 1 t_ptrace_wait4 GIO fd 1 wrote 75 bytes
> "failed: /ssd/src/tests/kernel/t_ptrace_wait.h:295: Reported exited pro\
> cess\n"
>
> Now note that wait4 is called on the childs forkee (5251) which does
> nothing but exit. So WIFEXITED() is correct, but you probably want
> to wait for the child (6264) instead?
>
Yes, we wait for two signals SIGTRAP.
Exactly the same interface is added for PTRACE_VFORK and the same behavior.
(However I have not managed to make it work so far)
> Martin
>
>
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/52117 CVS commit: src/tests/kernel
Date: Tue, 28 Mar 2017 16:35:15 +0200
Not sure I understand all contstraints, and this is completely untested,
but maybe as a base for discussions/testing: could you try this patch
and see if it still works on x86 ?
Martin
Index: kern/kern_fork.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_fork.c,v
retrieving revision 1.199
diff -u -p -r1.199 kern_fork.c
--- kern/kern_fork.c 13 Jan 2017 23:00:35 -0000 1.199
+++ kern/kern_fork.c 28 Mar 2017 14:33:30 -0000
@@ -464,6 +464,17 @@ fork1(struct lwp *l1, int flags, int exi
*/
mutex_enter(proc_lock);
+
+ /* If it is traced, signal the tracer */
+ if (p2->p_slflag & PSL_TRACED) {
+ ksiginfo_t ksi;
+
+ KSI_INIT_EMPTY(&ksi);
+ ksi.ksi_signo = SIGTRAP;
+ ksi.ksi_lid = l2->l_lid;
+ kpsignal(p2, &ksi, NULL);
+ }
+
if (p1->p_session->s_ttyvp != NULL && p1->p_lflag & PL_CONTROLT)
p2->p_lflag |= PL_CONTROLT;
Index: arch/x86/x86/syscall.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/syscall.c,v
retrieving revision 1.14
diff -u -p -r1.14 syscall.c
--- arch/x86/x86/syscall.c 7 Jul 2016 06:55:40 -0000 1.14
+++ arch/x86/x86/syscall.c 28 Mar 2017 14:33:30 -0000
@@ -65,18 +65,6 @@ child_return(void *arg)
{
struct lwp *l = arg;
struct trapframe *tf = l->l_md.md_regs;
- struct proc *p = l->l_proc;
-
- if (p->p_slflag & PSL_TRACED) {
- ksiginfo_t ksi;
-
- mutex_enter(proc_lock);
- KSI_INIT_EMPTY(&ksi);
- ksi.ksi_signo = SIGTRAP;
- ksi.ksi_lid = l->l_lid;
- kpsignal(p, &ksi, NULL);
- mutex_exit(proc_lock);
- }
X86_TF_RAX(tf) = 0;
X86_TF_RFLAGS(tf) &= ~PSL_C;
From: "Kamil Rytarowski" <n54@gmx.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
Date: Tue, 28 Mar 2017 16:44:50 +0200
This child_return has been added a while ago.
I'm not sure if we are supposed to interact with this in the context of emulated ABIs.
Author: christos <christos>
Date: Fri Sep 2 20:01:20 2011 +0000
If the process is traced, resulting from a PTRACE_FORK inherited setting,
stop it right now.
XXX[1]: Cannot make this MI, because I cannot wrap child_return because there
is MD code that checks fun == child_return. I think it is better to have an
mi child_return() and add a cpu_child_return()?
XXX[2]: Why do we need to stop so early? Perhaps stopping just after exec
is better?
https://github.com/jsonn/src/commit/ccb6242edfcac843fd3d6bdf5b514ac79bad889d
Answering why do we need to stop early, in debuggers we want to stop as early as
possible, before constructors will fire and with ability to set there a trap.
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
Date: Tue, 28 Mar 2017 16:47:55 +0200
On Tue, Mar 28, 2017 at 02:45:02PM +0000, Kamil Rytarowski wrote:
> Answering why do we need to stop early, in debuggers we want to stop as early as
> possible, before constructors will fire and with ability to set there a trap.
My suggested patch does not make the signal later.
We would have to clone it for emulations that do not go via fork1() - if
there are any, but it still is better than having to clone it for all
architectures.
Martin
From: "Kamil Rytarowski" <n54@gmx.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/52117 CVS commit: src/tests/kernel
Date: Tue, 28 Mar 2017 16:47:27 +0200
> Sent: Tuesday, March 28, 2017 at 4:40 PM
> From: "Martin Husemann" <martin@duskware.de>
> To: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, n54@gmx.com
> Subject: Re: PR/52117 CVS commit: src/tests/kernel
>
> The following reply was made to PR kern/52117; it has been noted by GNATS.
>
> From: Martin Husemann <martin@duskware.de>
> To: gnats-bugs@NetBSD.org
> Cc:
> Subject: Re: PR/52117 CVS commit: src/tests/kernel
> Date: Tue, 28 Mar 2017 16:35:15 +0200
>
> Not sure I understand all contstraints, and this is completely untested,
> but maybe as a base for discussions/testing: could you try this patch
> and see if it still works on x86 ?
>
> Martin
>
> Index: kern/kern_fork.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/kern_fork.c,v
> retrieving revision 1.199
> diff -u -p -r1.199 kern_fork.c
> --- kern/kern_fork.c 13 Jan 2017 23:00:35 -0000 1.199
> +++ kern/kern_fork.c 28 Mar 2017 14:33:30 -0000
> @@ -464,6 +464,17 @@ fork1(struct lwp *l1, int flags, int exi
> */
> mutex_enter(proc_lock);
>
> +
> + /* If it is traced, signal the tracer */
> + if (p2->p_slflag & PSL_TRACED) {
> + ksiginfo_t ksi;
> +
> + KSI_INIT_EMPTY(&ksi);
> + ksi.ksi_signo = SIGTRAP;
> + ksi.ksi_lid = l2->l_lid;
> + kpsignal(p2, &ksi, NULL);
> + }
> +
> if (p1->p_session->s_ttyvp != NULL && p1->p_lflag & PL_CONTROLT)
> p2->p_lflag |= PL_CONTROLT;
>
> Index: arch/x86/x86/syscall.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/x86/x86/syscall.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 syscall.c
> --- arch/x86/x86/syscall.c 7 Jul 2016 06:55:40 -0000 1.14
> +++ arch/x86/x86/syscall.c 28 Mar 2017 14:33:30 -0000
> @@ -65,18 +65,6 @@ child_return(void *arg)
> {
> struct lwp *l = arg;
> struct trapframe *tf = l->l_md.md_regs;
> - struct proc *p = l->l_proc;
> -
> - if (p->p_slflag & PSL_TRACED) {
> - ksiginfo_t ksi;
> -
> - mutex_enter(proc_lock);
> - KSI_INIT_EMPTY(&ksi);
> - ksi.ksi_signo = SIGTRAP;
> - ksi.ksi_lid = l->l_lid;
> - kpsignal(p, &ksi, NULL);
> - mutex_exit(proc_lock);
> - }
>
> X86_TF_RAX(tf) = 0;
> X86_TF_RFLAGS(tf) &= ~PSL_C;
>
>
I will give it a try and try to research a proper solution.
From: "Kamil Rytarowski" <n54@gmx.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/52117 CVS commit: src/tests/kernel
Date: Tue, 28 Mar 2017 18:07:27 +0200
I've prepared a patch - it works on amd64.
http://www.netbsd.org/~kamil/patch-00029-mi_child_return.txt
Please check it on possible ports - by adding appropriate code inspired by x86
Basically we need to call:
mi_child_return(l);
before a call of:
userret(l).
This approach has been inspired by mi_userret() and <sys/userret.h>.
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/52117: ptrace(2) PTRACE_FORK fails on some platforms
Date: Tue, 28 Mar 2017 18:10:16 +0200
Unfortunately it does not work...
kpsignal2() ignores attempts to send signals before we are actually
running in the new context:
/*
* If the process is being created by fork, is a zombie or is
* exiting, then just drop the signal here and bail out.
*/
if (p->p_stat != SACTIVE && p->p_stat != SSTOP)
return 0;
so we can not do the handling earlier, w/o duplicating some of the signal
handling stuff, which we likely do not want to do.
Anyone got better ideas than copying the x86 code over to all child_return
implementations?
Martin
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: n54@gmx.com
Subject: Re: PR/52117 CVS commit: src/tests/kernel
Date: Tue, 28 Mar 2017 21:55:03 +0200
Ok, here is an alternative patch which seems to work for me:
[..]
Before calling wait4() for the child 38
Reported PTRACE_FORK event with forkee 332
Before calling wait4() for the forkee 332 of the child 38
Before resuming the forkee process where it left off and without signal to be sent
Before resuming the child process where it left off and without signal to be sent
Before calling wait4() for the forkee - expected exited
Before calling wait4() for the forkee - expected no process
Before calling wait4() for the child - expected stopped SIGCHLD
Before resuming the child process where it left off and without signal to be sent
Before exiting of the child process
Before calling wait4() for the child - expected exited
Before calling wait4() for the child - expected no process
failed: Test case was expecting a failure but none were raised
It has the advantage of not touching all existing child_return, but doing the
work earlier. It is slightly hackish, but kpsignal2 already has special case
code for early signals, so I'd consider it OK.
What do others think?
Patch below, obvious removal from x86/x86/syscall.c ommited.
Martin
Index: kern_fork.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_fork.c,v
retrieving revision 1.199
diff -u -p -r1.199 kern_fork.c
--- kern_fork.c 13 Jan 2017 23:00:35 -0000 1.199
+++ kern_fork.c 28 Mar 2017 19:53:34 -0000
@@ -500,6 +500,16 @@ fork1(struct lwp *l1, int flags, int exi
(*p2->p_emul->e_syscall_intern)(p2);
#endif
+ /* if we are being traced, give the owner a chance to interfere */
+ if (p2->p_slflag & PSL_TRACED) {
+ ksiginfo_t ksi;
+
+ KSI_INIT_EMPTY(&ksi);
+ ksi.ksi_signo = SIGTRAP;
+ ksi.ksi_lid = l2->l_lid;
+ kpsignal(p2, &ksi, NULL);
+ }
+
/*
* Update stats now that we know the fork was successful.
*/
Index: kern_sig.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sig.c,v
retrieving revision 1.334
diff -u -p -r1.334 kern_sig.c
--- kern_sig.c 24 Mar 2017 17:40:44 -0000 1.334
+++ kern_sig.c 28 Mar 2017 19:53:34 -0000
@@ -1224,7 +1224,7 @@ kpsignal2(struct proc *p, ksiginfo_t *ks
ksiginfo_t *kp;
lwpid_t lid;
sig_t action;
- bool toall;
+ bool toall, debtrap = false;
int error = 0;
KASSERT(!cpu_intr_p());
@@ -1237,8 +1237,13 @@ kpsignal2(struct proc *p, ksiginfo_t *ks
* If the process is being created by fork, is a zombie or is
* exiting, then just drop the signal here and bail out.
*/
- if (p->p_stat != SACTIVE && p->p_stat != SSTOP)
+ if (p->p_stat == SIDL && signo == SIGTRAP
+ && (p->p_slflag & PSL_TRACED)) {
+ /* allow an initial SIGTRAP for traced processes */
+ debtrap = true;
+ } else if (p->p_stat != SACTIVE && p->p_stat != SSTOP) {
return 0;
+ }
/* XXX for core dump/debugger */
p->p_sigctx.ps_lwp = ksi->ksi_lid;
@@ -1353,7 +1358,13 @@ kpsignal2(struct proc *p, ksiginfo_t *ks
* the signal to it.
*/
if (lid != 0) {
- l = lwp_find(p, lid);
+ if (__predict_false(debtrap)) {
+ l = LIST_FIRST(&p->p_lwps);
+ if (l->l_lid != lid)
+ l = NULL;
+ } else {
+ l = lwp_find(p, lid);
+ }
if (l != NULL) {
if ((error = sigput(&l->l_sigpend, p, kp)) != 0)
goto out;
From: Kamil Rytarowski <krytarowski@gmail.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/52117 CVS commit: src/tests/kernel
Date: Tue, 28 Mar 2017 23:40:41 +0200
On 28.03.2017 22:00, Martin Husemann wrote:
> The following reply was made to PR kern/52117; it has been noted by GNATS.
>
> From: Martin Husemann <martin@duskware.de>
> To: gnats-bugs@NetBSD.org
> Cc: n54@gmx.com
> Subject: Re: PR/52117 CVS commit: src/tests/kernel
> Date: Tue, 28 Mar 2017 21:55:03 +0200
>
> Ok, here is an alternative patch which seems to work for me:
>
> [..]
> Before calling wait4() for the child 38
> Reported PTRACE_FORK event with forkee 332
> Before calling wait4() for the forkee 332 of the child 38
> Before resuming the forkee process where it left off and without signal to be sent
> Before resuming the child process where it left off and without signal to be sent
> Before calling wait4() for the forkee - expected exited
> Before calling wait4() for the forkee - expected no process
> Before calling wait4() for the child - expected stopped SIGCHLD
> Before resuming the child process where it left off and without signal to be sent
> Before exiting of the child process
> Before calling wait4() for the child - expected exited
> Before calling wait4() for the child - expected no process
> failed: Test case was expecting a failure but none were raised
>
>
> It has the advantage of not touching all existing child_return, but doing the
> work earlier. It is slightly hackish, but kpsignal2 already has special case
> code for early signals, so I'd consider it OK.
>
> What do others think?
>
> Patch below, obvious removal from x86/x86/syscall.c ommited.
>
> Martin
>
> Index: kern_fork.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/kern_fork.c,v
> retrieving revision 1.199
> diff -u -p -r1.199 kern_fork.c
> --- kern_fork.c 13 Jan 2017 23:00:35 -0000 1.199
> +++ kern_fork.c 28 Mar 2017 19:53:34 -0000
> @@ -500,6 +500,16 @@ fork1(struct lwp *l1, int flags, int exi
> (*p2->p_emul->e_syscall_intern)(p2);
> #endif
>
> + /* if we are being traced, give the owner a chance to interfere */
> + if (p2->p_slflag & PSL_TRACED) {
> + ksiginfo_t ksi;
> +
> + KSI_INIT_EMPTY(&ksi);
> + ksi.ksi_signo = SIGTRAP;
> + ksi.ksi_lid = l2->l_lid;
> + kpsignal(p2, &ksi, NULL);
> + }
> +
> /*
> * Update stats now that we know the fork was successful.
> */
> Index: kern_sig.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/kern_sig.c,v
> retrieving revision 1.334
> diff -u -p -r1.334 kern_sig.c
> --- kern_sig.c 24 Mar 2017 17:40:44 -0000 1.334
> +++ kern_sig.c 28 Mar 2017 19:53:34 -0000
> @@ -1224,7 +1224,7 @@ kpsignal2(struct proc *p, ksiginfo_t *ks
> ksiginfo_t *kp;
> lwpid_t lid;
> sig_t action;
> - bool toall;
> + bool toall, debtrap = false;
> int error = 0;
>
> KASSERT(!cpu_intr_p());
> @@ -1237,8 +1237,13 @@ kpsignal2(struct proc *p, ksiginfo_t *ks
> * If the process is being created by fork, is a zombie or is
> * exiting, then just drop the signal here and bail out.
> */
> - if (p->p_stat != SACTIVE && p->p_stat != SSTOP)
> + if (p->p_stat == SIDL && signo == SIGTRAP
> + && (p->p_slflag & PSL_TRACED)) {
> + /* allow an initial SIGTRAP for traced processes */
> + debtrap = true;
> + } else if (p->p_stat != SACTIVE && p->p_stat != SSTOP) {
> return 0;
> + }
>
> /* XXX for core dump/debugger */
> p->p_sigctx.ps_lwp = ksi->ksi_lid;
> @@ -1353,7 +1358,13 @@ kpsignal2(struct proc *p, ksiginfo_t *ks
> * the signal to it.
> */
> if (lid != 0) {
> - l = lwp_find(p, lid);
> + if (__predict_false(debtrap)) {
> + l = LIST_FIRST(&p->p_lwps);
> + if (l->l_lid != lid)
> + l = NULL;
> + } else {
> + l = lwp_find(p, lid);
> + }
> if (l != NULL) {
> if ((error = sigput(&l->l_sigpend, p, kp)) != 0)
> goto out;
>
>
I'm fine with both.
For consistency I would add "ksi.ksi_signo = TRAP_CHLD;". My motivation
is to attribute every SIGTRAP in the kernel with appropriate si_code,
even if it works for some reasons in some scenarios without them.
From user-space point of view we want a deterministic order of receiving
signals by a debugger. Signal for "child" and next "child2".
I prefer the <sys/childreturn.h> version as it has bit less overhead...
but it might be just a microoptimization not worth modifying the
child_return code in all ports.
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/52117 CVS commit: src/tests/kernel
Date: Wed, 29 Mar 2017 06:52:21 +0200
On Tue, Mar 28, 2017 at 09:40:00PM +0000, Kamil Rytarowski wrote:
> For consistency I would add "ksi.ksi_signo = TRAP_CHLD;". My motivation
> is to attribute every SIGTRAP in the kernel with appropriate si_code,
> even if it works for some reasons in some scenarios without them.
Yes, that certainly should be done.
> From user-space point of view we want a deterministic order of receiving
> signals by a debugger. Signal for "child" and next "child2".
We can not guarantee this. The signals happen as soon as the process
gets scheduled, it would be pretty hard to guarantee the forker running
first (at least I wouldn't know how to do that).
Is this realy a requirement from a debuggers perspective?
> I prefer the <sys/childreturn.h> version as it has bit less overhead...
> but it might be just a microoptimization not worth modifying the
> child_return code in all ports.
Yes, the code is a bit clunky, but runtime performance and locking is actually
better (I think).
Martin
From: Kamil Rytarowski <n54@gmx.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/52117 CVS commit: src/tests/kernel
Date: Wed, 29 Mar 2017 07:03:41 +0200
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--IPT5JTnnqUV0KqAkBTjQ4JPggK9tCQiR6
Content-Type: multipart/mixed; boundary="1NKi3KoowX2SBpG1jSLB5xbKPqOvhxb70";
protected-headers="v1"
From: Kamil Rytarowski <n54@gmx.com>
To: gnats-bugs@NetBSD.org
Message-ID: <422e9ad7-31b2-d556-f3db-b38b554a1c52@gmx.com>
Subject: Re: PR/52117 CVS commit: src/tests/kernel
References: <pr-kern-52117@gnats.netbsd.org>
<20170328020808.795E47A26D@mollari.NetBSD.org>
<20170329045501.9AF2A7A279@mollari.NetBSD.org>
In-Reply-To: <20170329045501.9AF2A7A279@mollari.NetBSD.org>
--1NKi3KoowX2SBpG1jSLB5xbKPqOvhxb70
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable
On 29.03.2017 06:55, Martin Husemann wrote:
> The following reply was made to PR kern/52117; it has been noted by GNA=
TS.
>=20
> From: Martin Husemann <martin@duskware.de>
> To: gnats-bugs@NetBSD.org
> Cc:=20
> Subject: Re: PR/52117 CVS commit: src/tests/kernel
> Date: Wed, 29 Mar 2017 06:52:21 +0200
>=20
> On Tue, Mar 28, 2017 at 09:40:00PM +0000, Kamil Rytarowski wrote:
> > For consistency I would add "ksi.ksi_signo =3D TRAP_CHLD;". My moti=
vation
> > is to attribute every SIGTRAP in the kernel with appropriate si_cod=
e,
> > even if it works for some reasons in some scenarios without them.
> =20
> Yes, that certainly should be done.
> =20
> > From user-space point of view we want a deterministic order of rece=
iving
> > signals by a debugger. Signal for "child" and next "child2".
> =20
> We can not guarantee this. The signals happen as soon as the process
> gets scheduled, it would be pretty hard to guarantee the forker runnin=
g
> first (at least I wouldn't know how to do that).
> =20
> Is this realy a requirement from a debuggers perspective?
> =20
No, it's not a hard requirement. It's in the class of nice-to-have to
make the code inside a debugger simpler. Random order in Linux is making
trouble for the designers of debuggers, but since we generate TRAP_CHLD
this is already mitigated.
We need to alter documentation and ATF tests for this.
> > I prefer the <sys/childreturn.h> version as it has bit less overhea=
d...
> > but it might be just a microoptimization not worth modifying the
> > child_return code in all ports.
> =20
> Yes, the code is a bit clunky, but runtime performance and locking is =
actually
> better (I think).
> =20
I think we can go this route. Thank you for your patch.
> Martin
> =20
>=20
--1NKi3KoowX2SBpG1jSLB5xbKPqOvhxb70--
--IPT5JTnnqUV0KqAkBTjQ4JPggK9tCQiR6
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJY20A9AAoJEEuzCOmwLnZsbt0QAKE9HagDmUqqViEICCvHfpZ8
nEug54yTgKyXzcc9/qXUxdQx11a+e5HpCDqz5HuC1i6xOflQb1tYXbMoRSpB0PG+
LauQ9txAAK6mhwDrOc3uGx2MHzKYAOTvK5PontaQ6h34GSe7ugfD8q/y2O/CApjH
M9aK72ZI40FmyWyy0sn3bCLctgx+H10uQifgFV6DpHzCkwnLoxFibdSVJWeW2ChF
DU0e601FBLb3RrUaNTCo+p7A4bCaQ+ZTqcqURMw0SKDa5I5+i3/dn+jfbEoEkdpC
ClXD6tJgdlPEAH/+Nu8a05+W6tbR2RZXXqupDcGsAZd5vuZ/QkcczfuP9d0I4SQH
ax1R9ROmoaBvVVf9t/TCnc73mdh4TmOqifhmBaxbYA64ZuEwpPawEpCdI/r8Uq3Y
8ckjtp/HL+VO6tC5jSPCqAf3dSjIdPT29JQryybcX/FzqpANeOHe5g1hulymk/9W
Kfp5aoVQRpgmjb1ASvQVHrEkXueGYxOc/rEsDrC5o57tYRT0A4D1lNjOg/WQDqN0
5rby+jORomQuwUxfv9ji1fEY9P3+Gj8z4kQRjmZK2zVaI5UAgM/n2SnhMYf6Znxd
WLpqnYSRfQkxXlCwR+/FmfM/woLagu8jEtpwDO1UmJ5SLt6k7QcbHSvolwVPL8gj
H3m+iJvUy8Bkf57VW9YC
=r1YV
-----END PGP SIGNATURE-----
--IPT5JTnnqUV0KqAkBTjQ4JPggK9tCQiR6--
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/52117 CVS commit: src/sys
Date: Fri, 31 Mar 2017 08:47:04 +0000
Module Name: src
Committed By: martin
Date: Fri Mar 31 08:47:04 UTC 2017
Modified Files:
src/sys/arch/x86/x86: syscall.c
src/sys/kern: kern_fork.c kern_sig.c
Log Message:
PR kern/52117: move stop code for debuged children after fork into MI code.
XXX we might want to revisit this when handling the same event for vfork
better.
To generate a diff of this commit:
cvs rdiff -u -r1.14 -r1.15 src/sys/arch/x86/x86/syscall.c
cvs rdiff -u -r1.199 -r1.200 src/sys/kern/kern_fork.c
cvs rdiff -u -r1.334 -r1.335 src/sys/kern/kern_sig.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->closed
State-Changed-By: kamil@NetBSD.org
State-Changed-When: Mon, 10 Apr 2017 18:41:16 +0200
State-Changed-Why:
Fix committed by <martin>
cvs rdiff -u -r1.14 -r1.15 src/sys/arch/x86/x86/syscall.c
cvs rdiff -u -r1.199 -r1.200 src/sys/kern/kern_fork.c
cvs rdiff -u -r1.334 -r1.335 src/sys/kern/kern_sig.c
From: "Kamil Rytarowski" <kamil@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/52117 CVS commit: src/tests/lib/libc/sys
Date: Mon, 10 Apr 2017 16:45:57 +0000
Module Name: src
Committed By: kamil
Date: Mon Apr 10 16:45:57 UTC 2017
Modified Files:
src/tests/lib/libc/sys: t_ptrace_wait.c
Log Message:
fork1 and siginfo5 no longer fails on !x86 platforms in t_ptrace_wait*
The PTRACE_FORK operation has been fixed. Patch committed by <martin>
PR kern/52117
To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.3 src/tests/lib/libc/sys/t_ptrace_wait.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home