Skip to content

Commit ca7752c

Browse files
prattmicKAGA-KOKO
authored andcommitted
posix-cpu-timers: Clear task::posix_cputimers_work in copy_process()
copy_process currently copies task_struct.posix_cputimers_work as-is. If a timer interrupt arrives while handling clone and before dup_task_struct completes then the child task will have: 1. posix_cputimers_work.scheduled = true 2. posix_cputimers_work.work queued. copy_process clears task_struct.task_works, so (2) will have no effect and posix_cpu_timers_work will never run (not to mention it doesn't make sense for two tasks to share a common linked list). Since posix_cpu_timers_work never runs, posix_cputimers_work.scheduled is never cleared. Since scheduled is set, future timer interrupts will skip scheduling work, with the ultimate result that the task will never receive timer expirations. Together, the complete flow is: 1. Task 1 calls clone(), enters kernel. 2. Timer interrupt fires, schedules task work on Task 1. 2a. task_struct.posix_cputimers_work.scheduled = true 2b. task_struct.posix_cputimers_work.work added to task_struct.task_works. 3. dup_task_struct() copies Task 1 to Task 2. 4. copy_process() clears task_struct.task_works for Task 2. 5. Future timer interrupts on Task 2 see task_struct.posix_cputimers_work.scheduled = true and skip scheduling work. Fix this by explicitly clearing contents of task_struct.posix_cputimers_work in copy_process(). This was never meant to be shared or inherited across tasks in the first place. Fixes: 1fb497d ("posix-cpu-timers: Provide mechanisms to defer timer handling to task_work") Reported-by: Rhys Hiltner <[email protected]> Signed-off-by: Michael Pratt <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Cc: <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 879dbe9 commit ca7752c

File tree

3 files changed

+20
-2
lines changed

3 files changed

+20
-2
lines changed

include/linux/posix-timers.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,10 @@ static inline void posix_cputimers_group_init(struct posix_cputimers *pct,
184184
#endif
185185

186186
#ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK
187+
void clear_posix_cputimers_work(struct task_struct *p);
187188
void posix_cputimers_init_work(void);
188189
#else
190+
static inline void clear_posix_cputimers_work(struct task_struct *p) { }
189191
static inline void posix_cputimers_init_work(void) { }
190192
#endif
191193

kernel/fork.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2279,6 +2279,7 @@ static __latent_entropy struct task_struct *copy_process(
22792279
p->pdeath_signal = 0;
22802280
INIT_LIST_HEAD(&p->thread_group);
22812281
p->task_works = NULL;
2282+
clear_posix_cputimers_work(p);
22822283

22832284
#ifdef CONFIG_KRETPROBES
22842285
p->kretprobe_instances.first = NULL;

kernel/time/posix-cpu-timers.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1158,14 +1158,29 @@ static void posix_cpu_timers_work(struct callback_head *work)
11581158
handle_posix_cpu_timers(current);
11591159
}
11601160

1161+
/*
1162+
* Clear existing posix CPU timers task work.
1163+
*/
1164+
void clear_posix_cputimers_work(struct task_struct *p)
1165+
{
1166+
/*
1167+
* A copied work entry from the old task is not meaningful, clear it.
1168+
* N.B. init_task_work will not do this.
1169+
*/
1170+
memset(&p->posix_cputimers_work.work, 0,
1171+
sizeof(p->posix_cputimers_work.work));
1172+
init_task_work(&p->posix_cputimers_work.work,
1173+
posix_cpu_timers_work);
1174+
p->posix_cputimers_work.scheduled = false;
1175+
}
1176+
11611177
/*
11621178
* Initialize posix CPU timers task work in init task. Out of line to
11631179
* keep the callback static and to avoid header recursion hell.
11641180
*/
11651181
void __init posix_cputimers_init_work(void)
11661182
{
1167-
init_task_work(&current->posix_cputimers_work.work,
1168-
posix_cpu_timers_work);
1183+
clear_posix_cputimers_work(current);
11691184
}
11701185

11711186
/*

0 commit comments

Comments
 (0)