Commit 9c59b4c8 authored by Sergey Senozhatsky's avatar Sergey Senozhatsky Committed by Greg Kroah-Hartman

tty/serial: do not free trasnmit buffer page under port lock

[ Upstream commit d7240214 ]

LKP has hit yet another circular locking dependency between uart
console drivers and debugobjects [1]:

     CPU0                                    CPU1

     uart_shutdown()                          /* db->lock */
      /* uart_port->lock */                    debug_print_object()
       free_page()                              printk()
        debug_check_no_obj_freed()                /* uart_port->lock */
         /* db->lock */

So there are two dependency chains:
	uart_port->lock -> db->lock
	db->lock -> uart_port->lock

This particular circular locking dependency can be addressed in several

a) One way would be to move debug_print_object() out of db->lock scope
   and, thus, break the db->lock -> uart_port->lock chain.
b) Another one would be to free() transmit buffer page out of db->lock
   in UART code; which is what this patch does.

It makes sense to apply a) and b) independently: there are too many things
going on behind free(), none of which depend on uart_port->lock.

The patch fixes transmit buffer page free() in uart_shutdown() and,
additionally, in uart_port_startup() (as was suggested by Dmitry Safonov).

[1] sergey-senozhatsky's avatarSergey Senozhatsky <>
Reviewed-by: default avatarPetr Mladek <>
Acked-by: default avatarPeter Zijlstra (Intel) <>
Cc: Greg Kroah-Hartman <>
Cc: Jiri Slaby <>
Cc: Andrew Morton <>
Cc: Waiman Long <>
Cc: Dmitry Safonov <>
Cc: Steven Rostedt <>
Signed-off-by: default avatarGreg Kroah-Hartman <>
Signed-off-by: default avatarSasha Levin <>
parent e18db412
......@@ -205,10 +205,15 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
if (!state->xmit.buf) {
state->xmit.buf = (unsigned char *) page;
uart_port_unlock(uport, flags);
} else {
uart_port_unlock(uport, flags);
* Do not free() the page under the port lock, see
* uart_shutdown().
uart_port_unlock(uport, flags);
retval = uport->ops->startup(uport);
if (retval == 0) {
......@@ -268,6 +273,7 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
struct uart_port *uport = uart_port_check(state);
struct tty_port *port = &state->port;
unsigned long flags = 0;
char *xmit_buf = NULL;
* Set the TTY IO error marker
......@@ -298,14 +304,18 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
tty_port_set_suspended(port, 0);
* Free the transmit buffer page.
* Do not free() the transmit buffer page under the port lock since
* this can create various circular locking scenarios. For instance,
* console driver may need to allocate/free a debug object, which
* can endup in printk() recursion.
uart_port_lock(state, flags);
if (state->xmit.buf) {
free_page((unsigned long)state->xmit.buf);
state->xmit.buf = NULL;
xmit_buf = state->xmit.buf;
state->xmit.buf = NULL;
uart_port_unlock(uport, flags);
if (xmit_buf)
free_page((unsigned long)xmit_buf);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment