Skip to content

Commit 6b03d13

Browse files
committed
proc: Ensure we see the exit of each process tid exactly once
When the thread group leader changes during exec and the old leaders thread is reaped proc_flush_pid will flush the dentries for the entire process because the leader still has it's original pid. Fix this by exchanging the pids in an rcu safe manner, and wrapping the code to do that up in a helper exchange_tids. When I removed switch_exec_pids and introduced this behavior in d73d652 ("[PATCH] pidhash: kill switch_exec_pids") there really was nothing that cared as flushing happened with the cached dentry and de_thread flushed both of them on exec. This lack of fully exchanging pids became a problem a few months later when I introduced 48e6484 ("[PATCH] proc: Rewrite the proc dentry flush on exit optimization"). Which overlooked the de_thread case was no longer swapping pids, and I was looking up proc dentries by task->pid. The current behavior isn't properly a bug as everything in proc will continue to work correctly just a little bit less efficiently. Fix this just so there are no little surprise corner cases waiting to bite people. -- Oleg points out this could be an issue in next_tgid in proc where has_group_leader_pid is called, and reording some of the assignments should fix that. -- Oleg points out this will break the 10 year old hack in __exit_signal.c > /* > * This can only happen if the caller is de_thread(). > * FIXME: this is the temporary hack, we should teach > * posix-cpu-timers to handle this case correctly. > */ > if (unlikely(has_group_leader_pid(tsk))) > posix_cpu_timers_exit_group(tsk); The code in next_tgid has been changed to use PIDTYPE_TGID, and the posix cpu timers code has been fixed so it does not need the 10 year old hack, so this should be safe to merge now. Link: https://lore.kernel.org/lkml/[email protected]/ Acked-by: Linus Torvalds <[email protected]> Acked-by: Oleg Nesterov <[email protected]> Fixes: 48e6484 ("[PATCH] proc: Rewrite the proc dentry flush on exit optimization"). Signed-off-by: Eric W. Biederman <[email protected]>
1 parent 35fc0e3 commit 6b03d13

File tree

3 files changed

+21
-4
lines changed

3 files changed

+21
-4
lines changed

fs/exec.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,11 +1186,8 @@ static int de_thread(struct task_struct *tsk)
11861186

11871187
/* Become a process group leader with the old leader's pid.
11881188
* The old leader becomes a thread of the this thread group.
1189-
* Note: The old leader also uses this pid until release_task
1190-
* is called. Odd but simple and correct.
11911189
*/
1192-
tsk->pid = leader->pid;
1193-
change_pid(tsk, PIDTYPE_PID, task_pid(leader));
1190+
exchange_tids(tsk, leader);
11941191
transfer_pid(leader, tsk, PIDTYPE_TGID);
11951192
transfer_pid(leader, tsk, PIDTYPE_PGID);
11961193
transfer_pid(leader, tsk, PIDTYPE_SID);

include/linux/pid.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ extern void attach_pid(struct task_struct *task, enum pid_type);
102102
extern void detach_pid(struct task_struct *task, enum pid_type);
103103
extern void change_pid(struct task_struct *task, enum pid_type,
104104
struct pid *pid);
105+
extern void exchange_tids(struct task_struct *task, struct task_struct *old);
105106
extern void transfer_pid(struct task_struct *old, struct task_struct *new,
106107
enum pid_type);
107108

kernel/pid.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,25 @@ void change_pid(struct task_struct *task, enum pid_type type,
363363
attach_pid(task, type);
364364
}
365365

366+
void exchange_tids(struct task_struct *left, struct task_struct *right)
367+
{
368+
struct pid *pid1 = left->thread_pid;
369+
struct pid *pid2 = right->thread_pid;
370+
struct hlist_head *head1 = &pid1->tasks[PIDTYPE_PID];
371+
struct hlist_head *head2 = &pid2->tasks[PIDTYPE_PID];
372+
373+
/* Swap the single entry tid lists */
374+
hlists_swap_heads_rcu(head1, head2);
375+
376+
/* Swap the per task_struct pid */
377+
rcu_assign_pointer(left->thread_pid, pid2);
378+
rcu_assign_pointer(right->thread_pid, pid1);
379+
380+
/* Swap the cached value */
381+
WRITE_ONCE(left->pid, pid_nr(pid2));
382+
WRITE_ONCE(right->pid, pid_nr(pid1));
383+
}
384+
366385
/* transfer_pid is an optimization of attach_pid(new), detach_pid(old) */
367386
void transfer_pid(struct task_struct *old, struct task_struct *new,
368387
enum pid_type type)

0 commit comments

Comments
 (0)