Hi Greg,
This patch series fixes several locking and functional problems
with the .set_ldisc() notification sent by the tty core:
1) termios is used without holding termios_rwsem
2) uart port flags are changed without holding port mutex
3) several of the drivers call their enable_ms() routines directly
without holding port lock
4) The set_ldisc notification will turn on modem status interrupts
if the CD line is being used for N_PPS time signal, but doesn't
turn modem status interrupts back off if N_PPS is being unset.
Note: this series depends on the 26-patch 'tty locking changes'
series and the 10-patch 'serial core fixes' series.
Regards,
Peter Hurley (5):
tty: Allow safe access to termios for set_ldisc() handlers
serial: core: Claim port mutex for set_ldisc()
serial: core: Pass termios to set_ldisc() notifications
serial: Take uart port lock for direct *_enable_ms()
serial: Test/disable MSIs if switching from N_PPS
drivers/tty/serial/8250/8250_core.c | 27 ++++++++++++++++++++++++---
drivers/tty/serial/amba-pl010.c | 24 +++++++++++++++++++++---
drivers/tty/serial/atmel_serial.c | 11 +++++++++--
drivers/tty/serial/bfin_uart.c | 5 +++--
drivers/tty/serial/clps711x.c | 5 +++--
drivers/tty/serial/serial_core.c | 7 +++++--
drivers/tty/tty_ldisc.c | 5 ++++-
include/linux/serial_core.h | 2 +-
8 files changed, 70 insertions(+), 16 deletions(-)
--
2.1.3
Three UART drivers (8250, atmel & amba-pl010) directly call their
enable_ms() method; the uart port lock must be acquired before
any h/w programming.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/serial/8250/8250_core.c | 2 ++
drivers/tty/serial/amba-pl010.c | 2 ++
drivers/tty/serial/atmel_serial.c | 2 ++
3 files changed, 6 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 6cfdb7b..8eb0654 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2607,7 +2607,9 @@ serial8250_set_ldisc(struct uart_port *port, struct ktermios *termios)
{
if (termios->c_line == N_PPS) {
port->flags |= UPF_HARDPPS_CD;
+ spin_lock_irq(&port->lock);
serial8250_enable_ms(port);
+ spin_unlock_irq(&port->lock);
} else
port->flags &= ~UPF_HARDPPS_CD;
}
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index 76e40b3..04b83bf 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -472,7 +472,9 @@ static void pl010_set_ldisc(struct uart_port *port, struct ktermios *termios)
{
if (termios->c_line == N_PPS) {
port->flags |= UPF_HARDPPS_CD;
+ spin_lock_irq(&port->lock);
pl010_enable_ms(port);
+ spin_unlock_irq(&port->lock);
} else
port->flags &= ~UPF_HARDPPS_CD;
}
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index a2374d9..b68dfa5 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2046,7 +2046,9 @@ static void atmel_set_ldisc(struct uart_port *port, struct ktermios *termios)
{
if (termios->c_line == N_PPS) {
port->flags |= UPF_HARDPPS_CD;
+ spin_lock_irq(&port->lock);
atmel_enable_ms(port);
+ spin_unlock_irq(&port->lock);
} else {
port->flags &= ~UPF_HARDPPS_CD;
}
--
2.1.3
Switching to the N_PPS line discipline may require enabling
modem status interrupts; conversely switching from N_PPS may
require disabling modem status interrupts.
Affected drivers:
8250
amba-pl010
atmel
Cc: Nicolas Ferre <[email protected]>
Cc: Russell King <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/serial/8250/8250_core.c | 21 ++++++++++++++++++++-
drivers/tty/serial/amba-pl010.c | 18 +++++++++++++++++-
drivers/tty/serial/atmel_serial.c | 5 +++++
3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 8eb0654..095319b 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1397,6 +1397,19 @@ static void serial8250_stop_rx(struct uart_port *port)
serial8250_rpm_put(up);
}
+static void serial8250_disable_ms(struct uart_port *port)
+{
+ struct uart_8250_port *up =
+ container_of(port, struct uart_8250_port, port);
+
+ /* no MSR capabilities */
+ if (up->bugs & UART_BUG_NOMSR)
+ return;
+
+ up->ier &= ~UART_IER_MSI;
+ serial_port_out(port, UART_IER, up->ier);
+}
+
static void serial8250_enable_ms(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
@@ -2610,8 +2623,14 @@ serial8250_set_ldisc(struct uart_port *port, struct ktermios *termios)
spin_lock_irq(&port->lock);
serial8250_enable_ms(port);
spin_unlock_irq(&port->lock);
- } else
+ } else {
port->flags &= ~UPF_HARDPPS_CD;
+ if (!UART_ENABLE_MS(port, termios->c_cflag)) {
+ spin_lock_irq(&port->lock);
+ serial8250_disable_ms(port);
+ spin_unlock_irq(&port->lock);
+ }
+ }
}
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index 04b83bf..6f8464e 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -103,6 +103,16 @@ static void pl010_stop_rx(struct uart_port *port)
writel(cr, uap->port.membase + UART010_CR);
}
+static void pl010_disable_ms(struct uart_port *port)
+{
+ struct uart_amba_port *uap = (struct uart_amba_port *)port;
+ unsigned int cr;
+
+ cr = readb(uap->port.membase + UART010_CR);
+ cr &= ~UART010_CR_MSIE;
+ writel(cr, uap->port.membase + UART010_CR);
+}
+
static void pl010_enable_ms(struct uart_port *port)
{
struct uart_amba_port *uap = (struct uart_amba_port *)port;
@@ -475,8 +485,14 @@ static void pl010_set_ldisc(struct uart_port *port, struct ktermios *termios)
spin_lock_irq(&port->lock);
pl010_enable_ms(port);
spin_unlock_irq(&port->lock);
- } else
+ } else {
port->flags &= ~UPF_HARDPPS_CD;
+ if (!UART_ENABLE_MS(port, termios->c_cflag)) {
+ spin_lock_irq(&port->lock);
+ pl010_disable_ms(port);
+ spin_unlock_irq(&port->lock);
+ }
+ }
}
static const char *pl010_type(struct uart_port *port)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index b68dfa5..21380b6 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2051,6 +2051,11 @@ static void atmel_set_ldisc(struct uart_port *port, struct ktermios *termios)
spin_unlock_irq(&port->lock);
} else {
port->flags &= ~UPF_HARDPPS_CD;
+ if (!UART_ENABLE_MS(port, termios->c_cflag)) {
+ spin_lock_irq(&port->lock);
+ atmel_disable_ms(port);
+ spin_unlock_irq(&port->lock);
+ }
}
}
--
2.1.3
UART drivers which enable modem status interrupts when switching
to N_PPS line discipline need to determine if modem status
interrupts should be disabled when switching from N_PPS.
Specifically, the set_ldisc() notification needs to evaluate
UART_ENABLE_MS() which requires termios->c_cflag.
Convert in-tree UART drivers to new interface.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/serial/8250/8250_core.c | 4 ++--
drivers/tty/serial/amba-pl010.c | 4 ++--
drivers/tty/serial/atmel_serial.c | 4 ++--
drivers/tty/serial/bfin_uart.c | 5 +++--
drivers/tty/serial/clps711x.c | 5 +++--
drivers/tty/serial/serial_core.c | 2 +-
include/linux/serial_core.h | 2 +-
7 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index ca5cfdc..6cfdb7b 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2603,9 +2603,9 @@ serial8250_set_termios(struct uart_port *port, struct ktermios *termios,
}
static void
-serial8250_set_ldisc(struct uart_port *port, int new)
+serial8250_set_ldisc(struct uart_port *port, struct ktermios *termios)
{
- if (new == N_PPS) {
+ if (termios->c_line == N_PPS) {
port->flags |= UPF_HARDPPS_CD;
serial8250_enable_ms(port);
} else
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index 2064d31..76e40b3 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -468,9 +468,9 @@ pl010_set_termios(struct uart_port *port, struct ktermios *termios,
spin_unlock_irqrestore(&uap->port.lock, flags);
}
-static void pl010_set_ldisc(struct uart_port *port, int new)
+static void pl010_set_ldisc(struct uart_port *port, struct ktermios *termios)
{
- if (new == N_PPS) {
+ if (termios->c_line == N_PPS) {
port->flags |= UPF_HARDPPS_CD;
pl010_enable_ms(port);
} else
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 8a84034..a2374d9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2042,9 +2042,9 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
spin_unlock_irqrestore(&port->lock, flags);
}
-static void atmel_set_ldisc(struct uart_port *port, int new)
+static void atmel_set_ldisc(struct uart_port *port, struct ktermios *termios)
{
- if (new == N_PPS) {
+ if (termios->c_line == N_PPS) {
port->flags |= UPF_HARDPPS_CD;
atmel_enable_ms(port);
} else {
diff --git a/drivers/tty/serial/bfin_uart.c b/drivers/tty/serial/bfin_uart.c
index 7da9911..44b27ec 100644
--- a/drivers/tty/serial/bfin_uart.c
+++ b/drivers/tty/serial/bfin_uart.c
@@ -944,12 +944,13 @@ bfin_serial_verify_port(struct uart_port *port, struct serial_struct *ser)
* Enable the IrDA function if tty->ldisc.num is N_IRDA.
* In other cases, disable IrDA function.
*/
-static void bfin_serial_set_ldisc(struct uart_port *port, int ld)
+static void bfin_serial_set_ldisc(struct uart_port *port,
+ struct ktermios *termios)
{
struct bfin_serial_port *uart = (struct bfin_serial_port *)port;
unsigned int val;
- switch (ld) {
+ switch (termios->c_line) {
case N_IRDA:
val = UART_GET_GCTL(uart);
val |= (UMOD_IRDA | RPOLC);
diff --git a/drivers/tty/serial/clps711x.c b/drivers/tty/serial/clps711x.c
index acfe317..f963c4c 100644
--- a/drivers/tty/serial/clps711x.c
+++ b/drivers/tty/serial/clps711x.c
@@ -225,13 +225,14 @@ static void uart_clps711x_break_ctl(struct uart_port *port, int break_state)
writel(ubrlcr, port->membase + UBRLCR_OFFSET);
}
-static void uart_clps711x_set_ldisc(struct uart_port *port, int ld)
+static void uart_clps711x_set_ldisc(struct uart_port *port,
+ struct ktermios *termios)
{
if (!port->line) {
struct clps711x_port *s = dev_get_drvdata(port->dev);
regmap_update_bits(s->syscon, SYSCON_OFFSET, SYSCON1_SIREN,
- (ld == N_IRDA) ? SYSCON1_SIREN : 0);
+ (termios->c_line == N_IRDA) ? SYSCON1_SIREN : 0);
}
}
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 5209eaa..c5fb08c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1247,7 +1247,7 @@ static void uart_set_ldisc(struct tty_struct *tty)
if (uport->ops->set_ldisc) {
mutex_lock(&state->port.mutex);
- uport->ops->set_ldisc(uport, tty->termios.c_line);
+ uport->ops->set_ldisc(uport, &tty->termios);
mutex_unlock(&state->port.mutex);
}
}
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index ad93296..40b4cc4 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -63,7 +63,7 @@ struct uart_ops {
void (*flush_buffer)(struct uart_port *);
void (*set_termios)(struct uart_port *, struct ktermios *new,
struct ktermios *old);
- void (*set_ldisc)(struct uart_port *, int new);
+ void (*set_ldisc)(struct uart_port *, struct ktermios *);
void (*pm)(struct uart_port *, unsigned int state,
unsigned int oldstate);
--
2.1.3
Three UART drivers (8250, atmel & amba-pl010) enable modem status
interrupts if the line discipline is changed to N_PPS. However,
the uart port flags may only be safely modified while holding the
port mutex.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/serial/serial_core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 1a2d90f..5209eaa 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1245,8 +1245,11 @@ static void uart_set_ldisc(struct tty_struct *tty)
struct uart_state *state = tty->driver_data;
struct uart_port *uport = state->uart_port;
- if (uport->ops->set_ldisc)
+ if (uport->ops->set_ldisc) {
+ mutex_lock(&state->port.mutex);
uport->ops->set_ldisc(uport, tty->termios.c_line);
+ mutex_unlock(&state->port.mutex);
+ }
}
static void uart_set_termios(struct tty_struct *tty,
--
2.1.3
Allow a tty driver to safely access termios settings while handling
the set_ldisc() notification. UART drivers use the set_ldisc()
notification to check if the N_PPS line discipline is being enabled;
if so, modem status interrupts may also need to be enabled. Conversely,
modem status interrupts may need to be disabled if switching away
from the N_PPS line discipline.
Signed-off-by: Peter Hurley <[email protected]>
---
drivers/tty/tty_ldisc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index b66a81d..3737f55 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -572,8 +572,11 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
tty_ldisc_restore(tty, old_ldisc);
}
- if (tty->ldisc->ops->num != old_ldisc->ops->num && tty->ops->set_ldisc)
+ if (tty->ldisc->ops->num != old_ldisc->ops->num && tty->ops->set_ldisc) {
+ down_read(&tty->termios_rwsem);
tty->ops->set_ldisc(tty);
+ up_read(&tty->termios_rwsem);
+ }
/* At this point we hold a reference to the new ldisc and a
reference to the old ldisc, or we hold two references to
--
2.1.3