NetBSD Problem Report #59523

From martin@aprisoft.de  Wed Jul  9 11:26:35 2025
Return-Path: <martin@aprisoft.de>
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)
	 client-signature RSA-PSS (2048 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 9591D1A923C
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  9 Jul 2025 11:26:35 +0000 (UTC)
Message-Id: <20250709112619.D457E5CC794@emmas.aprisoft.de>
Date: Wed, 09 Jul 2025 13:26:19 +0200 (CEST)
From: martin@NetBSD.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: posix_spawn(2) may fail eroneously trying to close an already closed file descriptor
X-Send-Pr-Version: 3.95

>Number:         59523
>Category:       kern
>Synopsis:       posix_spawn(2) may fail eroneously trying to close an already closed file descriptor
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          needs-pullups
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jul 09 11:30:01 +0000 2025
>Closed-Date:    
>Last-Modified:  Fri Jul 11 18:20:01 +0000 2025
>Originator:     Martin Husemann
>Release:        NetBSD 10.99.14
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD martins.aprisoft.de 10.99.14 NetBSD 10.99.14 (GENERIC) #270: Wed Jul 9 12:36:43 CEST 2025 martin@martins.aprisoft.de:/home/martin/current/src/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:

This came up due to syzkaller failing on NetBSD. Their code uses posix_spawnp(3)
and adds a "close" file action for every possible file descriptor starting one
higher than the highest descriptor they use themself up to the configured maximum.

This causes the posix_spawn(2) call to fail with EBADF, which is in violation
of posix:

  If the file_actions argument is not a null pointer, and specifies any
  chdir, close, dup2, fchdir, or open actions to be performed, and if
  posix_spawn() or posix_spawnp() fails for any of the reasons that would
  cause chdir(), close(), dup2(), fchdir(), or open() to fail, other than
							       ~~~~~~~~~~
  attempting a close() on a file descriptor that is in range but already
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  closed, an error value shall be returned [...]
  ~~~~~~

Original report:
	https://github.com/google/syzkaller/issues/6163

>How-To-Repeat:

Run code like this (from a new ATF test):

	int status, fd;
	pid_t pid;
	char * const args[2] = { __UNCONST("ls"), NULL };
	posix_spawn_file_actions_t fa;
	posix_spawnattr_t attr;

	/* get a free file descriptor number */
	fd = open("/dev/null", O_RDONLY);
	ATF_REQUIRE(fd >= 0);
	close(fd);
	RZ(posix_spawn_file_actions_init(&fa));
	// redirect output to /dev/null to not garble atf test results
	RZ(posix_spawn_file_actions_addopen(&fa, STDOUT_FILENO, "/dev/null",
	    O_WRONLY, 0));
	// known closed fd
	RZ(posix_spawn_file_actions_addclose(&fa, fd));
	// a random fd we know nothing about (cross fingers!
	RZ(posix_spawn_file_actions_addclose(&fa, fd+1));
	// high fd probably not ever been allocated, likely to trigger
	// a fd_getfile() failure in the kernel, which is another
	// path that originaly caused the fallout in syzkaller
	RZ(posix_spawn_file_actions_addclose(&fa, 560));

	RZ(posix_spawnattr_init(&attr));
	RZ(posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP));

	RZ(posix_spawn(&pid, "/bin/ls", &fa, &attr, args, NULL));
	RZ(posix_spawn_file_actions_destroy(&fa));

	/* ok, wait for the child to finish */
	RL(waitpid(pid, &status, 0));
	ATF_REQUIRE_MSG((WIFEXITED(status) &&
		WEXITSTATUS(status) == EXIT_SUCCESS),
	    "status=0x%x", status);


and watch it fail with posix_spawn returning EBADF.
There is another path in the kernel (where the parent process does not wait
due to no POSIX_SPAWN_SETPGROUP or the spawn attr pointer being NULL)
that makes the posix_spawn call return 127 in this case.

But it should return 0 (and actually run the client process) in both cases.

>Fix:
upcoming...

>Release-Note:

>Audit-Trail:
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59523 CVS commit: src/tests/lib/libc/gen/posix_spawn
Date: Wed, 9 Jul 2025 11:40:43 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Wed Jul  9 11:40:43 UTC 2025

 Modified Files:
 	src/tests/lib/libc/gen/posix_spawn: t_fileactions.c

 Log Message:
 Add two test cases triggereing the two paths in posix_spawn
 that cause PR 59523.


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/tests/lib/libc/gen/posix_spawn/t_fileactions.c

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

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59523 CVS commit: src/sys/kern
Date: Wed, 9 Jul 2025 11:43:48 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Wed Jul  9 11:43:48 UTC 2025

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

 Log Message:
 PR 59523: remove error handling for posix_spawn() close file actions,
 they are specified to be silently ignored.


 To generate a diff of this commit:
 cvs rdiff -u -r1.528 -r1.529 src/sys/kern/kern_exec.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: martin@NetBSD.org
State-Changed-When: Wed, 09 Jul 2025 11:55:32 +0000
State-Changed-Why:
Fixed, awaiting confirmation of the original report,
then needs to be pulled up to -10 and -9.


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59523 CVS commit: src/tests/lib/libc/gen/posix_spawn
Date: Thu, 10 Jul 2025 05:37:45 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Thu Jul 10 05:37:45 UTC 2025

 Modified Files:
 	src/tests/lib/libc/gen/posix_spawn: t_fileactions.c

 Log Message:
 In the test code for PR 59523 move the redirect of stdout to the end of
 the fileactions, to avoid any interference with the assumptions of the
 test-relevant file actions.
 We can not guarantee anny of this anyway (but it did trigger the original
 problem and both paths in the kernel before the bug was fixed).
 Pointed out by kre.


 To generate a diff of this commit:
 cvs rdiff -u -r1.9 -r1.10 src/tests/lib/libc/gen/posix_spawn/t_fileactions.c

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

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59523 CVS commit: src/sys/kern
Date: Fri, 11 Jul 2025 18:18:30 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Fri Jul 11 18:18:29 UTC 2025

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

 Log Message:
 PR 59523: initialize errror to 0, not all switch cases overwrite it
 now before it is used.
 Pointed out by hannken.


 To generate a diff of this commit:
 cvs rdiff -u -r1.529 -r1.530 src/sys/kern/kern_exec.c

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

>Unformatted:

Home	PR Database Search