NetBSD Problem Report #60962

From www@netbsd.org  Wed Nov  4 18:09:07 2026
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.netbsd.org", Issuer "YR1" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 6A5221A923B
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  4 Nov 2026 18:09:07 +0000 (UTC)
Message-Id: <20261104180905.C93631A923C@mollari.NetBSD.org>
Date: Wed,  4 Nov 2026 18:09:05 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: fork and spawn read p_pgrp without proc_lock
X-Send-Pr-Version: www-1.0
X-From4GNATS: "campbell+netbsd@mumble.net via gnats" <gnats-admin@NetBSD.org>

>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:          needs-pullups
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Nov 04 18:10:01 +0000 2026
>Closed-Date:    
>Last-Modified:  Thu Nov 05 00:15:00 +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.

>Release-Note:

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/60962 CVS commit: src/sys/kern
Date: Wed, 4 Nov 2026 18:18:00 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Wed Nov  4 18:18:00 UTC 2026

 Modified Files:
 	src/sys/kern: kern_exec.c kern_fork.c

 Log Message:
 fork, posix_spawn: Avoid race with setsid/setpgid.

 PR kern/60962: fork and spawn read p_pgrp without proc_lock


 To generate a diff of this commit:
 cvs rdiff -u -r1.535 -r1.536 src/sys/kern/kern_exec.c
 cvs rdiff -u -r1.236 -r1.237 src/sys/kern/kern_fork.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Wed, 04 Nov 2026 18:32:36 +0000
State-Changed-Why:
fixed in HEAD, needs pullup-11 and pullup-10


From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/60962 CVS commit: src/sys/sys
Date: Wed, 4 Nov 2026 18:41:11 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Wed Nov  4 18:41:11 UTC 2026

 Modified Files:
 	src/sys/sys: param.h proc.h

 Log Message:
 struct proc: Move p_pgrp out of p_startcopy/endcopy region.

 It cannot be safely copied with the current memcpy logic that logic
 uses happens without holding proc_lock, so we have to copy it
 separately anyway.  Leave a comment emphasizing this aspect of the
 locking rules so we don't have this bug again.

 Looks like p_mqueue_cnt may need similar treatment.

 As a result of this kernel ABI change, welcome to 11.99.10!

 PR kern/60962: fork and spawn read p_pgrp without proc_lock


 To generate a diff of this commit:
 cvs rdiff -u -r1.748 -r1.749 src/sys/sys/param.h
 cvs rdiff -u -r1.375 -r1.376 src/sys/sys/proc.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/60962: fork and spawn read p_pgrp without proc_lock
Date: Thu, 05 Nov 2026 07:10:17 +0700

 It looks to me there's an inherent race in the situation described,
 anyway, given the 2 threads running, one doing a fork() the other
 setsid() or setpgid(), the situation described

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

 might just as easily be

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

 and I don't believe

 	4. profit!

 applies at all.   This is a simple code bug in the application.
 If it cares what sid or pgrp the child gets, it shouldn't be
 altering those things at or around the time of the fork().

 Further, I don't think the change you made really alters anything,
 or not anything significant, if anything it makes it slightly more
 likely that the race will go the way of the fork'd child getting the
 pgrp from after the change (but obviously can't guarantee that, as in
 the first of the two scenarios above, 1 might be finished before 2 starts).

 That is unless you're imagining some kind of corruption if the code writing
 to p_pgrp can somehow mangle the value read from it by the copy.
 At the time the memcpy() is done, the target process (the one truly
 vulnerable here) doesn't yet exist as far as the rest of the kernel
 can tell (not yet linked anywhere), so nothing needs to be locked
 to alter it.

 kre


>Unformatted:

Home	PR Database Search