Re: [PATCH v2 1/5] media: ivtv: convert to kthread_run_worker

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

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:
- [High] kthread_queue_work() receives a double pointer instead of a pointer to kthread_worker, leading to a build error or runtime memory corruption.

Aware (sorta?)

- [High] kthread_destroy_worker() is called before free_irq(), creating a Use-After-Free race condition with the interrupt handler.

Oh!

--

commit a504d2685cd2f4c4e6b93a81c847d55367eae1c0
Author: Bradley Morgan [off-list ref]

media: ivtv: convert to kthread_run_worker

Convert ivtv 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/media/pci/ivtv/ivtv-driver.c

b/drivers/media/pci/ivtv/ivtv-driver.c

quoted

index b9ea56ec9593f..6f0e0e46deb5b 100644

--- a/drivers/media/pci/ivtv/ivtv-driver.c
+++ b/drivers/media/pci/ivtv/ivtv-driver.c

[ ... ]

quoted

@@ -1372,8 +1370,8 @@ static void ivtv_remove(struct pci_dev *pdev)
 	timer_shutdown_sync(&itv->dma_timer);
 
 	/* Kill irq worker */
-	kthread_flush_worker(&itv->irq_worker);
-	kthread_stop(itv->irq_worker_task);
+	kthread_flush_worker(itv->irq_worker);
+	kthread_destroy_worker(itv->irq_worker);

[Severity: High]
Does this create a use-after-free race condition with the interrupt
handler?

kthread_destroy_worker() frees the dynamically allocated worker structure
here,
but free_irq() is called later in ivtv_remove():

drivers/media/pci/ivtv/ivtv-driver.c:ivtv_remove() {
...
kthread_destroy_worker(itv->irq_worker);
...
free_irq(itv->pdev->irq, (void *)itv);
...
}

Because free_irq() is the synchronization point that guarantees no
interrupt
handlers are still running, an in-flight ivtv_irq_handler() on another CPU
can survive past kthread_destroy_worker().

If the handler subsequently attempts to queue work, it will dereference
the
already-freed irq_worker pointer, causing a kernel panic.

quoted

 
 	ivtv_streams_cleanup(itv);
 	ivtv_udma_free(itv);

diff --git a/drivers/media/pci/ivtv/ivtv-driver.h

b/drivers/media/pci/ivtv/ivtv-driver.h

quoted

index f1f18911332e8..41789a50d1ca6 100644

--- a/drivers/media/pci/ivtv/ivtv-driver.h
+++ b/drivers/media/pci/ivtv/ivtv-driver.h

@@ -668,8 +668,7 @@ struct ivtv {
 	/* Interrupts & DMA */
 	u32 irqmask;                    /* active interrupts */
 	u32 irq_rr_idx;                 /* round-robin stream index */
-	struct kthread_worker irq_worker;		/* kthread worker for PIO/YUV/VBI actions */
-	struct task_struct *irq_worker_task;		/* task for irq_worker */
+	struct kthread_worker *irq_worker;		/* kthread worker for PIO/YUV/VBI actions */

[Severity: High]
Does this change from a struct to a pointer cause a build error or memory
corruption in ivtv_irq_handler()?

Because irq_worker is now a pointer, the unmodified call site in
ivtv_irq_handler() now passes a double pointer:

drivers/media/pci/ivtv/ivtv-irq.c:ivtv_irq_handler() {
   ...
   kthread_queue_work(&itv->irq_worker, &itv->irq_work);
   ...
}

This violates the API contract and will cause a build failure with modern
compilers due to incompatible pointer types.

If the warning is ignored or coerced, kthread_queue_work() will treat the
offset within struct ivtv as a kthread_worker object, corrupting adjacent
fields when it acquires the spinlock, leading to a kernel crash.

--- Thanks!

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