Re: CVS commit: src/sys/kern

[christos%zoulas.com@localhost (Christos Zoulas)]

On Nov 13,  8:39am, kre%munnari.OZ.AU@localhost (Robert Elz) wrote:
-- Subject: Re: CVS commit: src/sys/kern

|     Date:        Sat, 12 Nov 2016 14:42:47 -0500
|     From:        "Christos Zoulas" <christos%netbsd.org@localhost>
|     Message-ID:  <20161112194247.37910FBA6%cvs.NetBSD.org@localhost>
| 
|   | PR/51624: Return the original parent for a traced process.
| 
| Maybe the real bug here was that proc_reparent() is changing the
| child's p_ppid ?
| 
| I can see no reason for that, and if it wasn't done, then p_ppid would
| be what is wanted by getppid() without needing kern_getppid() to
| do all that unwind logic (and assiated locting and unlocking to make
| it safe.)
| 
| Aside from proc_reparent() the only weirdness I can see with p_ppid are
| in kern_proc.c in fill_eproc() and fill_kproc2().   They both use (and
| continue to use, so the results will be different for a process being
| traced, and the same process when not traced) p_pptr->p_pid rather than
| the simpler p_ppid but I am not sure why (nor what the clients of those
| functions are or what the info is used for, so I am not sure what is correct.)

p_ppid is supposed to be the cached value of p->p_pptr->p_pid (or if you
want to make the situation more complex) or p->p_opptr->p_pid. We can undo
the complex logic if we make sure that p->p_ppid is always updated. Perhaps
we can put the refresh bits in proc_reparent()...

As far as leaving p->p_pptr alone, I am not sure. We'd have to check all
the places it's beeing used.

christos