Re: Cache parent's pid as `ps_ppid' and use it instead of `ps_pptr->ps_pid'.
On Wed, Jan 13, 2021 at 02:10:25PM -0300, Martin Pieuchot wrote:
> On 02/01/21(Sat) 21:54, Vitaliy Makkoveev wrote:
> > This allows us to unlock getppid(2). Also NetBSD, DragonflyBSD and OSX
> > do the same.
>
> Seems the way to go, two comments below.
>
> > @@ -694,6 +694,7 @@ process_reparent(struct process *child,
> > }
> >
> > child->ps_pptr = parent;
> > + child->ps_ppid = parent->ps_pid;
>
> Should the parent pid be changed when a process is re-parented when
> being traced? It seems that both Free and Net only change it when a
> process is re-arented to process 1 (init).
>
> > + pid_t ps_ppid;/* Cached parent pid */
> > pid_t ps_oppid; /* Save parent pid during ptrace. */
>
> Can't we re-use `ps_oppid' and always cache it? It seems that FreeBSD
> is doing something like that. Could this field be documented to make it
> clear why geppid(2) can be unlocked?
>
This changes getppid(2), ps(1) and coredump behavior. Currently we
return debuger's pid as parent while process is being traced. FreeBSD
returns original parent's pid. Philip Guenther pointed, that this
behavior was for the life of OpenBSD and it's not obvious that why it
should be changed. Also at least DragonflyBSD and OSX do the same.
I marked new field `ps_ppid' as atomic. `ps_oppid' is also atomic and
I'll mark it with separate diff.
Index: sys/kern/exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.156
diff -u -p -r1.156 exec_elf.c
--- sys/kern/exec_elf.c 7 Dec 2020 16:55:28 - 1.156
+++ sys/kern/exec_elf.c 13 Jan 2021 19:01:08 -
@@ -1257,7 +1257,7 @@ coredump_notes_elf(struct proc *p, void
cpi.cpi_sigcatch = pr->ps_sigacts->ps_sigcatch;
cpi.cpi_pid = pr->ps_pid;
- cpi.cpi_ppid = pr->ps_pptr->ps_pid;
+ cpi.cpi_ppid = pr->ps_ppid;
cpi.cpi_pgrp = pr->ps_pgid;
if (pr->ps_session->s_leader)
cpi.cpi_sid = pr->ps_session->s_leader->ps_pid;
Index: sys/kern/kern_exit.c
===
RCS file: /cvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.193
diff -u -p -r1.193 kern_exit.c
--- sys/kern/kern_exit.c9 Dec 2020 18:58:19 - 1.193
+++ sys/kern/kern_exit.c13 Jan 2021 19:01:08 -
@@ -694,6 +694,7 @@ process_reparent(struct process *child,
}
child->ps_pptr = parent;
+ child->ps_ppid = parent->ps_pid;
}
void
Index: sys/kern/kern_fork.c
===
RCS file: /cvs/src/sys/kern/kern_fork.c,v
retrieving revision 1.230
diff -u -p -r1.230 kern_fork.c
--- sys/kern/kern_fork.c7 Dec 2020 16:55:28 - 1.230
+++ sys/kern/kern_fork.c13 Jan 2021 19:01:08 -
@@ -231,6 +231,7 @@ process_new(struct proc *p, struct proce
/* post-copy fixups */
pr->ps_pptr = parent;
+ pr->ps_ppid = parent->ps_pid;
/* bump references to the text vnode (for sysctl) */
pr->ps_textvp = parent->ps_textvp;
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.c9 Jul 2019 12:23:25 - 1.76
+++ sys/kern/kern_prot.c13 Jan 2021 19:01:08 -
@@ -84,7 +84,7 @@ int
sys_getppid(struct proc *p, void *v, register_t *retval)
{
- *retval = p->p_p->ps_pptr->ps_pid;
+ *retval = p->p_p->ps_ppid;
return (0);
}
Index: sys/kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.387
diff -u -p -r1.387 kern_sysctl.c
--- sys/kern/kern_sysctl.c 9 Jan 2021 23:33:18 - 1.387
+++ sys/kern/kern_sysctl.c 13 Jan 2021 19:01:08 -
@@ -1647,7 +1647,7 @@ fill_kproc(struct process *pr, struct ki
/* stuff that's too painful to generalize into the macros */
if (pr->ps_pptr)
- ki->p_ppid = pr->ps_pptr->ps_pid;
+ ki->p_ppid = pr->ps_ppid;
if (s->s_leader)
ki->p_sid = s->s_leader->ps_pid;
Index: sys/sys/proc.h
===
RCS file: /cvs/src/sys/sys/proc.h,v
retrieving revision 1.304
diff -u -p -r1.304 proc.h
--- sys/sys/proc.h 11 Jan 2021 13:55:53 - 1.304
+++ sys/sys/proc.h 13 Jan 2021 19:01:08 -
@@ -216,6 +216,7 @@ struct process {
u_int ps_xexit; /* Exit status for wait */
int ps_xsig;/* Stopping or killing signal */
+ pid_t ps_ppid;/* [a] Cached parent pid */
pid_t ps_oppid; /* Save parent pid during ptrace. */
int
Re: Cache parent's pid as `ps_ppid' and use it instead of `ps_pptr->ps_pid'.
On 02/01/21(Sat) 21:54, Vitaliy Makkoveev wrote:
> This allows us to unlock getppid(2). Also NetBSD, DragonflyBSD and OSX
> do the same.
Seems the way to go, two comments below.
> Index: kern/exec_elf.c
> ===
> RCS file: /cvs/src/sys/kern/exec_elf.c,v
> retrieving revision 1.156
> diff -u -p -r1.156 exec_elf.c
> --- kern/exec_elf.c 7 Dec 2020 16:55:28 - 1.156
> +++ kern/exec_elf.c 2 Jan 2021 15:47:46 -
> @@ -1257,7 +1257,7 @@ coredump_notes_elf(struct proc *p, void
> cpi.cpi_sigcatch = pr->ps_sigacts->ps_sigcatch;
>
> cpi.cpi_pid = pr->ps_pid;
> - cpi.cpi_ppid = pr->ps_pptr->ps_pid;
> + cpi.cpi_ppid = pr->ps_ppid;
> cpi.cpi_pgrp = pr->ps_pgid;
> if (pr->ps_session->s_leader)
> cpi.cpi_sid = pr->ps_session->s_leader->ps_pid;
> Index: kern/kern_exit.c
> ===
> RCS file: /cvs/src/sys/kern/kern_exit.c,v
> retrieving revision 1.193
> diff -u -p -r1.193 kern_exit.c
> --- kern/kern_exit.c 9 Dec 2020 18:58:19 - 1.193
> +++ kern/kern_exit.c 2 Jan 2021 15:47:46 -
> @@ -694,6 +694,7 @@ process_reparent(struct process *child,
> }
>
> child->ps_pptr = parent;
> + child->ps_ppid = parent->ps_pid;
Should the parent pid be changed when a process is re-parented when
being traced? It seems that both Free and Net only change it when a
process is re-arented to process 1 (init).
> Index: kern/kern_fork.c
> ===
> RCS file: /cvs/src/sys/kern/kern_fork.c,v
> retrieving revision 1.230
> diff -u -p -r1.230 kern_fork.c
> --- kern/kern_fork.c 7 Dec 2020 16:55:28 - 1.230
> +++ kern/kern_fork.c 2 Jan 2021 15:47:46 -
> @@ -231,6 +231,7 @@ process_new(struct proc *p, struct proce
>
> /* post-copy fixups */
> pr->ps_pptr = parent;
> + pr->ps_ppid = parent->ps_pid;
>
> /* bump references to the text vnode (for sysctl) */
> pr->ps_textvp = parent->ps_textvp;
> Index: 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
> --- kern/kern_prot.c 9 Jul 2019 12:23:25 - 1.76
> +++ kern/kern_prot.c 2 Jan 2021 15:47:46 -
> @@ -84,7 +84,7 @@ int
> sys_getppid(struct proc *p, void *v, register_t *retval)
> {
>
> - *retval = p->p_p->ps_pptr->ps_pid;
> + *retval = p->p_p->ps_ppid;
> return (0);
> }
>
> Index: kern/kern_sysctl.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.385
> diff -u -p -r1.385 kern_sysctl.c
> --- kern/kern_sysctl.c28 Dec 2020 18:28:11 - 1.385
> +++ kern/kern_sysctl.c2 Jan 2021 15:47:46 -
> @@ -1666,7 +1666,7 @@ fill_kproc(struct process *pr, struct ki
>
> /* stuff that's too painful to generalize into the macros */
> if (pr->ps_pptr)
> - ki->p_ppid = pr->ps_pptr->ps_pid;
> + ki->p_ppid = pr->ps_ppid;
> if (s->s_leader)
> ki->p_sid = s->s_leader->ps_pid;
>
> Index: sys/proc.h
> ===
> RCS file: /cvs/src/sys/sys/proc.h,v
> retrieving revision 1.303
> diff -u -p -r1.303 proc.h
> --- sys/proc.h9 Dec 2020 18:58:19 - 1.303
> +++ sys/proc.h2 Jan 2021 15:47:47 -
> @@ -216,6 +216,7 @@ struct process {
> u_int ps_xexit; /* Exit status for wait */
> int ps_xsig;/* Stopping or killing signal */
>
> + pid_t ps_ppid;/* Cached parent pid */
> pid_t ps_oppid; /* Save parent pid during ptrace. */
Can't we re-use `ps_oppid' and always cache it? It seems that FreeBSD
is doing something like that. Could this field be documented to make it
clear why geppid(2) can be unlocked?
> int ps_ptmask; /* Ptrace event mask */
> struct ptrace_state *ps_ptstat;/* Ptrace state */
>
Cache parent's pid as `ps_ppid' and use it instead of `ps_pptr->ps_pid'.
This allows us to unlock getppid(2). Also NetBSD, DragonflyBSD and OSX
do the same.
Index: kern/exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.156
diff -u -p -r1.156 exec_elf.c
--- kern/exec_elf.c 7 Dec 2020 16:55:28 - 1.156
+++ kern/exec_elf.c 2 Jan 2021 15:47:46 -
@@ -1257,7 +1257,7 @@ coredump_notes_elf(struct proc *p, void
cpi.cpi_sigcatch = pr->ps_sigacts->ps_sigcatch;
cpi.cpi_pid = pr->ps_pid;
- cpi.cpi_ppid = pr->ps_pptr->ps_pid;
+ cpi.cpi_ppid = pr->ps_ppid;
cpi.cpi_pgrp = pr->ps_pgid;
if (pr->ps_session->s_leader)
cpi.cpi_sid = pr->ps_session->s_leader->ps_pid;
Index: kern/kern_exit.c
===
RCS file: /cvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.193
diff -u -p -r1.193 kern_exit.c
--- kern/kern_exit.c9 Dec 2020 18:58:19 - 1.193
+++ kern/kern_exit.c2 Jan 2021 15:47:46 -
@@ -694,6 +694,7 @@ process_reparent(struct process *child,
}
child->ps_pptr = parent;
+ child->ps_ppid = parent->ps_pid;
}
void
Index: kern/kern_fork.c
===
RCS file: /cvs/src/sys/kern/kern_fork.c,v
retrieving revision 1.230
diff -u -p -r1.230 kern_fork.c
--- kern/kern_fork.c7 Dec 2020 16:55:28 - 1.230
+++ kern/kern_fork.c2 Jan 2021 15:47:46 -
@@ -231,6 +231,7 @@ process_new(struct proc *p, struct proce
/* post-copy fixups */
pr->ps_pptr = parent;
+ pr->ps_ppid = parent->ps_pid;
/* bump references to the text vnode (for sysctl) */
pr->ps_textvp = parent->ps_textvp;
Index: 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
--- kern/kern_prot.c9 Jul 2019 12:23:25 - 1.76
+++ kern/kern_prot.c2 Jan 2021 15:47:46 -
@@ -84,7 +84,7 @@ int
sys_getppid(struct proc *p, void *v, register_t *retval)
{
- *retval = p->p_p->ps_pptr->ps_pid;
+ *retval = p->p_p->ps_ppid;
return (0);
}
Index: kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.385
diff -u -p -r1.385 kern_sysctl.c
--- kern/kern_sysctl.c 28 Dec 2020 18:28:11 - 1.385
+++ kern/kern_sysctl.c 2 Jan 2021 15:47:46 -
@@ -1666,7 +1666,7 @@ fill_kproc(struct process *pr, struct ki
/* stuff that's too painful to generalize into the macros */
if (pr->ps_pptr)
- ki->p_ppid = pr->ps_pptr->ps_pid;
+ ki->p_ppid = pr->ps_ppid;
if (s->s_leader)
ki->p_sid = s->s_leader->ps_pid;
Index: sys/proc.h
===
RCS file: /cvs/src/sys/sys/proc.h,v
retrieving revision 1.303
diff -u -p -r1.303 proc.h
--- sys/proc.h 9 Dec 2020 18:58:19 - 1.303
+++ sys/proc.h 2 Jan 2021 15:47:47 -
@@ -216,6 +216,7 @@ struct process {
u_int ps_xexit; /* Exit status for wait */
int ps_xsig;/* Stopping or killing signal */
+ pid_t ps_ppid;/* Cached parent pid */
pid_t ps_oppid; /* Save parent pid during ptrace. */
int ps_ptmask; /* Ptrace event mask */
struct ptrace_state *ps_ptstat;/* Ptrace state */
