kern/60962: fork and spawn read p_pgrp without proc_lock

["campbell+netbsd%mumble.net@localhost via gnats" <gnats-admin%NetBSD.org@localhost>]

>Number:         60962
>Category:       kern
>Synopsis:       fork and spawn read p_pgrp without proc_lock
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Nov 04 18:10:01 +0000 2026
>Originator:     Taylor R Campbell
>Release:        current, 11, 10, ...
>Organization:
The ForkBSD Foundarace
>Environment:
>Description:

	As documented in proc.h:

    210 /*
    211  * Description of a process.
...
    221  * Field markings and the corresponding locks:
...
    225  * l:	proc_lock
...
    230  */
...
    233 struct proc {
...
    320 /*
    321  * The following fields are all copied upon creation in fork.
    322  */
    323 #define	p_startcopy	p_sigctx
    324 
    325 	struct sigctx 	p_sigctx;	/* p: Shared signal state */
...
    330 	struct pgrp 	*p_pgrp;	/* l: Pointer to process group */
...
    336 /*
    337  * End area that is copied on creation
    338  */
    339 #define	p_endcopy	p_xsig
...

	https://nxr.netbsd.org/xref/src/sys/sys/proc.h?r=1.375#210

	But fork (and posix_spawn) will read p_pgrp from the parent,
	implicitly via memcpy of p_startcopy through p_endcopy, without
	holding proc_lock, and leave a possibly stale pointer to it in
	the child:

    340 	/*
    341 	 * Make a proc table entry for the new process.
    342 	 * Start by zeroing the section of proc that is zero-initialized,
    343 	 * then copy the section that is copied directly from the parent.
    344 	 */
    345 	memset(&p2->p_startzero, 0,
    346 	    (unsigned) ((char *)&p2->p_endzero - (char *)&p2->p_startzero));
    347 	memcpy(&p2->p_startcopy, &p1->p_startcopy,
    348 	    (unsigned) ((char *)&p2->p_endcopy - (char *)&p2->p_startcopy));
...
    521 	/*
    522 	 * It's now safe for the scheduler and other processes to see the
    523 	 * child process.
    524 	 */
    525 	mutex_enter(&proc_lock);

	https://nxr.netbsd.org/xref/src/sys/kern/kern_fork.c?r=1.236#340

	A concurrent thread in the parent can call setsid or setpgid
	and thereby its own p_pgrp in the interim before fork takes
	proc_lock and inserts the child on the (now wrong) process
	group list:

    543 	LIST_INSERT_AFTER(p1, p2, p_pglist);

	https://nxr.netbsd.org/xref/src/sys/kern/kern_fork.c?r=1.236#543

	Reported by alnsn@.

>How-To-Repeat:

	1. fork in one thread
	2. setsid/setpgid in another thread
	3. ???
	4. profit!

>Fix:

	1. Ignore the copy of p_pgrp copied with memcpy.

	2. Copy p_pgrp from the parent to the child only under
	   proc_lock, which must be held across that copy and the
	   LIST_INSERT_AFTER in p_pglist.

	3. In a separate change, because it affects the kernel ABI,
	   move the p_pgrp member out of the p_startcopy/endcopy region
	   of struct proc, and add a note to proc.h that only members
	   which are stable after process creation or changed under
	   struct proc::p_lock can be included in that region.