[question] Is it possible to remove an active port (without shutdown)?

[question] Is it possible to remove an active port (without shutdown)?

[Saubhik Mukherjee]

Consider the following events involving drivers/tty/serial/owl-uart.c:

Suppose the driver is registered, and the owl_uart_probe() was called.
Then uart_startup() can be called in serial core. This calls
owl_uart_startup() which registers the interrupt handler owl_uart_irq.

Now suppose uart_remove_one_port() in serial core is called. This
detaches port from the core. This calls owl_uart_release_port(port).
This writes NULL to port->membase after iounmap of port->membase from
port->dev.

During this point, an interrupt is triggered and the interrupt callback
owl_uart_irq() is called (parallel with uart_remove_one_port()). This
tries to read port->membase to send or receive chars (with spinlock on
port->lock). This introduces a race condition on port->membase.

QUESTION: Is it possible to remove an active port (without shutdown)?

Re: [question] Is it possible to remove an active port (without shutdown)?

[Greg KH]

On Fri, Jun 11, 2021 at 03:03:42PM +0530, Saubhik Mukherjee wrote:
> Consider the following events involving drivers/tty/serial/owl-uart.c:
> 
> Suppose the driver is registered, and the owl_uart_probe() was called.
> Then uart_startup() can be called in serial core. This calls
> owl_uart_startup() which registers the interrupt handler owl_uart_irq.
> 
> Now suppose uart_remove_one_port() in serial core is called. This
> detaches port from the core. This calls owl_uart_release_port(port).
> This writes NULL to port->membase after iounmap of port->membase from
> port->dev.
> 
> During this point, an interrupt is triggered and the interrupt callback
> owl_uart_irq() is called (parallel with uart_remove_one_port()). This
> tries to read port->membase to send or receive chars (with spinlock on
> port->lock). This introduces a race condition on port->membase.
> 
> QUESTION: Is it possible to remove an active port (without shutdown)?

You can remove it, if the driver is set up to do so properly.  Odds are
the owl-uart code is not written to expect that to ever happen.

How are you "removing" an active port?  What triggers this action?

thanks,

greg k-h

Re: [question] Is it possible to remove an active port (without shutdown)?

[Saubhik Mukherjee]

On 6/11/21 3:17 PM, Greg KH wrote:
> On Fri, Jun 11, 2021 at 03:03:42PM +0530, Saubhik Mukherjee wrote:
>> Consider the following events involving drivers/tty/serial/owl-uart.c:
>>
>> Suppose the driver is registered, and the owl_uart_probe() was called.
>> Then uart_startup() can be called in serial core. This calls
>> owl_uart_startup() which registers the interrupt handler owl_uart_irq.
>>
>> Now suppose uart_remove_one_port() in serial core is called. This
>> detaches port from the core. This calls owl_uart_release_port(port).
>> This writes NULL to port->membase after iounmap of port->membase from
>> port->dev.
>>
>> During this point, an interrupt is triggered and the interrupt callback
>> owl_uart_irq() is called (parallel with uart_remove_one_port()). This
>> tries to read port->membase to send or receive chars (with spinlock on
>> port->lock). This introduces a race condition on port->membase.
>>
>> QUESTION: Is it possible to remove an active port (without shutdown)?
> 
> You can remove it, if the driver is set up to do so properly.  Odds are
> the owl-uart code is not written to expect that to ever happen.
> 
> How are you "removing" an active port?  What triggers this action?

Thank you for the reply.

The active port is removed (without shutdown) due to the platform 
callback owl_uart_remove() during de-registration of the platform 
driver. The race condition described is due to the interrupt handler, 
owl_uart_irq(), executing in parallel.

Re: [question] Is it possible to remove an active port (without shutdown)?

[Greg KH]

On Mon, Jun 14, 2021 at 03:27:43PM +0530, Saubhik Mukherjee wrote:
> On 6/11/21 3:17 PM, Greg KH wrote:
> > On Fri, Jun 11, 2021 at 03:03:42PM +0530, Saubhik Mukherjee wrote:
> > > Consider the following events involving drivers/tty/serial/owl-uart.c:
> > > 
> > > Suppose the driver is registered, and the owl_uart_probe() was called.
> > > Then uart_startup() can be called in serial core. This calls
> > > owl_uart_startup() which registers the interrupt handler owl_uart_irq.
> > > 
> > > Now suppose uart_remove_one_port() in serial core is called. This
> > > detaches port from the core. This calls owl_uart_release_port(port).
> > > This writes NULL to port->membase after iounmap of port->membase from
> > > port->dev.
> > > 
> > > During this point, an interrupt is triggered and the interrupt callback
> > > owl_uart_irq() is called (parallel with uart_remove_one_port()). This
> > > tries to read port->membase to send or receive chars (with spinlock on
> > > port->lock). This introduces a race condition on port->membase.
> > > 
> > > QUESTION: Is it possible to remove an active port (without shutdown)?
> > 
> > You can remove it, if the driver is set up to do so properly.  Odds are
> > the owl-uart code is not written to expect that to ever happen.
> > 
> > How are you "removing" an active port?  What triggers this action?
> 
> Thank you for the reply.
> 
> The active port is removed (without shutdown) due to the platform callback
> owl_uart_remove() during de-registration of the platform driver. The race
> condition described is due to the interrupt handler, owl_uart_irq(),
> executing in parallel.

Then the remove function needs to disable the irq and make sure the
handler is finished in order to properly be able to remove the platform
driver.

thanks,

greg k-h