serial_core: Avoid NULL pointer dereference in uart_close()
authorGeert Uytterhoeven <geert+renesas@linux-m68k.org>
Mon, 17 Mar 2014 13:10:59 +0000 (14:10 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 17 Mar 2014 23:17:55 +0000 (16:17 -0700)
When unbinding a serial driver that's being used as a serial console,
the kernel may crash with a NULL pointer dereference in a uart_*() function
called from uart_close () (e.g. uart_flush_buffer() or
uart_chars_in_buffer()).

To fix this, let uart_close() check for port->count == 0. If this is the
case, bail out early. Else tty_port_close_start() will make the port
counts inconsistent, printing out warnings like

    tty_port_close_start: tty->count = 1 port count = 0.

and

    tty_port_close_start: count = -1

and once uport == NULL, it will also crash.

Also fix the related crash in pr_debug() by checking for a non-NULL uport
first.

Detailed description:

On driver unbind, uart_remove_one_port() is called. Basically it;
  - marks the port dead,
  - calls tty_vhangup(),
  - sets state->uart_port = NULL.

What will happen depends on whether the port is just in use by e.g. getty,
or was also opened as a console.

A. If the tty was not opened as a console:

  - tty_vhangup() will (in __tty_hangup()):
      - mark all file descriptors for this tty hung up by pointing them to
hung_up_tty_fops,
      - call uart_hangup(), which sets port->count to 0.

  - A subsequent uart_open() (this may be through /dev/ttyS*, or through
    /dev/console if this is a serial console) will fail with -ENXIO as the
    port was marked dead,
  - uart_close() after the failed uart_open() will return early, as
    tty_hung_up_p() (called from tty_port_close_start()) will notice it was
    hung up.

B. If the tty was also opened as a console:

  - tty_vhangup() will (in __tty_hangup()):
      - mark non-console file descriptors for this tty hung up by pointing
them to hung_up_tty_fops,
      - NOT call uart_hangup(), but instead call uart_close() for every
        non-console file descriptor, so port->count will still have a
non-zero value afterwards.

  - A subsequent uart_open() will fail with -ENXIO as the port was
    marked dead,
  - uart_close() after the failed uart_open() starts to misbehave:
      - tty_hung_up_p() will not notice it was hung up,
      - As port->count is non-zero, tty_port_close_start() will decrease
        port->count, making the tty and port counts inconsistent. Later,
warnings like these will be printed:

    tty_port_close_start: tty->count = 1 port count = 0.

and
    tty_port_close_start: count = -1

      - If all of this happens after state->uart_port was set to zero, a
        NULL pointer dereference will happen.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/tty/serial/serial_core.c

index 3253905428eaa86f6cd1449d982b940e556c0a3d..2cf5649a6dc038396636f09098ed6024d5880838 100644 (file)
@@ -1319,9 +1319,9 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
        uport = state->uart_port;
        port = &state->port;
 
-       pr_debug("uart_close(%d) called\n", uport->line);
+       pr_debug("uart_close(%d) called\n", uport ? uport->line : -1);
 
-       if (tty_port_close_start(port, tty, filp) == 0)
+       if (!port->count || tty_port_close_start(port, tty, filp) == 0)
                return;
 
        /*