staging: greybus: uart: fix tty use after free

BugLink: https://bugs.launchpad.net/bugs/1947886
commit 92dc0b1f46e12cfabd28d709bb34f7a39431b44f upstream.

User space can hold a tty open indefinitely and tty drivers must not
release the underlying structures until the last user is gone.

Switch to using the tty-port reference counter to manage the life time
of the greybus tty state to avoid use after free after a disconnect.

Fixes: a18e15175708 ("greybus: more uart work")
Cc: stable@vger.kernel.org      # 4.9
Reviewed-by: Alex Elder <elder@linaro.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
Link: https://lore.kernel.org/r/20210906124538.22358-1-johan@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Kelsey Skunberg <kelsey.skunberg@canonical.com>

diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
index fc1bd68889c9142aabb5a0afaaa3f316c36d0717..0e6bebc20695dd5a682da67773afcb8934c51117 100644 (file)
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -789,6 +789,17 @@ out:
        gbphy_runtime_put_autosuspend(gb_tty->gbphy_dev);
 }
 
+static void gb_tty_port_destruct(struct tty_port *port)
+{
+       struct gb_tty *gb_tty = container_of(port, struct gb_tty, port);
+
+       if (gb_tty->minor != GB_NUM_MINORS)
+               release_minor(gb_tty);
+       kfifo_free(&gb_tty->write_fifo);
+       kfree(gb_tty->buffer);
+       kfree(gb_tty);
+}
+
 static const struct tty_operations gb_ops = {
        .install =              gb_tty_install,
        .open =                 gb_tty_open,
@@ -814,6 +825,7 @@ static const struct tty_port_operations gb_port_ops = {
        .dtr_rts =              gb_tty_dtr_rts,
        .activate =             gb_tty_port_activate,
        .shutdown =             gb_tty_port_shutdown,
+       .destruct =             gb_tty_port_destruct,
 };
 
 static int gb_uart_probe(struct gbphy_device *gbphy_dev,
@@ -826,17 +838,11 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev,
        int retval;
        int minor;
 
-       gb_tty = kzalloc(sizeof(*gb_tty), GFP_KERNEL);
-       if (!gb_tty)
-               return -ENOMEM;
-
        connection = gb_connection_create(gbphy_dev->bundle,
                                          le16_to_cpu(gbphy_dev->cport_desc->id),
                                          gb_uart_request_handler);
-       if (IS_ERR(connection)) {
-               retval = PTR_ERR(connection);
-               goto exit_tty_free;
-       }
+       if (IS_ERR(connection))
+               return PTR_ERR(connection);
 
        max_payload = gb_operation_get_payload_size_max(connection);
        if (max_payload < sizeof(struct gb_uart_send_data_request)) {
@@ -844,13 +850,23 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev,
                goto exit_connection_destroy;
        }
 
+       gb_tty = kzalloc(sizeof(*gb_tty), GFP_KERNEL);
+       if (!gb_tty) {
+               retval = -ENOMEM;
+               goto exit_connection_destroy;
+       }
+
+       tty_port_init(&gb_tty->port);
+       gb_tty->port.ops = &gb_port_ops;
+       gb_tty->minor = GB_NUM_MINORS;
+
        gb_tty->buffer_payload_max = max_payload -
                        sizeof(struct gb_uart_send_data_request);
 
        gb_tty->buffer = kzalloc(gb_tty->buffer_payload_max, GFP_KERNEL);
        if (!gb_tty->buffer) {
                retval = -ENOMEM;
-               goto exit_connection_destroy;
+               goto exit_put_port;
        }
 
        INIT_WORK(&gb_tty->tx_work, gb_uart_tx_write_work);
@@ -858,7 +874,7 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev,
        retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE,
                             GFP_KERNEL);
        if (retval)
-               goto exit_buf_free;
+               goto exit_put_port;
 
        gb_tty->credits = GB_UART_FIRMWARE_CREDITS;
        init_completion(&gb_tty->credits_complete);
@@ -872,7 +888,7 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev,
                } else {
                        retval = minor;
                }
-               goto exit_kfifo_free;
+               goto exit_put_port;
        }
 
        gb_tty->minor = minor;
@@ -881,9 +897,6 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev,
        init_waitqueue_head(&gb_tty->wioctl);
        mutex_init(&gb_tty->mutex);
 
-       tty_port_init(&gb_tty->port);
-       gb_tty->port.ops = &gb_port_ops;
-
        gb_tty->connection = connection;
        gb_tty->gbphy_dev = gbphy_dev;
        gb_connection_set_data(connection, gb_tty);
@@ -891,7 +904,7 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev,
 
        retval = gb_connection_enable_tx(connection);
        if (retval)
-               goto exit_release_minor;
+               goto exit_put_port;
 
        send_control(gb_tty, gb_tty->ctrlout);
 
@@ -918,16 +931,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev,
 
 exit_connection_disable:
        gb_connection_disable(connection);
-exit_release_minor:
-       release_minor(gb_tty);
-exit_kfifo_free:
-       kfifo_free(&gb_tty->write_fifo);
-exit_buf_free:
-       kfree(gb_tty->buffer);
+exit_put_port:
+       tty_port_put(&gb_tty->port);
 exit_connection_destroy:
        gb_connection_destroy(connection);
-exit_tty_free:
-       kfree(gb_tty);
 
        return retval;
 }
@@ -958,15 +965,10 @@ static void gb_uart_remove(struct gbphy_device *gbphy_dev)
        gb_connection_disable_rx(connection);
        tty_unregister_device(gb_tty_driver, gb_tty->minor);
 
-       /* FIXME - free transmit / receive buffers */
-
        gb_connection_disable(connection);
-       tty_port_destroy(&gb_tty->port);
        gb_connection_destroy(connection);
-       release_minor(gb_tty);
-       kfifo_free(&gb_tty->write_fifo);
-       kfree(gb_tty->buffer);
-       kfree(gb_tty);
+
+       tty_port_put(&gb_tty->port);
 }
 
 static int gb_tty_init(void)