Re: fork(2), PT_ATTACH & SIGTRAP
> Date: Sun, 21 Mar 2021 16:56:47 +0100
> From: Martin Pieuchot
>
> On 21/03/21(Sun) 13:42, Mark Kettenis wrote:
> > > Date: Sat, 20 Mar 2021 13:10:17 +0100
> > > From: Martin Pieuchot
> > >
> > > On SP systems, like bluhm@'s armv7 regression machine, the kern/ptrace2
> > > test is failing due to a subtle behavior. Diff below makes it pass.
> > >
> > > http://bluhm.genua.de/regress/results/2021-03-19T15%3A17%3A02Z/logs/sys/kern/ptrace2/make.log
> > >
> > > The failing test does a fork(2) and the parent issues a PT_ATTACH on the
> > > child before it has been scheduled for the first time. Then the parent
> > > goes to sleep in waitpid() and when the child starts executing the check
> > > below overwrites the ptrace(2)-received SIGSTOP by a SIGTRAP.
> > >
> > > This scenario doesn't seem to happen on MP machine because the child
> > > starts to execute itself on a different core right after sys_fork() is
> > > finished.
> > >
> > > What is the purpose of this check? Should it be relaxed or removed?
> >
> > This is part of PT_SET_EVENT_MASK/PTRACE_FORK support:
> >
> > https://github.com/openbsd/src/commit/f38bed7f869bd3503530c554b4860228ea4e8641
> >
> > When reporting of the PTRACE_FORK event is requested, the debugger
> > expects to see a SIGTRAP in both the parent and the child. The code
> > expects that the only way to have PS_TRACED set in the child from the
> > start is when PTRACE_FORK is requested. But the failing test shows
> > there is a race with PT_ATTACH.
>
> Thanks for the explanation.
>
> > I think the solution is to have fork1() only run fork_return() if the
> > FORK_PTRACE flag is set, and use run child_return() otherwise.
>
> Diff below does that and prevent the race, ok?
ok kettenis@
> Index: kern/kern_fork.c
> ===
> RCS file: /cvs/src/sys/kern/kern_fork.c,v
> retrieving revision 1.234
> diff -u -p -r1.234 kern_fork.c
> --- kern/kern_fork.c 15 Feb 2021 09:35:59 - 1.234
> +++ kern/kern_fork.c 21 Mar 2021 15:55:26 -
> @@ -95,12 +95,15 @@ fork_return(void *arg)
> int
> sys_fork(struct proc *p, void *v, register_t *retval)
> {
> + void (*func)(void *) = child_return;
> int flags;
>
> flags = FORK_FORK;
> - if (p->p_p->ps_ptmask & PTRACE_FORK)
> + if (p->p_p->ps_ptmask & PTRACE_FORK) {
> flags |= FORK_PTRACE;
> - return fork1(p, flags, fork_return, NULL, retval, NULL);
> + func = fork_return;
> + }
> + return fork1(p, flags, func, NULL, retval, NULL);
> }
>
> int
>
Re: fork(2), PT_ATTACH & SIGTRAP
On 21/03/21(Sun) 13:42, Mark Kettenis wrote:
> > Date: Sat, 20 Mar 2021 13:10:17 +0100
> > From: Martin Pieuchot
> >
> > On SP systems, like bluhm@'s armv7 regression machine, the kern/ptrace2
> > test is failing due to a subtle behavior. Diff below makes it pass.
> >
> > http://bluhm.genua.de/regress/results/2021-03-19T15%3A17%3A02Z/logs/sys/kern/ptrace2/make.log
> >
> > The failing test does a fork(2) and the parent issues a PT_ATTACH on the
> > child before it has been scheduled for the first time. Then the parent
> > goes to sleep in waitpid() and when the child starts executing the check
> > below overwrites the ptrace(2)-received SIGSTOP by a SIGTRAP.
> >
> > This scenario doesn't seem to happen on MP machine because the child
> > starts to execute itself on a different core right after sys_fork() is
> > finished.
> >
> > What is the purpose of this check? Should it be relaxed or removed?
>
> This is part of PT_SET_EVENT_MASK/PTRACE_FORK support:
>
> https://github.com/openbsd/src/commit/f38bed7f869bd3503530c554b4860228ea4e8641
>
> When reporting of the PTRACE_FORK event is requested, the debugger
> expects to see a SIGTRAP in both the parent and the child. The code
> expects that the only way to have PS_TRACED set in the child from the
> start is when PTRACE_FORK is requested. But the failing test shows
> there is a race with PT_ATTACH.
Thanks for the explanation.
> I think the solution is to have fork1() only run fork_return() if the
> FORK_PTRACE flag is set, and use run child_return() otherwise.
Diff below does that and prevent the race, ok?
Index: kern/kern_fork.c
===
RCS file: /cvs/src/sys/kern/kern_fork.c,v
retrieving revision 1.234
diff -u -p -r1.234 kern_fork.c
--- kern/kern_fork.c15 Feb 2021 09:35:59 - 1.234
+++ kern/kern_fork.c21 Mar 2021 15:55:26 -
@@ -95,12 +95,15 @@ fork_return(void *arg)
int
sys_fork(struct proc *p, void *v, register_t *retval)
{
+ void (*func)(void *) = child_return;
int flags;
flags = FORK_FORK;
- if (p->p_p->ps_ptmask & PTRACE_FORK)
+ if (p->p_p->ps_ptmask & PTRACE_FORK) {
flags |= FORK_PTRACE;
- return fork1(p, flags, fork_return, NULL, retval, NULL);
+ func = fork_return;
+ }
+ return fork1(p, flags, func, NULL, retval, NULL);
}
int
Re: fork(2), PT_ATTACH & SIGTRAP
> Date: Sat, 20 Mar 2021 13:10:17 +0100
> From: Martin Pieuchot
>
> On SP systems, like bluhm@'s armv7 regression machine, the kern/ptrace2
> test is failing due to a subtle behavior. Diff below makes it pass.
>
> http://bluhm.genua.de/regress/results/2021-03-19T15%3A17%3A02Z/logs/sys/kern/ptrace2/make.log
>
> The failing test does a fork(2) and the parent issues a PT_ATTACH on the
> child before it has been scheduled for the first time. Then the parent
> goes to sleep in waitpid() and when the child starts executing the check
> below overwrites the ptrace(2)-received SIGSTOP by a SIGTRAP.
>
> This scenario doesn't seem to happen on MP machine because the child
> starts to execute itself on a different core right after sys_fork() is
> finished.
>
> What is the purpose of this check? Should it be relaxed or removed?
This is part of PT_SET_EVENT_MASK/PTRACE_FORK support:
https://github.com/openbsd/src/commit/f38bed7f869bd3503530c554b4860228ea4e8641
When reporting of the PTRACE_FORK event is requested, the debugger
expects to see a SIGTRAP in both the parent and the child. The code
expects that the only way to have PS_TRACED set in the child from the
start is when PTRACE_FORK is requested. But the failing test shows
there is a race with PT_ATTACH.
I think the solution is to have fork1() only run fork_return() if the
FORK_PTRACE flag is set, and use run child_return() otherwise.
> Index: kern/kern_fork.c
> ===
> RCS file: /cvs/src/sys/kern/kern_fork.c,v
> retrieving revision 1.234
> diff -u -p -r1.234 kern_fork.c
> --- kern/kern_fork.c 15 Feb 2021 09:35:59 - 1.234
> +++ kern/kern_fork.c 20 Mar 2021 11:59:18 -
> @@ -86,9 +86,6 @@ fork_return(void *arg)
> {
> struct proc *p = (struct proc *)arg;
>
> - if (p->p_p->ps_flags & PS_TRACED)
> - psignal(p, SIGTRAP);
> -
> child_return(p);
> }
>
>
>
fork(2), PT_ATTACH & SIGTRAP
On SP systems, like bluhm@'s armv7 regression machine, the kern/ptrace2
test is failing due to a subtle behavior. Diff below makes it pass.
http://bluhm.genua.de/regress/results/2021-03-19T15%3A17%3A02Z/logs/sys/kern/ptrace2/make.log
The failing test does a fork(2) and the parent issues a PT_ATTACH on the
child before it has been scheduled for the first time. Then the parent
goes to sleep in waitpid() and when the child starts executing the check
below overwrites the ptrace(2)-received SIGSTOP by a SIGTRAP.
This scenario doesn't seem to happen on MP machine because the child
starts to execute itself on a different core right after sys_fork() is
finished.
What is the purpose of this check? Should it be relaxed or removed?
Index: kern/kern_fork.c
===
RCS file: /cvs/src/sys/kern/kern_fork.c,v
retrieving revision 1.234
diff -u -p -r1.234 kern_fork.c
--- kern/kern_fork.c15 Feb 2021 09:35:59 - 1.234
+++ kern/kern_fork.c20 Mar 2021 11:59:18 -
@@ -86,9 +86,6 @@ fork_return(void *arg)
{
struct proc *p = (struct proc *)arg;
- if (p->p_p->ps_flags & PS_TRACED)
- psignal(p, SIGTRAP);
-
child_return(p);
}
