Re: [PATCH 01/11] drivers: tty: serial: Fix deadlock in sa1100_set_termios()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07. 04. 22, 8:33, Duoming Zhou wrote:
There is a deadlock in sa1100_set_termios(), which is shown
below:

    (Thread 1)              |      (Thread 2)
                            | sa1100_enable_ms()
sa1100_set_termios()       |  mod_timer()
  spin_lock_irqsave() //(1) |  (wait a time)
  ...                       | sa1100_timeout()
  del_timer_sync()          |  spin_lock_irqsave() //(2)
  (wait timer to stop)      |  ...

We hold sport->port.lock in position (1) of thread 1 and
use del_timer_sync() to wait timer to stop, but timer handler
also need sport->port.lock in position (2) of thread 2. As a result,
sa1100_set_termios() will block forever.

This patch extracts del_timer_sync() from the protection of
spin_lock_irqsave(), which could let timer handler to obtain
the needed lock.

Signed-off-by: Duoming Zhou <duoming@xxxxxxxxxx>
---
  drivers/tty/serial/sa1100.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index 5fe6cccfc1a..3a5f12ced0b 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -476,7 +476,9 @@ sa1100_set_termios(struct uart_port *port, struct ktermios *termios,
  				UTSR1_TO_SM(UTSR1_ROR);
  	}
+ spin_unlock_irqrestore(&sport->port.lock, flags);

Unlocking the lock at this point doesn't look safe at all. Maybe moving the timer deletion before the lock? There is no current maintainer to ask. Most of the driver originates from rmk. Ccing him just in case.

FWIW the lock was moved by this commit around linux 2.5.55 (from full-history-linux [1])
commit f38aef3e62c26a33ea360a86fde9b27e183a3748
Author: Russell King <rmk@xxxxxxxxxxxxxxxxxxxxxx>
Date:   Fri Jan 3 15:42:09 2003 +0000

    [SERIAL] Convert change_speed() to settermios()

[1] https://archive.org/download/git-history-of-linux/full-history-linux.git.tar

  	del_timer_sync(&sport->timer);
+	spin_lock_irqsave(&sport->port.lock, flags);
/*
  	 * Update the per-port timeout.

thanks,
--
js
suse labs




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux