On 16/02/2022 15:20, Greg Kroah-Hartman wrote:
On Wed, Feb 16, 2022 at 03:08:58PM +0000, Phil Elwell wrote:
UART drivers are meant to use the port spinlock within certain
methods, to protect against reentrancy. The sc16is7xx driver does
very little locking, presumably because when added it triggers
"scheduling while atomic" errors. This is due to the use of mutexes
within the regmap abstraction layer, and the mutex implementation's
habit of sleeping the current thread while waiting for access.
Unfortunately this lack of interlocking can lead to corruption of
outbound data, which occurs when the buffer used for I2C transmission
is used simultaneously by two threads - a work queue thread running
sc16is7xx_tx_proc, and an IRQ thread in sc16is7xx_port_irq, both
of which can call sc16is7xx_handle_tx.
An earlier patch added efr_lock, a mutex that controls access to the
EFR register. This mutex is already claimed in the IRQ handler, and
all that is required is to claim the same mutex in sc16is7xx_tx_proc.
See: https://github.com/raspberrypi/linux/issues/4885
Signed-off-by: Phil Elwell <phil@xxxxxxxxxxxxxxx>
---
drivers/tty/serial/sc16is7xx.c | 3 +++
1 file changed, 3 insertions(+)
What commit id does this fix?
That's a good question, and one you shouldn't have to ask. I was going to say
[1] because it deleted a previous mutex, but I think it's use of a single
workqueue for both IRQ and TX tasks made it safe. That leaves [2], which
replaces the IRQ work queue with a threaded IRQ, removing the reentrancy protection.
If you're in agreement I'll send a v2 with the appropriate Fixes: tag.
[1] commit 9e6f0ca3e567 ("sc16is7xx: use kthread_worker for tx_work and irq")
[2] commit 6393ff1c4435 ("sc16is7xx: Use threaded IRQ")
Phil
