On 9/5/20, Philip Guenther <guent...@gmail.com> wrote: > On Fri, Sep 4, 2020 at 2:59 PM Mateusz Guzik <mjgu...@gmail.com> wrote: > >> On 9/5/20, Philip Guenther <guent...@gmail.com> wrote: >> > On Fri, Sep 4, 2020 at 1:06 PM Mateusz Guzik <mjgu...@gmail.com> wrote: >> > >> >> On 9/4/20, Vitaliy Makkoveev <m...@openbsd.org> wrote: >> >> > On Fri, Sep 04, 2020 at 05:24:42PM +0200, Mateusz Guzik wrote: >> >> >> getppid blindly follows the parent pointer and reads the pid. >> >> >> >> >> >> The problem is that ptrace reparents the traced process, so in >> >> >> particular if you gdb -p $something, the target proc will start >> seeing >> >> >> gdb instead of its actual parent. >> >> >> >> >> >> There is a lot to say about the entire reparenting business or >> storing >> >> >> the original pid in ps_oppid (instead of some form of a reference >> >> >> to >> >> >> the process). >> >> >> >> >> >> However, I think the most feasible fix for now is the same thing >> >> >> FreeBSD did: *always* store the actual parent pid in ps_oppid. This >> >> >> means all repareting will keep updating it (most notably when >> >> >> abandoning children on exit), while ptrace will skip that part. >> >> >> >> >> >> Side effect of such a change be that getppid will stop requiring >> >> >> the >> >> >> kernel lock. >> >> >> >> >> > >> >> > Thanks for report. But we are in beta stage now so such modification >> is >> >> > impossible until next iteration. >> >> > >> >> > Since original parent identifier is stored as `ps_oppid' while >> >> > process >> >> > is traced we just return it to userland for this case. This is the >> >> > way >> >> > I >> >> > propose to fix this bug for now. >> >> > >> >> > Comments? OKs? >> >> > >> >> > Index: sys/kern/kern_prot.c >> >> > =================================================================== >> >> > RCS file: /cvs/src/sys/kern/kern_prot.c,v >> >> > retrieving revision 1.76 >> >> > diff -u -p -r1.76 kern_prot.c >> >> > --- sys/kern/kern_prot.c 9 Jul 2019 12:23:25 -0000 1.76 >> >> > +++ sys/kern/kern_prot.c 4 Sep 2020 21:12:15 -0000 >> >> > @@ -84,7 +84,11 @@ int >> >> > sys_getppid(struct proc *p, void *v, register_t *retval) >> >> > { >> >> > >> >> > - *retval = p->p_p->ps_pptr->ps_pid; >> >> > + if (p->p_p->ps_flags & PS_TRACED) >> >> > + *retval = p->p_p->ps_oppid; >> >> > + else >> >> > + *retval = p->p_p->ps_pptr->ps_pid; >> >> > + >> >> > return (0); >> >> > } >> >> >> >> This is definitely a bare minimum fix, but it does the job. >> >> >> > >> > ptrace() has behaved like this for the life of OpenBSD and an >> > indefinite >> > number of years previous in the BSD releases. What has happened that a >> > definitely incomplete fix is needed Right Now? >> >> I don't see how this reads as a demand this is fixed Right Now. >> > > I didn't call it a demand, but the point stands: what has changed? >
Nothing has changed. I don't use the system apart from sometimes testing stuff in a VM. There is 0 pressure on my end to fix this. > > I don't see how the fix is incomplete either. It can be done better >> with more effort, but AFAICS the above results in correct behavior. >> > > There are at least 2 other uses of ps_pptr->ps_pid that should also change, > unless you like coredumps and ps disagreeing with getppid(), and someone > needs to think how it affects doas. > I see, I did not grep for these. -- Mateusz Guzik <mjguzik gmail.com>