[PATCH] workqueue: Put the pwq after detaching the rescuer from the pool

Lai Jiangshan posted 1 patch 11 months ago
kernel/workqueue.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
[PATCH] workqueue: Put the pwq after detaching the rescuer from the pool
Posted by Lai Jiangshan 11 months ago
From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

The commit 68f83057b913("workqueue: Reap workers via kthread_stop() and
remove detach_completion") adds code to reap the normal workers but
mistakenly does not handle the rescuer and also removes the code waiting
for the rescuer in put_unbound_pool(), which caused a use-after-free bug
reported by Cheung Wall.

To avoid the use-after-free bug, the pool’s reference must be held until
the detachment is complete. Therefore, move the code that puts the pwq
after detaching the rescuer from the pool.

Reported-by: cheung wall <zzqq0103.hey@gmail.com>
Cc: cheung wall <zzqq0103.hey@gmail.com>
Link: https://lore.kernel.org/lkml/CAKHoSAvP3iQW+GwmKzWjEAOoPvzeWeoMO0Gz7Pp3_4kxt-RMoA@mail.gmail.com/
Fixes: 68f83057b913("workqueue: Reap workers via kthread_stop() and remove detach_completion")
Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 kernel/workqueue.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 33a23c7b2274..ccad33001c58 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3516,12 +3516,6 @@ static int rescuer_thread(void *__rescuer)
 			}
 		}
 
-		/*
-		 * Put the reference grabbed by send_mayday().  @pool won't
-		 * go away while we're still attached to it.
-		 */
-		put_pwq(pwq);
-
 		/*
 		 * Leave this pool. Notify regular workers; otherwise, we end up
 		 * with 0 concurrency and stalling the execution.
@@ -3532,6 +3526,12 @@ static int rescuer_thread(void *__rescuer)
 
 		worker_detach_from_pool(rescuer);
 
+		/*
+		 * Put the reference grabbed by send_mayday().  @pool might
+		 * go away any time after it.
+		 */
+		put_pwq_unlocked(pwq);
+
 		raw_spin_lock_irq(&wq_mayday_lock);
 	}
 
-- 
2.19.1.6.gb485710b

Re: [PATCH] workqueue: Put the pwq after detaching the rescuer from the pool
Posted by Tejun Heo 10 months, 4 weeks ago
On Thu, Jan 23, 2025 at 04:25:35PM +0800, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> 
> The commit 68f83057b913("workqueue: Reap workers via kthread_stop() and
> remove detach_completion") adds code to reap the normal workers but
> mistakenly does not handle the rescuer and also removes the code waiting
> for the rescuer in put_unbound_pool(), which caused a use-after-free bug
> reported by Cheung Wall.
> 
> To avoid the use-after-free bug, the pool’s reference must be held until
> the detachment is complete. Therefore, move the code that puts the pwq
> after detaching the rescuer from the pool.
> 
> Reported-by: cheung wall <zzqq0103.hey@gmail.com>
> Cc: cheung wall <zzqq0103.hey@gmail.com>
> Link: https://lore.kernel.org/lkml/CAKHoSAvP3iQW+GwmKzWjEAOoPvzeWeoMO0Gz7Pp3_4kxt-RMoA@mail.gmail.com/
> Fixes: 68f83057b913("workqueue: Reap workers via kthread_stop() and remove detach_completion")
> Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>

Applied to wq/for-6.14-fixes.

Thanks.

-- 
tejun