[PATCH] serial: pl011: Don't leak amba_ports entry on driver register error

[PATCH] serial: pl011: Don't leak amba_ports entry on driver register error

[Lukas Wunner]

pl011_probe() calls pl011_setup_port() to reserve an amba_ports[] entry,
then calls pl011_register_port() to register the uart driver with the
tty layer.

If registration of the uart driver fails, the amba_ports[] entry is not
released.  If this happens 14 times (value of UART_NR macro), then all
amba_ports[] entries will have been leaked and driver probing is no
longer possible.  (To be fair, that can only happen if the DeviceTree
doesn't contain alias IDs since they cause the same entry to be used for
a given port.)   Fix it.

Fixes: ef2889f7ffee ("serial: pl011: Move uart_register_driver call to device")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v3.15+
Cc: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/tty/serial/amba-pl011.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index cd1ba8d8b0e6..67498594d7d7 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2614,7 +2614,7 @@ static int pl011_setup_port(struct device *dev, struct uart_amba_port *uap,
 
 static int pl011_register_port(struct uart_amba_port *uap)
 {
-	int ret;
+	int ret, i;
 
 	/* Ensure interrupts from this UART are masked and cleared */
 	pl011_write(0, uap, REG_IMSC);
@@ -2625,6 +2625,9 @@ static int pl011_register_port(struct uart_amba_port *uap)
 		if (ret < 0) {
 			dev_err(uap->port.dev,
 				"Failed to register AMBA-PL011 driver\n");
+			for (i = 0; i < ARRAY_SIZE(amba_ports); i++)
+				if (amba_ports[i] == uap)
+					amba_ports[i] = NULL;
 			return ret;
 		}
 	}
-- 
2.27.0

Re: [PATCH] serial: pl011: Don't leak amba_ports entry on driver register error

[Jiri Slaby]

On 13. 08. 20, 12:59, Lukas Wunner wrote:
> pl011_probe() calls pl011_setup_port() to reserve an amba_ports[] entry,
> then calls pl011_register_port() to register the uart driver with the
> tty layer.
> 
> If registration of the uart driver fails, the amba_ports[] entry is not
> released.  If this happens 14 times (value of UART_NR macro), then all
> amba_ports[] entries will have been leaked and driver probing is no
> longer possible.  (To be fair, that can only happen if the DeviceTree
> doesn't contain alias IDs since they cause the same entry to be used for
> a given port.)   Fix it.
> 
> Fixes: ef2889f7ffee ("serial: pl011: Move uart_register_driver call to device")
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: stable@vger.kernel.org # v3.15+
> Cc: Tushar Behera <tushar.behera@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index cd1ba8d8b0e6..67498594d7d7 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2614,7 +2614,7 @@ static int pl011_setup_port(struct device *dev, struct uart_amba_port *uap,
>  
>  static int pl011_register_port(struct uart_amba_port *uap)
>  {
> -	int ret;
> +	int ret, i;
>  
>  	/* Ensure interrupts from this UART are masked and cleared */
>  	pl011_write(0, uap, REG_IMSC);
> @@ -2625,6 +2625,9 @@ static int pl011_register_port(struct uart_amba_port *uap)
>  		if (ret < 0) {
>  			dev_err(uap->port.dev,
>  				"Failed to register AMBA-PL011 driver\n");
> +			for (i = 0; i < ARRAY_SIZE(amba_ports); i++)
> +				if (amba_ports[i] == uap)
> +					amba_ports[i] = NULL;

This doesn't look like the right place to free it. The callers should do
it instead. Or if you really want to do it here, just pass the index to
the function, so that you don't have to find it.

thanks,
-- 
js
suse labs