[PATCH v2 0/3] perf: Fix SIGCHLD vs pause() race with short-lived workloads

[PATCH v2 0/3] perf: Fix SIGCHLD vs pause() race with short-lived workloads

[Swapnil Sapkal]

Several perf subcommands (sched stats, lock contention) use the pattern
of forking a workload child, calling evlist__start_workload() to uncork
it, and then calling pause() to wait for a signal (typically SIGCHLD
when the child exits, or SIGINT/SIGTERM from the user).

This pattern has a race condition: if the workload is very short-lived,
the child can exit and deliver SIGCHLD in the window between
evlist__start_workload() and pause(). Since pause() only returns when a
signal is received *while the process is suspended*, and SIGCHLD has
already been delivered and handled by the empty sighandler(), pause()
blocks indefinitely.

The fix uses the standard POSIX pattern for this class of bug:

1. Block SIGCHLD, SIGINT, and SIGTERM (via sigprocmask) after
   evlist__prepare_workload() returns but before
   evlist__start_workload(). Blocking after the fork ensures the
   child workload does not inherit a modified signal mask.

2. Replace pause() with sigsuspend(&oldmask), which atomically
   restores the original mask and suspends the process. There is no
   window where any signal can slip through unnoticed.

3. Restore the original signal mask after sigsuspend() returns.

All three signals are blocked (not just SIGCHLD) so that an early
Ctrl+C during the remaining setup before sigsuspend() cannot be
consumed and lost, which would cause a hang in system-wide mode where
no SIGCHLD would follow.

Three call sites are affected across two files:
  - perf_sched__schedstat_record() in builtin-sched.c
  - perf_sched__schedstat_live()   in builtin-sched.c
  - __cmd_contention()             in builtin-lock.c

The two pause() sites in builtin-kwork.c are NOT affected because they
do not register SIGCHLD or fork workload children; they only wait for
user-initiated SIGINT/SIGTERM.

Changes since v1:
  - Moved sigprocmask() to after evlist__prepare_workload() so the
    forked child does not inherit a blocked SIGCHLD mask, which would
    break workloads relying on SIGCHLD (e.g., Node.js, Python asyncio).
    (Sashiko review)
  - Block SIGINT and SIGTERM in addition to SIGCHLD to prevent an
    early Ctrl+C during setup from being consumed before sigsuspend().
  - Error paths before sigprocmask no longer need mask restoration
    since the mask is not yet modified at that point.
    (Sashiko review)

Swapnil Sapkal (3):
  perf sched stats: Fix SIGCHLD race in schedstat_record()
  perf sched stats: Fix SIGCHLD race in schedstat_live()
  perf lock contention: Fix SIGCHLD race in __cmd_contention()

 tools/perf/builtin-lock.c  | 18 ++++++++++++++++--
 tools/perf/builtin-sched.c | 30 ++++++++++++++++++++++++++----
 2 files changed, 42 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] perf sched stats: Fix SIGCHLD race in schedstat_record()

[Swapnil Sapkal]

When a very short-lived workload is used with 'perf sched stats record',
the child process can exit and deliver SIGCHLD between
evlist__start_workload() and pause(). Since pause() only returns when a
signal is received while suspended, and the SIGCHLD has already been
delivered and handled by then, pause() blocks indefinitely.

Fix this by blocking SIGCHLD, SIGINT and SIGTERM after
evlist__prepare_workload() (so the forked child does not inherit a
modified signal mask) and replacing pause() with sigsuspend().
sigsuspend() atomically restores the original mask and suspends the
process, ensuring no signal is lost regardless of how quickly the child
exits or the user sends Ctrl+C.

Reviewed-by: James Clark <james.clark@linaro.org>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
---
 tools/perf/builtin-sched.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 3f509cfdd58c..3f448acd5046 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -3807,6 +3807,7 @@ const char *output_name;
 static int perf_sched__schedstat_record(struct perf_sched *sched,
 					int argc, const char **argv)
 {
+	sigset_t sig_mask, oldmask;
 	struct perf_session *session;
 	struct target target = {};
 	struct evlist *evlist;
@@ -3899,11 +3900,25 @@ static int perf_sched__schedstat_record(struct perf_sched *sched,
 	if (err < 0)
 		goto out;
 
+	/*
+	 * Block all handled signals so that a short-lived workload exiting
+	 * (SIGCHLD) or an early Ctrl+C (SIGINT/SIGTERM) during the remaining
+	 * setup cannot be delivered before we are ready to wait. sigsuspend()
+	 * below will atomically unblock them. This is done after
+	 * evlist__prepare_workload() so the forked child does not inherit a
+	 * modified signal mask.
+	 */
+	sigemptyset(&sig_mask);
+	sigaddset(&sig_mask, SIGCHLD);
+	sigaddset(&sig_mask, SIGINT);
+	sigaddset(&sig_mask, SIGTERM);
+	sigprocmask(SIG_BLOCK, &sig_mask, &oldmask);
+
 	if (argc)
 		evlist__start_workload(evlist);
 
-	/* wait for signal */
-	pause();
+	sigsuspend(&oldmask);
+	sigprocmask(SIG_SETMASK, &oldmask, NULL);
 
 	if (reset) {
 		err = disable_sched_schedstat();
-- 
2.43.0

Re: [PATCH v2 1/3] perf sched stats: Fix SIGCHLD race in schedstat_record()

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [Medium] The patch leaves a race window where early signals (like SIGINT) are lost, causing sigsuspend() to hang indefinitely.
--

commit 82e42f3b8c8062e3b83019c07f66b6e7199c4104
Author: Swapnil Sapkal <swapnil.sapkal@amd.com>

perf sched stats: Fix SIGCHLD race in schedstat_record()

This commit attempts to fix a SIGCHLD race in perf sched stats record by
replacing pause() with sigsuspend(). It blocks SIGCHLD, SIGINT, and SIGTERM
before starting the workload to ensure no signal is lost if the child exits
quickly.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c

[ ... ]

> @@ -3899,11 +3900,25 @@ static int perf_sched__schedstat_record(struct perf_sched *sched,
>  	if (err < 0)
>  		goto out;
>  
> +	/*
> +	 * Block all handled signals so that a short-lived workload exiting
> +	 * (SIGCHLD) or an early Ctrl+C (SIGINT/SIGTERM) during the remaining
> +	 * setup cannot be delivered before we are ready to wait. sigsuspend()
> +	 * below will atomically unblock them. This is done after
> +	 * evlist__prepare_workload() so the forked child does not inherit a
> +	 * modified signal mask.
> +	 */
> +	sigemptyset(&sig_mask);
> +	sigaddset(&sig_mask, SIGCHLD);
> +	sigaddset(&sig_mask, SIGINT);
> +	sigaddset(&sig_mask, SIGTERM);
> +	sigprocmask(SIG_BLOCK, &sig_mask, &oldmask);

Does this leave a race window where early signals like SIGINT can be lost,
causing sigsuspend() to hang indefinitely?

If a signal like SIGINT or SIGTERM is delivered before this sigprocmask()
call, the existing sighandler will consume it without setting any state flag
that gets checked here.

>  	if (argc)
>  		evlist__start_workload(evlist);
>  
> -	/* wait for signal */
> -	pause();
> +	sigsuspend(&oldmask);

When execution subsequently reaches sigsuspend(), wouldn't it block
indefinitely waiting for an event that was already processed?

To robustly resolve the TOCTOU race without leaking a modified signal mask
to the child, should the signal handler set a volatile sig_atomic_t flag,
and sigsuspend() be called conditionally based on that flag?

> +	sigprocmask(SIG_SETMASK, &oldmask, NULL);
>  
>  	if (reset) {
>  		err = disable_sched_schedstat();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260409162249.25581-1-swapnil.sapkal@amd.com?part=1

[PATCH v2 2/3] perf sched stats: Fix SIGCHLD race in schedstat_live()

[Swapnil Sapkal]

The same signal race that exists in perf_sched__schedstat_record() also
affects perf_sched__schedstat_live(). A very short-lived workload can
exit and deliver SIGCHLD before pause() is entered, causing an
indefinite hang.

Apply the same fix: block SIGCHLD, SIGINT and SIGTERM after the
workload is forked (evlist__prepare_workload) but before it is started
(evlist__start_workload), then replace pause() with sigsuspend() to
atomically unblock and wait.

Reviewed-by: James Clark <james.clark@linaro.org>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
---
 tools/perf/builtin-sched.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 3f448acd5046..c716dd33a9c9 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -4653,6 +4653,7 @@ static int perf_sched__schedstat_live(struct perf_sched *sched,
 				      int argc, const char **argv)
 {
 	struct cpu_domain_map **cd_map = NULL;
+	sigset_t sig_mask, oldmask;
 	struct target target = {};
 	u32 __maybe_unused md;
 	struct evlist *evlist;
@@ -4702,11 +4703,17 @@ static int perf_sched__schedstat_live(struct perf_sched *sched,
 	if (err < 0)
 		goto out;
 
+	sigemptyset(&sig_mask);
+	sigaddset(&sig_mask, SIGCHLD);
+	sigaddset(&sig_mask, SIGINT);
+	sigaddset(&sig_mask, SIGTERM);
+	sigprocmask(SIG_BLOCK, &sig_mask, &oldmask);
+
 	if (argc)
 		evlist__start_workload(evlist);
 
-	/* wait for signal */
-	pause();
+	sigsuspend(&oldmask);
+	sigprocmask(SIG_SETMASK, &oldmask, NULL);
 
 	if (reset) {
 		err = disable_sched_schedstat();
-- 
2.43.0

Re: [PATCH v2 2/3] perf sched stats: Fix SIGCHLD race in schedstat_live()

[sashiko-bot]

Sashiko AI review found 2 potential issue(s):
- [High] Unconditional use of sigsuspend() without a state flag creates a window where early signals are lost, leading to an indefinite hang.
- [High] Workload child process is orphaned if the tool is interrupted via SIGINT/SIGTERM.
--

commit f1c0b504081c33314d50fb6bf644c4e537e47385
Author: Swapnil Sapkal <swapnil.sapkal@amd.com>
Subject: perf sched stats: Fix SIGCHLD race in schedstat_live()

This commit attempts to fix a lost-wakeup race by blocking SIGCHLD, SIGINT,
and SIGTERM before starting the workload, and then using sigsuspend()
instead of pause() to wait for the workload to finish.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 3f448acd5046f..c716dd33a9c93 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -4702,11 +4703,17 @@ static int perf_sched__schedstat_live(struct perf_sched *sched,
>  	if (err < 0)
>  		goto out;
>  
> +	sigemptyset(&sig_mask);
> +	sigaddset(&sig_mask, SIGCHLD);
> +	sigaddset(&sig_mask, SIGINT);
> +	sigaddset(&sig_mask, SIGTERM);
> +	sigprocmask(SIG_BLOCK, &sig_mask, &oldmask);

Will this still hang if a signal arrives before sigprocmask() is called?

Because the registered signal handler is an empty dummy function without
a state flag, any signal arriving before sigprocmask() will be consumed
by the handler.

If a signal such as SIGINT or a premature SIGCHLD arrives during the earlier
setup phases (e.g., during /proc/schedstat parsing), the signal is
discarded and sigsuspend() will block indefinitely waiting for a signal
that has already arrived.

Should this handler set a flag (like a volatile sig_atomic_t) that is
checked after the sigprocmask(SIG_BLOCK) call but before sigsuspend()?

> +
>  	if (argc)
>  		evlist__start_workload(evlist);
>  
> -	/* wait for signal */
> -	pause();
> +	sigsuspend(&oldmask);
> +	sigprocmask(SIG_SETMASK, &oldmask, NULL);

Does the exit path leave the child workload running if interrupted?

While not introduced by this patch, if sigsuspend() returns due to an
interruption like SIGINT, the code proceeds to the cleanup path.

This path calls evlist__delete() but does not appear to send SIGTERM or
SIGKILL to the workload child process (evlist->workload.pid), nor does it
waitpid() for the child to terminate.

Could this leave the workload executing as an orphaned process in the
background, leaking resources?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260409162249.25581-1-swapnil.sapkal@amd.com?part=2

[PATCH v2 3/3] perf lock contention: Fix SIGCHLD race in __cmd_contention()

[Swapnil Sapkal]

__cmd_contention() in builtin-lock.c has the same signal race condition
as the perf sched stats code paths. When running with a short-lived
workload via 'perf lock contention -- <cmd>', the child can exit and
deliver SIGCHLD before pause() is entered, causing an indefinite hang.

Fix this by blocking SIGCHLD, SIGINT and SIGTERM after the workload is
forked but before it is started, then replacing pause() with
sigsuspend() to atomically unblock and wait. Blocking is done after
evlist__prepare_workload() so the child does not inherit a modified
signal mask that could break workloads relying on SIGCHLD delivery.

Reviewed-by: James Clark <james.clark@linaro.org>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
---
 tools/perf/builtin-lock.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index e8962c985d34..2da689de134a 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -1991,6 +1991,7 @@ static int check_lock_contention_options(const struct option *options,
 
 static int __cmd_contention(int argc, const char **argv)
 {
+	sigset_t sig_mask, oldmask;
 	int err = -EINVAL;
 	struct perf_tool eops;
 	struct perf_data data = {
@@ -2123,12 +2124,25 @@ static int __cmd_contention(int argc, const char **argv)
 	}
 
 	if (use_bpf) {
+		/*
+		 * Block all handled signals after evlist__prepare_workload()
+		 * so the forked child does not inherit a modified signal
+		 * mask, then use sigsuspend() to atomically unblock and
+		 * wait. This prevents a race where a short-lived workload
+		 * exits and delivers SIGCHLD before we are ready to wait.
+		 */
+		sigemptyset(&sig_mask);
+		sigaddset(&sig_mask, SIGCHLD);
+		sigaddset(&sig_mask, SIGINT);
+		sigaddset(&sig_mask, SIGTERM);
+		sigprocmask(SIG_BLOCK, &sig_mask, &oldmask);
+
 		lock_contention_start();
 		if (argc)
 			evlist__start_workload(con.evlist);
 
-		/* wait for signal */
-		pause();
+		sigsuspend(&oldmask);
+		sigprocmask(SIG_SETMASK, &oldmask, NULL);
 
 		lock_contention_stop();
 		lock_contention_read(&con);
-- 
2.43.0

Re: [PATCH v2 3/3] perf lock contention: Fix SIGCHLD race in __cmd_contention()

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [High] Unconditional use of `sigsuspend()` without a state flag fails to fix the race condition and causes indefinite hangs.
--

commit bbcae9323bad3368550c5fd9165d487b92cd3bbd
Author: Swapnil Sapkal <swapnil.sapkal@amd.com>

perf lock contention: Fix SIGCHLD race in __cmd_contention()

This commit replaces pause() with sigsuspend() to prevent a race condition
where a short-lived workload exits and delivers SIGCHLD before the wait
begins.

> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index e8962c985d34a..2da689de134a4 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c

[ ... ]

> @@ -2123,12 +2124,25 @@ static int __cmd_contention(int argc, const char **argv)
>  	}
>  
>  	if (use_bpf) {
> +		/*
> +		 * Block all handled signals after evlist__prepare_workload()
> +		 * so the forked child does not inherit a modified signal
> +		 * mask, then use sigsuspend() to atomically unblock and
> +		 * wait. This prevents a race where a short-lived workload
> +		 * exits and delivers SIGCHLD before we are ready to wait.
> +		 */
> +		sigemptyset(&sig_mask);
> +		sigaddset(&sig_mask, SIGCHLD);
> +		sigaddset(&sig_mask, SIGINT);
> +		sigaddset(&sig_mask, SIGTERM);
> +		sigprocmask(SIG_BLOCK, &sig_mask, &oldmask);
> +
>  		lock_contention_start();
>  		if (argc)
>  			evlist__start_workload(con.evlist);
>  
> -		/* wait for signal */
> -		pause();
> +		sigsuspend(&oldmask);

If SIGINT or SIGCHLD is delivered before sigprocmask() blocks them, won't it
be consumed by sighandler without setting any checked state, causing
sigsuspend() to hang indefinitely?

The signals are registered earlier in the function, leaving a window during
evlist__prepare_workload() and lock_contention_prepare() where they are
unblocked.

Should there be a volatile sig_atomic_t flag set by sighandler that is
checked in a loop, for example:

while (!done)
        sigsuspend(&oldmask);

This would avoid missing signals that arrive before the block and ensure
sigsuspend() doesn't hang.

> +		sigprocmask(SIG_SETMASK, &oldmask, NULL);
>  
>  		lock_contention_stop();
>  		lock_contention_read(&con);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260409162249.25581-1-swapnil.sapkal@amd.com?part=3