Re: tty: timbuart: tasklet may trigger UAF during remove

Re: tty: timbuart: tasklet may trigger UAF during remove

[Greg KH]

On Mon, Oct 19, 2026 at 07:09:12PM +0800, LI xiao wrote:
> Hello,
> I suspect a tasklet lifetime race in the timbuart driver.

<snip>

Great!  Can you please provide a working patch for this issue as you
have a reproducer for it?

Also, html emails are rejected by the mailing lists, as you have seen.
Please read the kernel documentation for how to fix up your email client
to work properly.

thanks,

greg k-h

[PATCH] tty: timbuart: kill tasklet after freeing IRQ

[LI xiao]

The timbuart driver calls tasklet_kill() before uart_remove_one_port().
An interrupt can schedule the tasklet again after tasklet_kill()
returns but before timbuart_shutdown() frees the IRQ. The subsequent
free_irq() synchronizes the IRQ handler, but does not drain a tasklet
that the handler has already queued.

Move tasklet_kill() to timbuart_shutdown(), immediately after
free_irq(), so that any tasklet queued by the IRQ handler is drained
before uart_remove_one_port() releases the port resources.

I tested the intended synchronization ordering with a KASAN-enabled
kernel in a QEMU guest using a test module. The module queued
the tasklet, waited for the simulated IRQ handler to exit, called
tasklet_kill(), and then freed the UART object. The tasklet completed
without a KASAN report or scheduling warning.

This was a controlled test of the synchronization ordering. It did
not exercise timbuart's actual IRQ handler or Timberdale UART hardware,
which is not modeled by QEMU.


Signed-off-by: LI Xiao <lixiao0@hdu.edu.cn>

---
 drivers/tty/serial/timbuart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/timbuart.c b/drivers/tty/serial/timbuart.c
index 7a0326e71..c561ca702 100644
--- a/drivers/tty/serial/timbuart.c
+++ b/drivers/tty/serial/timbuart.c
@@ -254,6 +254,7 @@ static void timbuart_shutdown(struct uart_port *port)
 		container_of(port, struct timbuart_port, port);
 	dev_dbg(port->dev, "%s\n", __func__);
 	free_irq(port->irq, uart);
+	tasklet_kill(&uart->tasklet);
 	iowrite32(0, port->membase + TIMBUART_IER);
 
 	timbuart_flush_buffer(port);
@@ -474,7 +475,6 @@ static void timbuart_remove(struct platform_device *dev)
 {
 	struct timbuart_port *uart = platform_get_drvdata(dev);
 
-	tasklet_kill(&uart->tasklet);
 	uart_remove_one_port(&timbuart_driver, &uart->port);
 	uart_unregister_driver(&timbuart_driver);
 	kfree(uart);
-- 
2.34.1





From: Greg KH <gregkh@linuxfoundation.org>
Date: 2026-10-19 19:46:40
To:  LI xiao <lixiao0@hdu.edu.cn>
Cc:  jirislaby@kernel.org,linux-kernel@vger.kernel.org,linux-serial@vger.kernel.org
Subject: Re: tty: timbuart: tasklet may trigger UAF during remove>On Mon, Oct 19, 2026 at 07:09:12PM +0800, LI xiao wrote:
>> Hello,
>> I suspect a tasklet lifetime race in the timbuart driver.
>
><snip>
>
>Great!  Can you please provide a working patch for this issue as you
>have a reproducer for it?
>
>Also, html emails are rejected by the mailing lists, as you have seen.
>Please read the kernel documentation for how to fix up your email client
>to work properly.
>
>thanks,
>
>greg k-h
>

Re: [PATCH] tty: timbuart: kill tasklet after freeing IRQ

[Greg KH]

On Mon, Oct 19, 2026 at 09:31:55PM +0800, LI xiao wrote:
> The timbuart driver calls tasklet_kill() before uart_remove_one_port().
> An interrupt can schedule the tasklet again after tasklet_kill()
> returns but before timbuart_shutdown() frees the IRQ. The subsequent
> free_irq() synchronizes the IRQ handler, but does not drain a tasklet
> that the handler has already queued.
> 
> Move tasklet_kill() to timbuart_shutdown(), immediately after
> free_irq(), so that any tasklet queued by the IRQ handler is drained
> before uart_remove_one_port() releases the port resources.
> 
> I tested the intended synchronization ordering with a KASAN-enabled
> kernel in a QEMU guest using a test module. The module queued
> the tasklet, waited for the simulated IRQ handler to exit, called
> tasklet_kill(), and then freed the UART object. The tasklet completed
> without a KASAN report or scheduling warning.
> 
> This was a controlled test of the synchronization ordering. It did
> not exercise timbuart's actual IRQ handler or Timberdale UART hardware,
> which is not modeled by QEMU.

Ah, that might be tricky, I think you need to test this on real hardware
as you are changing the logic here.

> Signed-off-by: LI Xiao <lixiao0@hdu.edu.cn>

No fixes: tag?  Cc: stable?


> 
> ---
>  drivers/tty/serial/timbuart.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/timbuart.c b/drivers/tty/serial/timbuart.c
> index 7a0326e71..c561ca702 100644
> --- a/drivers/tty/serial/timbuart.c
> +++ b/drivers/tty/serial/timbuart.c
> @@ -254,6 +254,7 @@ static void timbuart_shutdown(struct uart_port *port)
>  		container_of(port, struct timbuart_port, port);
>  	dev_dbg(port->dev, "%s\n", __func__);
>  	free_irq(port->irq, uart);
> +	tasklet_kill(&uart->tasklet);
>  	iowrite32(0, port->membase + TIMBUART_IER);
>  
>  	timbuart_flush_buffer(port);

Are you sure you can flush after the tasklet is killed?  Nothing
requires it to still be running at this point in time?

And why not stop it before the irq is freed as that's what it is
handling?  Note, I could be totally wrong, I'm just guessing here...

thanks,

greg k-h