Skip to content

Commit 28aaa9c

Browse files
committed
kthread: consolidate kthread exit paths to prevent use-after-free
Guillaume reported crashes via corrupted RCU callback function pointers during KUnit testing. The crash was traced back to the pidfs rhashtable conversion which replaced the 24-byte rb_node with an 8-byte rhash_head in struct pid, shrinking it from 160 to 144 bytes. struct kthread (without CONFIG_BLK_CGROUP) is also 144 bytes. With CONFIG_SLAB_MERGE_DEFAULT and SLAB_HWCACHE_ALIGN both round up to 192 bytes and share the same slab cache. struct pid.rcu.func and struct kthread.affinity_node both sit at offset 0x78. When a kthread exits via make_task_dead() it bypasses kthread_exit() and misses the affinity_node cleanup. free_kthread_struct() frees the memory while the node is still linked into the global kthread_affinity_list. A subsequent list_del() by another kthread writes through dangling list pointers into the freed and reused memory, corrupting the pid's rcu.func pointer. Instead of patching free_kthread_struct() to handle the missed cleanup, consolidate all kthread exit paths. Turn kthread_exit() into a macro that calls do_exit() and add kthread_do_exit() which is called from do_exit() for any task with PF_KTHREAD set. This guarantees that kthread-specific cleanup always happens regardless of the exit path - make_task_dead(), direct do_exit(), or kthread_exit(). Replace __to_kthread() with a new tsk_is_kthread() accessor in the public header. Export do_exit() since module code using the kthread_exit() macro now needs it directly. Reported-by: Guillaume Tucker <[email protected]> Tested-by: Guillaume Tucker <[email protected]> Tested-by: Mark Brown <[email protected]> Tested-by: David Gow <[email protected]> Cc: <[email protected]> Link: https://lore.kernel.org/all/20260224-mittlerweile-besessen-2738831ae7f6@brauner Co-developed-by: Linus Torvalds <[email protected]> Fixes: 4d13f43 ("kthread: Implement preferred affinity") Signed-off-by: Linus Torvalds <[email protected]> Signed-off-by: Christian Brauner <[email protected]>
1 parent cd3c877 commit 28aaa9c

3 files changed

Lines changed: 31 additions & 37 deletions

File tree

include/linux/kthread.h

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,24 @@
77

88
struct mm_struct;
99

10+
/* opaque kthread data */
11+
struct kthread;
12+
13+
/*
14+
* When "(p->flags & PF_KTHREAD)" is set the task is a kthread and will
15+
* always remain a kthread. For kthreads p->worker_private always
16+
* points to a struct kthread. For tasks that are not kthreads
17+
* p->worker_private is used to point to other things.
18+
*
19+
* Return NULL for any task that is not a kthread.
20+
*/
21+
static inline struct kthread *tsk_is_kthread(struct task_struct *p)
22+
{
23+
if (p->flags & PF_KTHREAD)
24+
return p->worker_private;
25+
return NULL;
26+
}
27+
1028
__printf(4, 5)
1129
struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
1230
void *data,
@@ -98,9 +116,10 @@ void *kthread_probe_data(struct task_struct *k);
98116
int kthread_park(struct task_struct *k);
99117
void kthread_unpark(struct task_struct *k);
100118
void kthread_parkme(void);
101-
void kthread_exit(long result) __noreturn;
119+
#define kthread_exit(result) do_exit(result)
102120
void kthread_complete_and_exit(struct completion *, long) __noreturn;
103121
int kthreads_update_housekeeping(void);
122+
void kthread_do_exit(struct kthread *, long);
104123

105124
int kthreadd(void *unused);
106125
extern struct task_struct *kthreadd_task;

kernel/exit.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -896,11 +896,16 @@ static void synchronize_group_exit(struct task_struct *tsk, long code)
896896
void __noreturn do_exit(long code)
897897
{
898898
struct task_struct *tsk = current;
899+
struct kthread *kthread;
899900
int group_dead;
900901

901902
WARN_ON(irqs_disabled());
902903
WARN_ON(tsk->plug);
903904

905+
kthread = tsk_is_kthread(tsk);
906+
if (unlikely(kthread))
907+
kthread_do_exit(kthread, code);
908+
904909
kcov_task_exit(tsk);
905910
kmsan_task_exit(tsk);
906911

@@ -1013,6 +1018,7 @@ void __noreturn do_exit(long code)
10131018
lockdep_free_task(tsk);
10141019
do_task_dead();
10151020
}
1021+
EXPORT_SYMBOL(do_exit);
10161022

10171023
void __noreturn make_task_dead(int signr)
10181024
{

kernel/kthread.c

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -85,24 +85,6 @@ static inline struct kthread *to_kthread(struct task_struct *k)
8585
return k->worker_private;
8686
}
8787

88-
/*
89-
* Variant of to_kthread() that doesn't assume @p is a kthread.
90-
*
91-
* When "(p->flags & PF_KTHREAD)" is set the task is a kthread and will
92-
* always remain a kthread. For kthreads p->worker_private always
93-
* points to a struct kthread. For tasks that are not kthreads
94-
* p->worker_private is used to point to other things.
95-
*
96-
* Return NULL for any task that is not a kthread.
97-
*/
98-
static inline struct kthread *__to_kthread(struct task_struct *p)
99-
{
100-
void *kthread = p->worker_private;
101-
if (kthread && !(p->flags & PF_KTHREAD))
102-
kthread = NULL;
103-
return kthread;
104-
}
105-
10688
void get_kthread_comm(char *buf, size_t buf_size, struct task_struct *tsk)
10789
{
10890
struct kthread *kthread = to_kthread(tsk);
@@ -193,7 +175,7 @@ EXPORT_SYMBOL_GPL(kthread_should_park);
193175

194176
bool kthread_should_stop_or_park(void)
195177
{
196-
struct kthread *kthread = __to_kthread(current);
178+
struct kthread *kthread = tsk_is_kthread(current);
197179

198180
if (!kthread)
199181
return false;
@@ -234,7 +216,7 @@ EXPORT_SYMBOL_GPL(kthread_freezable_should_stop);
234216
*/
235217
void *kthread_func(struct task_struct *task)
236218
{
237-
struct kthread *kthread = __to_kthread(task);
219+
struct kthread *kthread = tsk_is_kthread(task);
238220
if (kthread)
239221
return kthread->threadfn;
240222
return NULL;
@@ -266,7 +248,7 @@ EXPORT_SYMBOL_GPL(kthread_data);
266248
*/
267249
void *kthread_probe_data(struct task_struct *task)
268250
{
269-
struct kthread *kthread = __to_kthread(task);
251+
struct kthread *kthread = tsk_is_kthread(task);
270252
void *data = NULL;
271253

272254
if (kthread)
@@ -309,19 +291,8 @@ void kthread_parkme(void)
309291
}
310292
EXPORT_SYMBOL_GPL(kthread_parkme);
311293

312-
/**
313-
* kthread_exit - Cause the current kthread return @result to kthread_stop().
314-
* @result: The integer value to return to kthread_stop().
315-
*
316-
* While kthread_exit can be called directly, it exists so that
317-
* functions which do some additional work in non-modular code such as
318-
* module_put_and_kthread_exit can be implemented.
319-
*
320-
* Does not return.
321-
*/
322-
void __noreturn kthread_exit(long result)
294+
void kthread_do_exit(struct kthread *kthread, long result)
323295
{
324-
struct kthread *kthread = to_kthread(current);
325296
kthread->result = result;
326297
if (!list_empty(&kthread->affinity_node)) {
327298
mutex_lock(&kthread_affinity_lock);
@@ -333,9 +304,7 @@ void __noreturn kthread_exit(long result)
333304
kthread->preferred_affinity = NULL;
334305
}
335306
}
336-
do_exit(0);
337307
}
338-
EXPORT_SYMBOL(kthread_exit);
339308

340309
/**
341310
* kthread_complete_and_exit - Exit the current kthread.
@@ -683,7 +652,7 @@ void kthread_set_per_cpu(struct task_struct *k, int cpu)
683652

684653
bool kthread_is_per_cpu(struct task_struct *p)
685654
{
686-
struct kthread *kthread = __to_kthread(p);
655+
struct kthread *kthread = tsk_is_kthread(p);
687656
if (!kthread)
688657
return false;
689658

0 commit comments

Comments
 (0)