On 21/03/21(Sun) 13:42, Mark Kettenis wrote:
> > Date: Sat, 20 Mar 2021 13:10:17 +0100
> > From: Martin Pieuchot <m...@openbsd.org>
> >
> > 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.c 15 Feb 2021 09:35:59 -0000 1.234
+++ kern/kern_fork.c 21 Mar 2021 15:55:26 -0000
@@ -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
