serial: sprd: Assign sprd_port after initialized to avoid wrong access
authorChunyan Zhang <chunyan.zhang@unisoc.com>
Tue, 25 Jul 2023 06:40:52 +0000 (14:40 +0800)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sun, 30 Jul 2023 11:53:55 +0000 (13:53 +0200)
The global pointer 'sprd_port' may not zero when sprd_probe returns
failure, that is a risk for sprd_port to be accessed afterward, and
may lead to unexpected errors.

For example:

There are two UART ports, UART1 is used for console and configured in
kernel command line, i.e. "console=";

The UART1 probe failed and the memory allocated to sprd_port[1] was
released, but sprd_port[1] was not set to NULL;

In UART2 probe, the same virtual address was allocated to sprd_port[2],
and UART2 probe process finally will go into sprd_console_setup() to
register UART1 as console since it is configured as preferred console
(filled to console_cmdline[]), but the console parameters (sprd_port[1])
belong to UART2.

So move the sprd_port[] assignment to where the port already initialized
can avoid the above issue.

Fixes: b7396a38fb28 ("tty/serial: Add Spreadtrum sc9836-uart driver support")
Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
Link: https://lore.kernel.org/r/20230725064053.235448-1-chunyan.zhang@unisoc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/tty/serial/sprd_serial.c

index 792d016..efca715 100644 (file)
@@ -1107,7 +1107,7 @@ static bool sprd_uart_is_console(struct uart_port *uport)
 static int sprd_clk_init(struct uart_port *uport)
 {
        struct clk *clk_uart, *clk_parent;
-       struct sprd_uart_port *u = sprd_port[uport->line];
+       struct sprd_uart_port *u = container_of(uport, struct sprd_uart_port, port);
 
        clk_uart = devm_clk_get(uport->dev, "uart");
        if (IS_ERR(clk_uart)) {
@@ -1150,22 +1150,22 @@ static int sprd_probe(struct platform_device *pdev)
 {
        struct resource *res;
        struct uart_port *up;
+       struct sprd_uart_port *sport;
        int irq;
        int index;
        int ret;
 
        index = of_alias_get_id(pdev->dev.of_node, "serial");
-       if (index < 0 || index >= ARRAY_SIZE(sprd_port)) {
+       if (index < 0 || index >= UART_NR_MAX) {
                dev_err(&pdev->dev, "got a wrong serial alias id %d\n", index);
                return -EINVAL;
        }
 
-       sprd_port[index] = devm_kzalloc(&pdev->dev, sizeof(*sprd_port[index]),
-                                       GFP_KERNEL);
-       if (!sprd_port[index])
+       sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
+       if (!sport)
                return -ENOMEM;
 
-       up = &sprd_port[index]->port;
+       up = &sport->port;
        up->dev = &pdev->dev;
        up->line = index;
        up->type = PORT_SPRD;
@@ -1195,7 +1195,7 @@ static int sprd_probe(struct platform_device *pdev)
         * Allocate one dma buffer to prepare for receive transfer, in case
         * memory allocation failure at runtime.
         */
-       ret = sprd_rx_alloc_buf(sprd_port[index]);
+       ret = sprd_rx_alloc_buf(sport);
        if (ret)
                return ret;
 
@@ -1206,14 +1206,23 @@ static int sprd_probe(struct platform_device *pdev)
                        return ret;
                }
        }
+
        sprd_ports_num++;
+       sprd_port[index] = sport;
 
        ret = uart_add_one_port(&sprd_uart_driver, up);
        if (ret)
-               sprd_remove(pdev);
+               goto clean_port;
 
        platform_set_drvdata(pdev, up);
 
+       return 0;
+
+clean_port:
+       sprd_port[index] = NULL;
+       if (--sprd_ports_num == 0)
+               uart_unregister_driver(&sprd_uart_driver);
+       sprd_rx_free_buf(sport);
        return ret;
 }