Re: [PATCH] [RFC] Add test to detect spurious suspend wakeups

On 12.05.26 09:17, Richard Weinberger wrote:
> If a task is suspend and SIGSHADOW is used to harden it,
> it can happen that the target task itself harends (e.g. by using
> a syscall). Then the already hardened and suspended task will see
> SIGSHADOW and resumes.
> 
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Richard Weinberger <[email protected]>
> ---
>  configure.ac                                  |   1 +
>  testsuite/smokey/Makefile.am                  |   3 +
>  .../smokey/spurious-suspend-wake/Makefile.am  |  10 +
>  .../spurious-suspend-wake.c                   | 238 ++++++++++++++++++
>  4 files changed, 252 insertions(+)
>  create mode 100644 testsuite/smokey/spurious-suspend-wake/Makefile.am
>  create mode 100644 testsuite/smokey/spurious-suspend-wake/spurious-suspend-wake.c
> 
> diff --git a/configure.ac b/configure.ac
> index 0d6dd87b6..69d9947fd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1086,6 +1086,7 @@ AC_CONFIG_FILES([ \
>  	testsuite/smokey/bufp/Makefile \
>  	testsuite/smokey/sigdebug/Makefile \
>  	testsuite/smokey/timerfd/Makefile \
> +	testsuite/smokey/spurious-suspend-wake/Makefile \
>  	testsuite/smokey/tsc/Makefile \
>  	testsuite/smokey/leaks/Makefile \
>  	testsuite/smokey/memcheck/Makefile \
> diff --git a/testsuite/smokey/Makefile.am b/testsuite/smokey/Makefile.am
> index c49a5d77f..f3c1151b6 100644
> --- a/testsuite/smokey/Makefile.am
> +++ b/testsuite/smokey/Makefile.am
> @@ -37,6 +37,7 @@ COBALT_SUBDIRS = 	\
>  	setsched	\
>  	sigdebug	\
>  	timerfd		\
> +	spurious-suspend-wake \
>  	tsc		\
>  	xddp		\
>  	y2038		\
> @@ -82,6 +83,7 @@ DIST_SUBDIRS = 		\
>  	setsched	\
>  	sigdebug	\
>  	timerfd		\
> +	spurious-suspend-wake \
>  	tsc		\
>  	xddp		\
>  	y2038		\
> @@ -130,6 +132,7 @@ smokey_LDADD = 					\
>  	$(plugin_list)				\
>  	../../lib/smokey/libsmokey@[email protected]		\
>  	../../lib/copperplate/libcopperplate@[email protected]	\
> +	../../lib/alchemy/libalchemy@[email protected] \
>  	@XENO_CORE_LDADD@			\
>  	 @XENO_USER_LDADD@			\
>  	-lpthread -lrt
> diff --git a/testsuite/smokey/spurious-suspend-wake/Makefile.am b/testsuite/smokey/spurious-suspend-wake/Makefile.am
> new file mode 100644
> index 000000000..b2f4f3b6c
> --- /dev/null
> +++ b/testsuite/smokey/spurious-suspend-wake/Makefile.am
> @@ -0,0 +1,10 @@
> +noinst_LIBRARIES = libspurious-suspend-wake.a
> +
> +libspurious_suspend_wake_a_SOURCES = spurious-suspend-wake.c
> +
> +CCLD = $(top_srcdir)/scripts/wrap-link.sh $(CC)
> +
> +libspurious_suspend_wake_a_CPPFLAGS =   \
> +        @XENO_USER_CFLAGS@              \
> +        -I$(top_srcdir)/include
> +
> diff --git a/testsuite/smokey/spurious-suspend-wake/spurious-suspend-wake.c b/testsuite/smokey/spurious-suspend-wake/spurious-suspend-wake.c
> new file mode 100644
> index 000000000..969bd5344
> --- /dev/null
> +++ b/testsuite/smokey/spurious-suspend-wake/spurious-suspend-wake.c
> @@ -0,0 +1,238 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <sched.h>
> +#include <sys/types.h>
> +#include <sys/syscall.h>
> +#include <smokey/smokey.h>
> +#include <alchemy/task.h>
> +
> +smokey_test_plugin(spurious_suspend_wake,
> +		   SMOKEY_ARGLIST(
> +			   SMOKEY_INT(iters),
> +			   SMOKEY_INT(settle_us),
> +			   SMOKEY_INT(quiet_us),
> +			   SMOKEY_INT(victim_cpu),
> +			   SMOKEY_INT(suspender_cpu),
> +		   ),
> +		   "Detect SIGSHADOW vs xnthread_suspend race that "
> +		   "spuriously resumes an rt_task_suspend()'d task.");
> +
> +static volatile unsigned long victim_counter;
> +static volatile bool stop;
> +static RT_TASK victim_task;
> +static RT_TASK suspender_task;
> +
> +static int iters		= 5000;
> +static int settle_us		= 100;
> +static int quiet_us		= 200;
> +static int victim_cpu		= 0;
> +static int suspender_cpu	= 1;
> +
> +static unsigned int spurious;
> +static unsigned long max_delta;
> +static int cycles_done;
> +
> +static void victim_body(void *arg)
> +{
> +	(void)arg;
> +
> +	/*
> +	 * Tight relax/harden bouncing:
> +	 *  - syscall(getpid) is a raw Linux syscall, sets XNRELAX on
> +	 *    return, leaving us in secondary mode.
> +	 *  - rt_task_sleep() resolves via libcobalt as a primary-mode
> +	 *    syscall, so the wrapper runs xnthread_harden() before the
> +	 *    sleep proper. On return we are back in primary mode.
> +	 *
> +	 * The window between the two is when the deferred SIGSHADOW
> +	 * HARDEN posted by a concurrent rt_task_suspend() can land on a
> +	 * non-XNRELAX victim and trigger the spurious wake.
> +	 */
> +	while (!stop) {
> +		syscall(__NR_getpid);
> +		rt_task_sleep(1000);	/* 1us, also forces a harden */
> +		__atomic_add_fetch(&victim_counter, 1, __ATOMIC_RELAXED);
> +	}
> +}
> +
> +static void suspender_body(void *arg)
> +{
> +	int i, ret;
> +
> +	(void)arg;
> +
> +	for (i = 0; i < iters && !stop; i++) {
> +		unsigned long c1, c2, delta;
> +
> +		ret = rt_task_suspend(&victim_task);
> +		if (ret) {
> +			smokey_warning("rt_task_suspend at iter %d: %s",
> +				       i, symerror(ret));
> +			break;
> +		}
> +
> +		/*
> +		 * Let SIGSHADOW HARDEN actually fire so the victim
> +		 * either properly blocks on XNSUSP or, if the race
> +		 * fired, has already been spuriously resumed.
> +		 */
> +		rt_task_sleep((RTIME)settle_us * 1000);
> +
> +		c1 = __atomic_load_n(&victim_counter, __ATOMIC_RELAXED);
> +
> +		/* Quiet window: victim must NOT advance. */
> +		rt_task_sleep((RTIME)quiet_us * 1000);
> +
> +		c2 = __atomic_load_n(&victim_counter, __ATOMIC_RELAXED);
> +
> +		ret = rt_task_resume(&victim_task);
> +		if (ret) {
> +			smokey_warning("rt_task_resume at iter %d: %s",
> +				       i, symerror(ret));
> +			break;
> +		}
> +
> +		delta = c2 - c1;
> +		if (delta != 0) {
> +			spurious++;
> +			if (delta > max_delta)
> +				max_delta = delta;
> +			if (spurious <= 5)
> +				smokey_trace("iter %d: victim advanced by "
> +					     "%lu in quiet window", i, delta);
> +		}
> +
> +		/* Let the victim accumulate forward progress between
> +		 * cycles, otherwise we hammer it continuously and can no
> +		 * longer tell forward progress from wake. */
> +		rt_task_sleep((RTIME)quiet_us * 1000);
> +	}
> +
> +	cycles_done = i;
> +}
> +
> +static int run_spurious_suspend_wake(struct smokey_test *t, int argc,
> +				     char *const argv[])
> +{
> +	cpu_set_t set;
> +	int ret, ncpu;
> +
> +	smokey_parse_args(t, argc, argv);
> +	if (SMOKEY_ARG_ISSET(spurious_suspend_wake, iters))
> +		iters = SMOKEY_ARG_INT(spurious_suspend_wake, iters);
> +	if (SMOKEY_ARG_ISSET(spurious_suspend_wake, settle_us))
> +		settle_us = SMOKEY_ARG_INT(spurious_suspend_wake, settle_us);
> +	if (SMOKEY_ARG_ISSET(spurious_suspend_wake, quiet_us))
> +		quiet_us = SMOKEY_ARG_INT(spurious_suspend_wake, quiet_us);
> +	if (SMOKEY_ARG_ISSET(spurious_suspend_wake, victim_cpu))
> +		victim_cpu = SMOKEY_ARG_INT(spurious_suspend_wake, victim_cpu);
> +	if (SMOKEY_ARG_ISSET(spurious_suspend_wake, suspender_cpu))
> +		suspender_cpu = SMOKEY_ARG_INT(spurious_suspend_wake,
> +					       suspender_cpu);

Do we really need all these options? Affinity tuning could rather be
done within the set of CPUs the test is given.

> +
> +	ncpu = sysconf(_SC_NPROCESSORS_ONLN);
> +	if (ncpu < 2) {

See above: You should better check for size of the affinity mask the
test has been given.

> +		smokey_note("need >= 2 online CPUs to drive the race");
> +		return -ENOSYS;

Let's warn and skip but not fail.

> +	}
> +	if (victim_cpu < 0 || suspender_cpu < 0 ||
> +	    victim_cpu >= ncpu || suspender_cpu >= ncpu ||
> +	    victim_cpu == suspender_cpu) {
> +		smokey_warning("victim_cpu and suspender_cpu must be "
> +			       "distinct values in [0, %d)", ncpu);
> +		return -EINVAL;
> +	}
> +	if (iters <= 0 || settle_us <= 0 || quiet_us <= 0) {
> +		smokey_warning("iters, settle_us, quiet_us must be positive");
> +		return -EINVAL;
> +	}
> +
> +	stop = false;
> +	victim_counter = 0;
> +	spurious = 0;
> +	max_delta = 0;
> +	cycles_done = 0;
> +
> +	ret = rt_task_create(&victim_task, "ssw-victim", 0, 50, T_JOINABLE);
> +	if (ret) {
> +		smokey_warning("rt_task_create(victim): %s", symerror(ret));
> +		return ret;
> +	}
> +
> +	CPU_ZERO(&set);
> +	CPU_SET(victim_cpu, &set);
> +	ret = rt_task_set_affinity(&victim_task, &set);
> +	if (ret) {
> +		smokey_warning("rt_task_set_affinity(victim, %d): %s",
> +			       victim_cpu, symerror(ret));
> +		goto fail_after_victim_create;
> +	}
> +
> +	ret = rt_task_create(&suspender_task, "ssw-suspender", 0, 50,
> +			     T_JOINABLE);
> +	if (ret) {
> +		smokey_warning("rt_task_create(suspender): %s", symerror(ret));
> +		goto fail_after_victim_create;
> +	}
> +
> +	CPU_ZERO(&set);
> +	CPU_SET(suspender_cpu, &set);
> +	ret = rt_task_set_affinity(&suspender_task, &set);
> +	if (ret) {
> +		smokey_warning("rt_task_set_affinity(suspender, %d): %s",
> +			       suspender_cpu, symerror(ret));
> +		goto fail_after_suspender_create;
> +	}
> +
> +	ret = rt_task_start(&victim_task, victim_body, NULL);
> +	if (ret) {
> +		smokey_warning("rt_task_start(victim): %s", symerror(ret));
> +		goto fail_after_suspender_create;
> +	}
> +
> +	smokey_trace("victim on CPU%d, suspender on CPU%d, %d iters, "
> +		     "settle=%dus, quiet=%dus",
> +		     victim_cpu, suspender_cpu, iters, settle_us, quiet_us);
> +
> +	ret = rt_task_start(&suspender_task, suspender_body, NULL);
> +	if (ret) {
> +		smokey_warning("rt_task_start(suspender): %s", symerror(ret));
> +		stop = true;
> +		rt_task_join(&victim_task);
> +		goto fail_after_suspender_create;
> +	}
> +
> +	rt_task_join(&suspender_task);
> +
> +	stop = true;
> +	/*
> +	 * Belt and braces: the suspender may have left the victim
> +	 * suspended if it broke out early. rt_task_resume is a no-op on
> +	 * a non-suspended task, so this is always safe.
> +	 */
> +	rt_task_resume(&victim_task);
> +	rt_task_join(&victim_task);
> +
> +	rt_task_delete(&suspender_task);
> +	rt_task_delete(&victim_task);
> +
> +	if (spurious) {
> +		smokey_warning("FAIL: %u/%d cycles saw the victim advance "
> +			       "during the quiet window (max delta %lu)",
> +			       spurious, cycles_done, max_delta);
> +		return -EPROTO;
> +	}
> +
> +	smokey_trace("no spurious wakes in %d cycles", cycles_done);
> +	return 0;
> +
> +fail_after_suspender_create:
> +	rt_task_delete(&suspender_task);
> +fail_after_victim_create:
> +	rt_task_delete(&victim_task);
> +	return ret;
> +}

Would love to add it to our testsuite, but maybe rather as part of the
alchemy tests.

Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center