NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/57527: system() waits for last child if SIG_CHLD is ignored



The following reply was made to PR kern/57527; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: mlelstv%NetBSD.org@localhost
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: kern/57527: system() waits for last child if SIG_CHLD is ignored
Date: Mon, 17 Jul 2023 12:45:07 +0000

 This is a multi-part message in MIME format.
 --=_skMWPwotJfxMxufC+Kk6N1dx2rTmwBMC
 
 > Date: Sat, 15 Jul 2023 20:04:50 +0000
 > From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
 > 
 > The following clause in POSIX on wait/waitpid that mlelstv found
 > shortly after filing the bug looks relevant:
 > 
 >    If the calling process has SA_NOCLDWAIT set or has SIGCHLD set to
 >    SIG_IGN, and the process has no unwaited for children that were
 >    transformed into zombie processes, the calling thread will block
 >    until all of the children of the process containing the calling
 >    thread terminate, and wait() and waitpid() will fail and set errno
 >    to [ECHILD].
 > 
 >    https://pubs.opengroup.org/onlinepubs/7908799/xsh/wait.html
 
 Sorry, that's an ancient POSIX URL.  Corrected URL that has the quote:
 https://pubs.opengroup.org/onlinepubs/9699919799/functions/wait.html
 
 The attached three-patch series adds automatic tests for these corners
 of system(3), and shows that the temporary SIG_IGN -> SIG_DFL change
 addresses them.
 
 However, it doesn't address the case of a signal handler function (not
 SIG_IGN) with SA_NOCLDWAIT set, as demonstrated in the third patch.
 That sounds difficult to reconcile:
 
 - If we leave the signal handler in place, waitpid doesn't work.
 - If we touch sigaction, queued signals are lost.
 - If we try to requeue signals with raise or kill, siginfo is
   destroyed.
 
 What to do?  There's some long text in POSIX about whether wait(2)
 consumes (`accepts'/`discards') SIGCHLD signals and its interaction
 with system(3), but I'm out of time for now to think further about
 this.
 
 --=_skMWPwotJfxMxufC+Kk6N1dx2rTmwBMC
 Content-Type: text/plain; charset="ISO-8859-1"; name="pr57527-system-v3"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="pr57527-system-v3.patch"
 
 From 83ffe8640761b46ca49538a438c741f7f24d4acb Mon Sep 17 00:00:00 2001
 From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
 Date: Mon, 17 Jul 2023 11:51:09 +0000
 Subject: [PATCH 1/3] system(3): Add test for SA_NOCLDWAIT and SIG_IGN SIGCH=
 LD
  interaction.
 
 PR 57527
 ---
  tests/lib/libc/stdlib/Makefile           |   1 +
  tests/lib/libc/stdlib/t_system_sigchld.c | 459 +++++++++++++++++++++++
  2 files changed, 460 insertions(+)
  create mode 100644 tests/lib/libc/stdlib/t_system_sigchld.c
 
 diff --git a/tests/lib/libc/stdlib/Makefile b/tests/lib/libc/stdlib/Makefile
 index a7221b5033fc..493152d47882 100644
 --- a/tests/lib/libc/stdlib/Makefile
 +++ b/tests/lib/libc/stdlib/Makefile
 @@ -20,6 +20,7 @@ TESTS_C+=3D	t_strtod
  TESTS_C+=3D	t_strtol
  TESTS_C+=3D	t_strtoi
  TESTS_C+=3D	t_system
 +TESTS_C+=3D	t_system_sigchld
 =20
  TESTS_SH+=3D	t_atexit
  TESTS_SH+=3D	t_getopt
 diff --git a/tests/lib/libc/stdlib/t_system_sigchld.c b/tests/lib/libc/stdl=
 ib/t_system_sigchld.c
 new file mode 100644
 index 000000000000..9463fc5994ec
 --- /dev/null
 +++ b/tests/lib/libc/stdlib/t_system_sigchld.c
 @@ -0,0 +1,459 @@
 +/*	$NetBSD$	*/
 +
 +/*-
 + * Copyright (c) 2023 The NetBSD Foundation, Inc.
 + * All rights reserved.
 + *
 + * Redistribution and use in source and binary forms, with or without
 + * modification, are permitted provided that the following conditions
 + * are met:
 + * 1. Redistributions of source code must retain the above copyright
 + *    notice, this list of conditions and the following disclaimer.
 + * 2. Redistributions in binary form must reproduce the above copyright
 + *    notice, this list of conditions and the following disclaimer in the
 + *    documentation and/or other materials provided with the distribution.
 + *
 + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTO=
 RS
 + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIM=
 ITED
 + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICU=
 LAR
 + * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTO=
 RS
 + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
 + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
 + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
 + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
 + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
 + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF =
 THE
 + * POSSIBILITY OF SUCH DAMAGE.
 + */
 +
 +#include <sys/cdefs.h>
 +__RCSID("$NetBSD: t_system.c,v 1.1 2011/09/11 10:32:23 jruoho Exp $");
 +
 +#include <sys/wait.h>
 +
 +#include <atf-c.h>
 +#include <errno.h>
 +#include <fcntl.h>
 +#include <signal.h>
 +#include <stdlib.h>
 +#include <string.h>
 +#include <unistd.h>
 +
 +#include "h_macros.h"
 +
 +static void
 +emptyhandler(int signo)
 +{
 +}
 +
 +static bool woken;
 +
 +static void
 +wakeup(int signo)
 +{
 +
 +	woken =3D true;
 +}
 +
 +static bool sigchld;
 +
 +static void
 +sigchldhandler(int signo)
 +{
 +
 +	sigchld =3D true;
 +}
 +
 +static siginfo_t sigchldinfo;
 +
 +static void
 +sigchldaction(int signo, siginfo_t *info, void *ctx)
 +{
 +
 +	sigchld =3D true;
 +	sigchldinfo =3D *info;
 +}
 +
 +static void
 +go(bool dowait)
 +{
 +	int pfd[2], ffd;
 +	char c =3D 1;
 +	ssize_t n;
 +	int status =3D 0;
 +	pid_t child, waitret;
 +
 +	/*
 +	 * Create a pipe for communication with the asynchronous child
 +	 * process.
 +	 */
 +	RL(pipe(pfd));
 +
 +	/*
 +	 * Fork an asynchronous child process that will wait until
 +	 * we've written to the pipe before exiting.
 +	 */
 +	RL(child =3D fork());
 +	if (child =3D=3D 0) {
 +		if ((n =3D read(pfd[0], &c, 1)) =3D=3D -1)
 +			_exit(1);
 +		if (n !=3D 1)
 +			_exit(1);
 +		_exit(0);
 +	}
 +
 +	/*
 +	 * Set an alarm to kill the process if system(3) hasn't
 +	 * returned in 5sec.
 +	 */
 +	ATF_REQUIRE_MSG(signal(SIGALRM, &wakeup) !=3D SIG_ERR,
 +	    "signal(SIGALRM): %s", strerror(errno));
 +	RL((int)alarm(2));
 +
 +	/*
 +	 * Run a synchronous child process.  This should complete
 +	 * promptly without error.  Paranoia: make sure the output file
 +	 * is not exist already.
 +	 */
 +	(void)unlink("ok");
 +	RL(system("touch ok"));
 +
 +	/*
 +	 * Clear the alarm.  Fail if it had woken us already.
 +	 */
 +	RL((int)alarm(0));
 +	ATF_REQUIRE(!woken);
 +
 +	/*
 +	 * Make sure the synchronous child process took effect by
 +	 * creating a file `ok'.
 +	 */
 +	RL(ffd =3D open("ok", O_RDONLY));
 +	RL(close(ffd));
 +
 +	/*
 +	 * Wake the asynchronous child process so it can exit.
 +	 */
 +	RL(n =3D write(pfd[1], &c, 1));
 +	ATF_REQUIRE_MSG(n =3D=3D 1, "n=3D%zd", n);
 +
 +	/*
 +	 * Wait for the asynchronous child process -- or expect waitpid
 +	 * to fail with ECHILD if we can't.  Set an alarm so the wait
 +	 * can't take too long.
 +	 */
 +	RL((int)alarm(2));
 +	waitret =3D waitpid(child, &status, 0);
 +	if (dowait) {
 +		ATF_REQUIRE_EQ_MSG(waitret, child,
 +		    "waitret=3D%ld status=3D0x%x child=3D%ld",
 +		    (long)waitret, status, (long)child);
 +		ATF_REQUIRE(WIFEXITED(status));
 +		ATF_REQUIRE_EQ_MSG(WEXITSTATUS(status), 0,
 +		    "status=3D0x%x WEXITSTATUS(status)=3D%d",
 +		    status, WEXITSTATUS(status));
 +	} else {
 +		ATF_REQUIRE_EQ_MSG(waitret, -1,
 +		    "waitret=3D%ld status=3D0x%x child=3D%ld",
 +		    (long)waitret, status, (long)child);
 +		ATF_REQUIRE_EQ_MSG(errno, ECHILD, "errno=3D%d %s",
 +		    errno, strerror(errno));
 +	}
 +	RL((int)alarm(0));
 +	ATF_REQUIRE(!woken);
 +
 +	RL(close(pfd[0]));
 +	RL(close(pfd[1]));
 +}
 +
 +static void
 +goqueued(bool dowait)
 +{
 +	sigset_t block, omask;
 +	pid_t child0, waitret0;
 +	int status0 =3D 0;
 +
 +	/*
 +	 * Block SIGCHLD for most of the test -- we are going to make
 +	 * sure that the signal remains queued.
 +	 */
 +	RL(sigemptyset(&block));
 +	RL(sigaddset(&block, SIGCHLD));
 +	RL(sigprocmask(SIG_BLOCK, &block, &omask));
 +
 +	/*
 +	 * Fork a child that immediately exits -- this should promptly
 +	 * deliver a queued SIGCHLD.
 +	 */
 +	RL(child0 =3D fork());
 +	if (child0 =3D=3D 0)
 +		_exit(0);
 +
 +	/*
 +	 * Wait for the child and verify it exited cleanly.  This
 +	 * should ensure the SIGCHLD is queued before we proceed.
 +	 */
 +	waitret0 =3D waitpid(child0, &status0, 0);
 +	if (dowait) {
 +		ATF_REQUIRE_EQ_MSG(waitret0, child0,
 +		    "waitret0=3D%ld status0=3D0x%x child0=3D%ld",
 +		    (long)waitret0, status0, (long)child0);
 +		ATF_REQUIRE(WIFEXITED(status0));
 +		ATF_REQUIRE_EQ_MSG(WEXITSTATUS(status0), 0,
 +		    "status0=3D0x%x WEXITSTATUS(status0)=3D%d",
 +		    status0, WEXITSTATUS(status0));
 +	} else {
 +		ATF_REQUIRE_EQ_MSG(waitret0, -1,
 +		    "waitret0=3D%ld status0=3D0x%x child0=3D%ld",
 +		    (long)waitret0, status0, (long)child0);
 +		ATF_REQUIRE_EQ_MSG(errno, ECHILD, "errno=3D%d %s",
 +		    errno, strerror(errno));
 +	}
 +
 +	/*
 +	 * Do the main test.
 +	 */
 +	go(dowait);
 +
 +	/*
 +	 * Unblock SIGCHLD and verify that we got the signal.
 +	 */
 +	RL(sigprocmask(SIG_SETMASK, &omask, NULL));
 +	ATF_REQUIRE(sigchld);
 +}
 +
 +ATF_TC(system_default);
 +ATF_TC_HEAD(system_default, tc)
 +{
 +	atf_tc_set_md_var(tc, "descr", "system(3) with asynchronous child");
 +	atf_tc_set_md_var(tc, "require.progs", "touch");
 +}
 +ATF_TC_BODY(system_default, tc)
 +{
 +	go(/*dowait*/true);
 +}
 +
 +ATF_TC(system_nocldwait);
 +ATF_TC_HEAD(system_nocldwait, tc)
 +{
 +	atf_tc_set_md_var(tc, "descr",
 +	    "system(3) with asynchronous child using SA_NOCLDWAIT");
 +	atf_tc_set_md_var(tc, "require.progs", "touch");
 +}
 +ATF_TC_BODY(system_nocldwait, tc)
 +{
 +	struct sigaction sa =3D {
 +		.sa_handler =3D &emptyhandler,
 +		.sa_flags =3D SA_NOCLDWAIT,
 +	};
 +	struct sigaction nsa;
 +
 +	atf_tc_expect_fail("PR 57527");
 +
 +	RL(sigaction(SIGCHLD, &sa, NULL));
 +
 +	/*
 +	 * With SIGCHLD set with SA_NOCLDWAIT, the asynchronous child
 +	 * process will be reaped without waiting.
 +	 */
 +	go(/*dowait*/false);
 +
 +	/*
 +	 * Verify the signal configuration hasn't changed.
 +	 */
 +	RL(sigaction(SIGCHLD, NULL, &nsa));
 +	ATF_REQUIRE_EQ_MSG(nsa.sa_handler, &emptyhandler, "%p",
 +	    nsa.sa_handler);
 +	ATF_REQUIRE_EQ_MSG(nsa.sa_flags, SA_NOCLDWAIT, "0x%x", nsa.sa_flags);
 +}
 +
 +ATF_TC(system_sigign);
 +ATF_TC_HEAD(system_sigign, tc)
 +{
 +	atf_tc_set_md_var(tc, "descr",
 +	    "system(3) with asynchronous child using SIG_IGN");
 +	atf_tc_set_md_var(tc, "require.progs", "touch");
 +}
 +ATF_TC_BODY(system_sigign, tc)
 +{
 +	void (*h)(int);
 +
 +	atf_tc_expect_fail("PR 57527");
 +
 +	ATF_REQUIRE_MSG(signal(SIGCHLD, SIG_IGN) !=3D SIG_ERR,
 +	    "signal(SIGCHLD): %s", strerror(errno));
 +
 +	/*
 +	 * With SIGCHLD set to SIG_IGN, the asynchronous child process
 +	 * will be reaped without waiting.
 +	 */
 +	go(/*dowait*/false);
 +
 +	/*
 +	 * Verify the signal configuration hasn't changed.
 +	 */
 +	h =3D signal(SIGCHLD, SIG_DFL);
 +	ATF_REQUIRE_EQ_MSG(h, SIG_IGN, "%p", h);
 +}
 +
 +ATF_TC(system_sigign_nocldwait);
 +ATF_TC_HEAD(system_sigign_nocldwait, tc)
 +{
 +	atf_tc_set_md_var(tc, "descr",
 +	    "system(3) with asynchronous child"
 +	    " using SIG_IGN and SA_NOCLDWAIT");
 +	atf_tc_set_md_var(tc, "require.progs", "touch");
 +}
 +ATF_TC_BODY(system_sigign_nocldwait, tc)
 +{
 +	struct sigaction sa =3D {
 +		.sa_handler =3D SIG_IGN,
 +		.sa_flags =3D SA_NOCLDWAIT,
 +	};
 +	struct sigaction nsa;
 +
 +	atf_tc_expect_fail("PR 57527");
 +
 +	RL(sigaction(SIGCHLD, &sa, NULL));
 +
 +	/*
 +	 * With SIGCHLD set to SIG_IGN and/or SA_NOCLDWAIT set, the
 +	 * asynchronous child process will be reaped without waiting.
 +	 */
 +	go(/*dowait*/false);
 +
 +	RL(sigaction(SIGCHLD, NULL, &nsa));
 +	ATF_REQUIRE_EQ_MSG(nsa.sa_handler, SIG_IGN, "%p", nsa.sa_handler);
 +	ATF_REQUIRE_EQ_MSG(nsa.sa_flags, SA_NOCLDWAIT, "0x%x", nsa.sa_flags);
 +}
 +
 +ATF_TC(system_queued);
 +ATF_TC_HEAD(system_queued, tc)
 +{
 +	atf_tc_set_md_var(tc, "descr",
 +	    "system(3) with asynchronous child and a queued SIGCHLD");
 +	atf_tc_set_md_var(tc, "require.progs", "touch");
 +}
 +ATF_TC_BODY(system_queued, tc)
 +{
 +	void (*h)(int);
 +
 +	/*
 +	 * Set up a signal handler to set a flag when SIGCHLD is
 +	 * delivered.
 +	 */
 +	ATF_REQUIRE_MSG(signal(SIGCHLD, &sigchldhandler) !=3D SIG_ERR,
 +	    "signal(SIGCHLD): %s", strerror(errno));
 +
 +	goqueued(/*dowait*/true);
 +
 +	/*
 +	 * Verify the signal configuration hasn't changed.
 +	 */
 +	h =3D signal(SIGCHLD, SIG_DFL);
 +	ATF_REQUIRE_EQ_MSG(h, &sigchldhandler, "%p", h);
 +}
 +
 +ATF_TC(system_queued_action);
 +ATF_TC_HEAD(system_queued_action, tc)
 +{
 +	atf_tc_set_md_var(tc, "descr",
 +	    "system(3) with asynchronous child and a queued SIGCHLD,"
 +	    " with sigaction");
 +	atf_tc_set_md_var(tc, "require.progs", "touch");
 +}
 +ATF_TC_BODY(system_queued_action, tc)
 +{
 +	struct sigaction sa =3D {
 +		.sa_sigaction =3D &sigchldaction,
 +		.sa_flags =3D SA_SIGINFO,
 +	};
 +	struct sigaction nsa;
 +
 +	/*
 +	 * Set up a signal action to set a flag when SIGCHLD is
 +	 * delivered.
 +	 */
 +	RL(sigaction(SIGCHLD, &sa, NULL));
 +
 +	goqueued(/*dowait*/true);
 +
 +	/*
 +	 * Verify we got the right siginfo.
 +	 */
 +	ATF_CHECK_EQ_MSG(sigchldinfo.si_signo, SIGCHLD, "%d",
 +	    sigchldinfo.si_signo);
 +	ATF_CHECK_EQ_MSG(sigchldinfo.si_errno, 0, "%d",
 +	    sigchldinfo.si_errno);
 +	ATF_CHECK_EQ_MSG(sigchldinfo.si_code, CLD_EXITED, "%d",
 +	    sigchldinfo.si_code);
 +
 +	/*
 +	 * Verify the signal configuration hasn't changed.
 +	 */
 +	RL(sigaction(SIGCHLD, NULL, &nsa));
 +	ATF_REQUIRE_EQ_MSG(nsa.sa_sigaction, &sigchldaction,
 +	    "%p", nsa.sa_sigaction);
 +	ATF_REQUIRE_EQ_MSG(nsa.sa_flags, SA_SIGINFO,
 +	    "0x%x", nsa.sa_flags);
 +}
 +
 +ATF_TC(system_queued_nocldwait);
 +ATF_TC_HEAD(system_queued_nocldwait, tc)
 +{
 +	atf_tc_set_md_var(tc, "descr",
 +	    "system(3) with asynchronous child and a queued SIGCHLD,"
 +	    " with SA_NOCLDWAIT");
 +	atf_tc_set_md_var(tc, "require.progs", "touch");
 +}
 +ATF_TC_BODY(system_queued_nocldwait, tc)
 +{
 +	struct sigaction sa =3D {
 +		.sa_sigaction =3D &sigchldaction,
 +		.sa_flags =3D SA_NOCLDWAIT|SA_SIGINFO,
 +	};
 +	struct sigaction nsa;
 +
 +	atf_tc_expect_fail("PR 57527");
 +
 +	/*
 +	 * Set up a signal action to set a flag when SIGCHLD is
 +	 * delivered.
 +	 */
 +	RL(sigaction(SIGCHLD, &sa, NULL));
 +
 +	goqueued(/*dowait*/false);
 +
 +	/*
 +	 * Verify we got the right siginfo.
 +	 */
 +	ATF_CHECK_EQ_MSG(sigchldinfo.si_signo, SIGCHLD, "%d",
 +	    sigchldinfo.si_signo);
 +	ATF_CHECK_EQ_MSG(sigchldinfo.si_errno, 0, "%d",
 +	    sigchldinfo.si_errno);
 +	ATF_CHECK_EQ_MSG(sigchldinfo.si_code, CLD_EXITED, "%d",
 +	    sigchldinfo.si_code);
 +
 +	/*
 +	 * Verify the signal configuration hasn't changed.
 +	 */
 +	RL(sigaction(SIGCHLD, NULL, &nsa));
 +	ATF_REQUIRE_EQ_MSG(nsa.sa_sigaction, &sigchldaction,
 +	    "%p", nsa.sa_sigaction);
 +	ATF_REQUIRE_EQ_MSG(nsa.sa_flags, SA_NOCLDWAIT|SA_SIGINFO,
 +	    "0x%x", nsa.sa_flags);
 +}
 +
 +ATF_TP_ADD_TCS(tp)
 +{
 +
 +	ATF_TP_ADD_TC(tp, system_default);
 +	ATF_TP_ADD_TC(tp, system_nocldwait);
 +	ATF_TP_ADD_TC(tp, system_queued);
 +	ATF_TP_ADD_TC(tp, system_queued_action);
 +	ATF_TP_ADD_TC(tp, system_queued_nocldwait);
 +	ATF_TP_ADD_TC(tp, system_sigign);
 +	ATF_TP_ADD_TC(tp, system_sigign_nocldwait);
 +
 +	return atf_no_error();
 +}
 
 From daf08fb3988bd6de0ec12348c12a6131ddc279e5 Mon Sep 17 00:00:00 2001
 From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
 Date: Sat, 15 Jul 2023 20:02:28 +0000
 Subject: [PATCH 2/3] system(3): Change SIG_IGN to SIG_DFL for SIGCHLD while=
  we
  work.
 
 Note: This doesn't address the scenario that SIGCHLD is set to a
 handler function, not SIG_IGN, but has the SA_NOCLDWAIT flag set.
 
 PR 57527
 ---
  lib/libc/stdlib/system.c                 | 42 ++++++++++++++++++++++--
  tests/lib/libc/stdlib/t_system_sigchld.c |  4 ---
  2 files changed, 40 insertions(+), 6 deletions(-)
 
 diff --git a/lib/libc/stdlib/system.c b/lib/libc/stdlib/system.c
 index 5a19bc9e0568..8101278c1bd3 100644
 --- a/lib/libc/stdlib/system.c
 +++ b/lib/libc/stdlib/system.c
 @@ -54,7 +54,7 @@ int
  system(const char *command)
  {
  	pid_t pid;
 -	struct sigaction intsa, quitsa, sa;
 +	struct sigaction chldsa, intsa, quitsa, sa;
  	sigset_t nmask, omask, sigdefault;
  	int pstat;
  	const char *argp[] =3D {"sh", "-c", "--", command, NULL};
 @@ -90,6 +90,42 @@ system(const char *command)
  		return -1;
  	}
 =20
 +	/*
 +	 * Make sure SIGCHLD is not set to SIG_IGN, because that means
 +	 * waitpid will wait until _all_ children have exited, under a
 +	 * peculiar clause of POSIX enshrining a quirk of SysV
 +	 * semantics in wait/waitpid:
 +	 *
 +	 *    If the calling process has SA_NOCLDWAIT set or has
 +	 *    SIGCHLD set to SIG_IGN, and the process has no unwaited
 +	 *    for children that were transformed into zombie processes,
 +	 *    the calling thread will block until all of the children
 +	 *    of the process containing the calling thread terminate,
 +	 *    and wait() and waitpid() will fail and set errno to
 +	 *    [ECHILD].
 +	 *
 +	 * https://pubs.opengroup.org/onlinepubs/9699919799/functions/wait.html
 +	 *
 +	 * If SIGCHLD was set to anything other than SIG_IGN, we must
 +	 * not interfere with it -- changing the action may clear any
 +	 * queued signals for for processes _other_ than the one that
 +	 * system(3) itself spawns.
 +	 */
 +	if (sigaction(SIGCHLD, NULL, &chldsa) =3D=3D -1) {
 +		(void)sigprocmask(SIG_SETMASK, &omask, NULL);
 +		sigaction(SIGINT, &intsa, NULL);
 +		sigaction(SIGQUIT, &quitsa, NULL);
 +		return -1;
 +	}
 +	if (chldsa.sa_handler =3D=3D SIG_IGN &&
 +	    sigaction(SIGCHLD, &(struct sigaction){ .sa_handler =3D SIG_DFL },
 +		NULL) =3D=3D -1) {
 +		(void)sigprocmask(SIG_SETMASK, &omask, NULL);
 +		sigaction(SIGINT, &intsa, NULL);
 +		sigaction(SIGQUIT, &quitsa, NULL);
 +		return -1;
 +	}
 +
  	/*
  	 * We arrange to inherit all signal handlers from the caller by
  	 * default, except possibly SIGINT and SIGQUIT.  These we have
 @@ -133,7 +169,9 @@ system(const char *command)
  		}
  	}
 =20
 -out:	sigaction(SIGINT, &intsa, NULL);
 +out:	if (chldsa.sa_handler =3D=3D SIG_IGN)
 +		sigaction(SIGCHLD, &chldsa, NULL);
 +	sigaction(SIGINT, &intsa, NULL);
  	sigaction(SIGQUIT, &quitsa, NULL);
  	(void)sigprocmask(SIG_SETMASK, &omask, NULL);
 =20
 diff --git a/tests/lib/libc/stdlib/t_system_sigchld.c b/tests/lib/libc/stdl=
 ib/t_system_sigchld.c
 index 9463fc5994ec..78d44a8bd797 100644
 --- a/tests/lib/libc/stdlib/t_system_sigchld.c
 +++ b/tests/lib/libc/stdlib/t_system_sigchld.c
 @@ -278,8 +278,6 @@ ATF_TC_BODY(system_sigign, tc)
  {
  	void (*h)(int);
 =20
 -	atf_tc_expect_fail("PR 57527");
 -
  	ATF_REQUIRE_MSG(signal(SIGCHLD, SIG_IGN) !=3D SIG_ERR,
  	    "signal(SIGCHLD): %s", strerror(errno));
 =20
 @@ -312,8 +310,6 @@ ATF_TC_BODY(system_sigign_nocldwait, tc)
  	};
  	struct sigaction nsa;
 =20
 -	atf_tc_expect_fail("PR 57527");
 -
  	RL(sigaction(SIGCHLD, &sa, NULL));
 =20
  	/*
 
 From 57a2d906999c44e409cdf3bc50d84615b390a0c0 Mon Sep 17 00:00:00 2001
 From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
 Date: Mon, 17 Jul 2023 12:37:06 +0000
 Subject: [PATCH 3/3] WIP: system(3): defend against SA_NOCLDWAIT too
 
 ---
  lib/libc/stdlib/system.c                 | 4 ++--
  tests/lib/libc/stdlib/t_system_sigchld.c | 2 --
  2 files changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/lib/libc/stdlib/system.c b/lib/libc/stdlib/system.c
 index 8101278c1bd3..256324a0869e 100644
 --- a/lib/libc/stdlib/system.c
 +++ b/lib/libc/stdlib/system.c
 @@ -117,7 +117,7 @@ system(const char *command)
  		sigaction(SIGQUIT, &quitsa, NULL);
  		return -1;
  	}
 -	if (chldsa.sa_handler =3D=3D SIG_IGN &&
 +	if ((chldsa.sa_handler =3D=3D SIG_IGN || chldsa.sa_flags & SA_NOCLDWAIT) =
 &&
  	    sigaction(SIGCHLD, &(struct sigaction){ .sa_handler =3D SIG_DFL },
  		NULL) =3D=3D -1) {
  		(void)sigprocmask(SIG_SETMASK, &omask, NULL);
 @@ -169,7 +169,7 @@ system(const char *command)
  		}
  	}
 =20
 -out:	if (chldsa.sa_handler =3D=3D SIG_IGN)
 +out:	if (chldsa.sa_handler =3D=3D SIG_IGN || chldsa.sa_flags & SA_NOCLDWAI=
 T)
  		sigaction(SIGCHLD, &chldsa, NULL);
  	sigaction(SIGINT, &intsa, NULL);
  	sigaction(SIGQUIT, &quitsa, NULL);
 diff --git a/tests/lib/libc/stdlib/t_system_sigchld.c b/tests/lib/libc/stdl=
 ib/t_system_sigchld.c
 index 78d44a8bd797..88ad880efef2 100644
 --- a/tests/lib/libc/stdlib/t_system_sigchld.c
 +++ b/tests/lib/libc/stdlib/t_system_sigchld.c
 @@ -248,8 +248,6 @@ ATF_TC_BODY(system_nocldwait, tc)
  	};
  	struct sigaction nsa;
 =20
 -	atf_tc_expect_fail("PR 57527");
 -
  	RL(sigaction(SIGCHLD, &sa, NULL));
 =20
  	/*
 
 --=_skMWPwotJfxMxufC+Kk6N1dx2rTmwBMC--
 


Home | Main Index | Thread Index | Old Index