Re: [PATCH v2 3/5] tty: sc16is7xx: convert to kthread_run_worker

On 5 September 2026 10:40:35 BST, sashiko-bot@kernel.org wrote:

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

New issues:
- [High] The explicitly allocated kthread worker is destroyed before the devm-managed threaded IRQ handler is freed, causing a Use-After-Free.

Awesome. Seriously

Pre-existing issues:
- [High] Closing any single port cancels the shared polling timer, starving all other active ports on the same multi-port chip.
--

commit 3bc3e4920cc0fac18e8f989a3296c6b4222d065c
Author: Bradley Morgan [off-list ref]

tty: sc16is7xx: convert to kthread_run_worker
   
Convert sc16is7xx to use kthread_run_worker() instead of the
deprecated kthread_run(kthread_worker_fn) pattern. The new API sets
worker->task before the worker starts.

quoted

diff --git a/drivers/tty/serial/sc16is7xx.c

b/drivers/tty/serial/sc16is7xx.c

quoted

index 7107a0fb1e7bb..2cfc1f2900091 100644

--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c

@@ -1186,7 +1185,7 @@ static void sc16is7xx_shutdown(struct uart_port

*port)

quoted

 	if (s->polling)
 		kthread_cancel_delayed_work_sync(&s->poll_work);
 
-	kthread_flush_worker(&s->kworker);
+	kthread_flush_worker(s->kworker);
 }

[Severity: High]
This is a pre-existing issue, but does closing any single port cancel the
shared polling timer for all ports?

In polling mode, s->poll_work is shared across all ports on the chip. If
one port is closed, sc16is7xx_shutdown() unconditionally cancels
s->poll_work,
which seems to starve any other active ports on the same chip.

quoted

@@ -1705,8 +1702,8 @@ void sc16is7xx_remove(struct device *dev)
 	if (s->polling)
 		kthread_cancel_delayed_work_sync(&s->poll_work);
 
-	kthread_flush_worker(&s->kworker);
-	kthread_stop(s->kworker_task);
+	kthread_flush_worker(s->kworker);
+	kthread_destroy_worker(s->kworker);
 
 	clk_disable_unprepare(s->clk);
 }

[Severity: High]
Can kthread_destroy_worker() lead to a use-after-free if the threaded IRQ
handler fires concurrently?

The driver uses devm_request_threaded_irq() in sc16is7xx_probe() to manage
its
interrupt, meaning the IRQ handler remains active until after
sc16is7xx_remove() returns and devres cleans it up.

If an interrupt fires or an in-flight threaded IRQ executes in parallel
right
after kthread_destroy_worker() frees s->kworker, the IRQ handler could
call
functions like sc16is7xx_ier_clear() which enqueue work:

drivers/tty/serial/sc16is7xx.c:sc16is7xx_ier_clear() {
   ...
   kthread_queue_work(s->kworker, &one->reg_work);
}

This would dereference the freed s->kworker pointer. Previously kworker
was
embedded in the sc16is7xx_port struct, so stopping the task was safe from
a
use-after-free, but now kthread_destroy_worker() frees the dynamically
allocated worker before the IRQ handler is disabled.

--- Thanks!

https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/ (local)