* [PATCH 1/1] add pid::tasks_lock to fix send_sigio() and send_sigurg()
2026-10-17 15:49 [PATCH 0/1] add pid::tasks_lock to fix send_sigio() and send_sigurg() Oleg Nesterov
@ 2026-10-17 15:49 ` Oleg Nesterov
2026-10-17 16:10 ` [PATCH 0/1] " Oleg Nesterov
1 sibling, 0 replies; 3+ messages in thread
From: Oleg Nesterov @ 2026-10-17 15:49 UTC (permalink / raw)
To: Christian Brauner, David Laight, Jeff Layton, Mingyu Wang
Cc: alex.aring, jack, viro, linux-fsdevel, linux-kernel
send_sigio() and send_sigurg() assume that do_each_pid_task() is
rcu-safe but it is not. A lockless user can:
- race with a change_pid() that moves the task to another hlist,
and continue to scan the wrong list.
- race with a copy_process() that adds a new task to the start of hlist,
and miss the new child.
As for send_sigio/sigurg in particular, they can also race with de_thread(),
this needs another note.
Add the new per pid lock to protect pid->tasks hlist. Note that detach_pid()
doesn't take this lock, so do_each_pid_task() without tasklist_lock needs
rcu_read_lock().
The "type <= PIDTYPE_TGID" case is wrong too, but this needs another patch.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
fs/fcntl.c | 4 ++++
include/linux/pid.h | 1 +
kernel/pid.c | 24 +++++++++++++++++++-----
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index c158f082f1da..74c8a96e64b2 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -930,9 +930,11 @@ void send_sigio(struct fown_struct *fown, int fd, int band)
rcu_read_unlock();
} else {
rcu_read_lock();
+ spin_lock(&pid->tasks_lock);
do_each_pid_task(pid, type, p) {
send_sigio_to_task(p, fown, fd, band, type);
} while_each_pid_task(pid, type, p);
+ spin_unlock(&pid->tasks_lock);
rcu_read_unlock();
}
out_unlock_fown:
@@ -976,9 +978,11 @@ int send_sigurg(struct file *file)
rcu_read_unlock();
} else {
rcu_read_lock();
+ spin_lock(&pid->tasks_lock);
do_each_pid_task(pid, type, p) {
send_sigurg_to_task(p, fown, type);
} while_each_pid_task(pid, type, p);
+ spin_unlock(&pid->tasks_lock);
rcu_read_unlock();
}
out_unlock_fown:
diff --git a/include/linux/pid.h b/include/linux/pid.h
index f5476d7140a3..ca5177fdae81 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -59,6 +59,7 @@ struct pid {
refcount_t count;
unsigned int level;
spinlock_t lock;
+ spinlock_t tasks_lock;
struct {
u64 ino;
struct rhash_head pidfs_hash;
diff --git a/kernel/pid.c b/kernel/pid.c
index 95b8ccfa8269..41fe55fefc3c 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -45,9 +45,11 @@
#include <linux/pidfs.h>
#include <net/sock.h>
#include <uapi/linux/pidfd.h>
+#include "sched/sched.h" /* double_lock() */
struct pid init_struct_pid = {
.count = REFCOUNT_INIT(1),
+ .tasks_lock = __SPIN_LOCK_UNLOCKED(init_struct_pid.tasks_lock),
.tasks = {
{ .first = NULL },
{ .first = NULL },
@@ -193,6 +195,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *arg_set_tid,
pid->level = ns->level;
refcount_set(&pid->count, 1);
spin_lock_init(&pid->lock);
+ spin_lock_init(&pid->tasks_lock);
for (type = 0; type < PIDTYPE_MAX; ++type)
INIT_HLIST_HEAD(&pid->tasks[type]);
init_waitqueue_head(&pid->wait_pidfd);
@@ -396,7 +399,8 @@ void attach_pid(struct task_struct *task, enum pid_type type)
lockdep_assert_held_write(&tasklist_lock);
pid = *task_pid_ptr(task, type);
- hlist_add_head_rcu(&task->pid_links[type], &pid->tasks[type]);
+ scoped_guard(spinlock, &pid->tasks_lock)
+ hlist_add_head_rcu(&task->pid_links[type], &pid->tasks[type]);
}
static void __change_pid(struct pid **pids, struct task_struct *task,
@@ -427,10 +431,15 @@ void detach_pid(struct pid **pids, struct task_struct *task, enum pid_type type)
}
void change_pid(struct pid **pids, struct task_struct *task, enum pid_type type,
- struct pid *pid)
+ struct pid *new)
{
- __change_pid(pids, task, type, pid);
- attach_pid(task, type);
+ struct pid *old = task->signal->pids[type];
+
+ double_lock(&old->tasks_lock, &new->tasks_lock);
+ __change_pid(pids, task, type, new);
+ hlist_add_head_rcu(&task->pid_links[type], &new->tasks[type]);
+ spin_unlock(&old->tasks_lock);
+ spin_unlock(&new->tasks_lock);
}
void exchange_tids(struct task_struct *left, struct task_struct *right)
@@ -441,6 +450,7 @@ void exchange_tids(struct task_struct *left, struct task_struct *right)
struct hlist_head *head2 = &pid2->tasks[PIDTYPE_PID];
lockdep_assert_held_write(&tasklist_lock);
+ double_lock(&pid1->tasks_lock, &pid2->tasks_lock);
/* Swap the single entry tid lists */
hlists_swap_heads_rcu(head1, head2);
@@ -452,6 +462,9 @@ void exchange_tids(struct task_struct *left, struct task_struct *right)
/* Swap the cached value */
WRITE_ONCE(left->pid, pid_nr(pid2));
WRITE_ONCE(right->pid, pid_nr(pid1));
+
+ spin_unlock(&pid1->tasks_lock);
+ spin_unlock(&pid2->tasks_lock);
}
/* transfer_pid is an optimization of attach_pid(new), detach_pid(old) */
@@ -460,7 +473,8 @@ void transfer_pid(struct task_struct *old, struct task_struct *new,
{
WARN_ON_ONCE(type == PIDTYPE_PID);
lockdep_assert_held_write(&tasklist_lock);
- hlist_replace_rcu(&old->pid_links[type], &new->pid_links[type]);
+ scoped_guard(spinlock, &old->signal->pids[type]->tasks_lock)
+ hlist_replace_rcu(&old->pid_links[type], &new->pid_links[type]);
}
struct task_struct *pid_task(struct pid *pid, enum pid_type type)
--
2.55.0
^ permalink raw reply related [flat|nested] 3+ messages in thread