Re: [PATCH] tty: serial: owl: Fix data race in owl_uart_remove

From: Saubhik Mukherjee
Date: Fri Jun 25 2021 - 03:52:22 EST


On 6/24/21 5:59 PM, Johan Hovold wrote:
On Thu, Jun 24, 2021 at 05:37:38PM +0530, Saubhik Mukherjee wrote:
On 6/23/21 12:46 PM, Johan Hovold wrote:
On Wed, Jun 23, 2021 at 11:06:53AM +0530, Saubhik Mukherjee wrote:
On 6/17/21 4:52 PM, Greg KH wrote:
On Thu, Jun 17, 2021 at 04:34:43PM +0530, Saubhik Mukherjee wrote:
Suppose the driver is registered and a UART port is added. Once an
application opens the port, owl_uart_startup is called which registers
the interrupt handler owl_uart_irq.

We could have the following race condition:

When device is removed, owl_uart_remove is called, which calls
uart_remove_one_port, which calls owl_uart_release_port, which writes
NULL to port->membase. At this point parallely, an interrupt could be
handled by owl_uart_irq which reads port->membase.

This is because it is possible to remove device without closing a port.
Thus, we need to check it and call owl_uart_shutdown in owl_uart_remove.

No, this makes no sense at all. The port is deregistered and hung up by
uart_remove_one_port() (and the interrupt line is consequently disabled
by the driver) before it is released so this can never happen.

Thanks for the reply. I am not sure I understand. I could not find any
interrupt disabling in owl_uart_remove. Could you point out where/how is
the interrupt line is disabled before releasing the port?

The interrupt line is disabled by owl_uart_shutdown(), which is called
when uart_remove_one_port() hangs up an open tty. And as I mentioned
this happens after deregistering the port (so no new opens) and before
releasing the port.

Johan


Thank you very much for the explanation. So, indeed shutdown is called before releasing port. There is no need for a patch.