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.

-- 
Mateusz Guzik <mjguzik gmail.com>

Reply via email to