Re: [PATCH] exec: remove redundant save asides of old pid/vpid

[Nir Lichtman]

On Sat, Feb 01, 2025 at 01:40:00AM -0800, Kees Cook wrote:
> 
> 
> On February 1, 2025 12:31:27 AM PST, Nir Lichtman <nir@xxxxxxxxxxxx> wrote:
> >Problem: Old pid and vpid are redundantly saved aside before starting to
> >parse the binary, with the comment claiming that it is required since
> >load_binary changes it, though from inspection in the source,
> >load_binary does not change the pid and this wouldn't make sense since
> >execve does not create any new process, quote from man page of execve:
> >"there is no new process; many attributes of the calling process remain
> >unchanged (in particular, its PID)."
> 
> See commit bb188d7e64de ("ptrace: make former thread ID available via PTRACE_GETEVENTMSG after PTRACE_EVENT_EXEC stop")
> 
> This is for making sense of a concurrent exec made by a multi threaded process. Specifically see de_thread(), where the pid *can* change:
> 
>  /*
>   * At this point all other threads have exited, all we have to
> 	 * do is to wait for the thread group leader to become inactive,
> 	 * and to assume its PID:
> 	 */
> 
> The described problem in the commit hasn't changed, so this code needs to stay as-is. Or perhaps the comment could be improved?

Thanks for answering, interesting, I'll take a deeper look.

Nir