From adf4e04ff70cc17d3951049c433bbc871fa6710a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20V=C3=A1zquez=20Blanco?= Date: Fri, 21 Aug 2026 18:47:21 +0200 Subject: [PATCH] thread: Hold a task_struct reference across kthread_stop() ipts_thread_runner() signals thread->done and then returns, so the kthread exits and gets reaped as soon as the thread function is done. ipts_thread_stop() waits for that completion and only then calls kthread_stop(), by which point nothing holds a reference to the task_struct any more. If the stopping task is preempted in that window, kthread_stop() operates on freed memory. kthread_stop()'s documentation states the requirement: If threadfn() may call kthread_exit() itself, the caller must ensure task_struct can't go away. which is always the case here, since the thread function returns on its own rather than waiting for kthread_should_stop(). Waiting for the completion first is what makes the wrapper safe with respect to MEI commands issued during shutdown, but it also guarantees the thread has already exited, so the wrapper hits this race by design rather than avoiding it. Observed on a Surface Pro 4 (EDS v1) while iptsd switched the digitizer into multitouch mode, which calls ipts_control_restart(): refcount_t: addition on 0; use-after-free. WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x6a/0x90, CPU#3: iptsd/395 kthread_stop+0x18b/0x190 ipts_thread_stop+0x32/0x60 [ipts] ipts_receiver_stop+0x22/0x40 [ipts] _ipts_control_stop.part.0+0x29/0x50 [ipts] ipts_control_restart+0x21/0x40 [ipts] ipts_eds1_raw_request+0x49/0x90 [ipts] refcount_t: underflow; use-after-free. WARNING: kernel/fork.c:784 at __put_task_struct+0x129/0x1a0, CPU#2: libinput-device/406 Split kthread_run() into kthread_create() + wake_up_process() so that a reference can be taken while the thread is still guaranteed to exist, and release it with kthread_stop_put(). While at it, stop leaving an error pointer in thread->thread when thread creation fails. ipts_thread_stop() only tests for NULL, so it would hand that error pointer to kthread_stop(). --- src/thread.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/thread.c b/src/thread.c index e6dc919..c54c5df 100644 --- a/src/thread.c +++ b/src/thread.c @@ -9,6 +9,7 @@ #include #include #include +#include #include "thread.h" @@ -19,7 +20,9 @@ * will get the actual thread function from there and call it. * * Once the actual thread function has exited, this function will trigger the completion object - * so that the thread can be cleaned up using kthread_stop(). + * so that the thread can be cleaned up using kthread_stop(). Because the thread function + * returns on its own, the task_struct is only guaranteed to still be there because + * ipts_thread_start() holds a reference to it. */ static int ipts_thread_runner(void *data) { @@ -40,14 +43,36 @@ bool ipts_thread_should_stop(struct ipts_thread *thread) int ipts_thread_start(struct ipts_thread *thread, int (*threadfn)(struct ipts_thread *thread), void *data, const char *name) { + struct task_struct *task = NULL; + init_completion(&thread->done); thread->data = data; thread->should_stop = false; thread->threadfn = threadfn; - thread->thread = kthread_run(ipts_thread_runner, thread, name); - return PTR_ERR_OR_ZERO(thread->thread); + task = kthread_create(ipts_thread_runner, thread, name); + if (IS_ERR(task)) { + /* + * Do not leave an error pointer behind: ipts_thread_stop() only checks for + * NULL, and would pass it on to kthread_stop(). + */ + thread->thread = NULL; + return PTR_ERR(task); + } + + /* + * ipts_thread_runner() completes &thread->done and then returns, so the kthread can + * exit and have its task_struct freed before ipts_thread_stop() gets to call + * kthread_stop() on it. Hold a reference so that the task stays valid until then, as + * kthread_stop() requires of a thread function that may exit on its own. + */ + get_task_struct(task); + thread->thread = task; + + wake_up_process(task); + + return 0; } int ipts_thread_stop(struct ipts_thread *thread) @@ -65,7 +90,7 @@ int ipts_thread_stop(struct ipts_thread *thread) wmb(); wait_for_completion(&thread->done); - ret = kthread_stop(thread->thread); + ret = kthread_stop_put(thread->thread); thread->thread = NULL; thread->data = NULL;