On Tue, May 12, 2020 at 02:40:01PM +0200, Lukas Wunner wrote:
> If the call to uart_add_one_port() in serial8250_register_8250_port()
> fails, a half-initialized entry in the serial_8250ports[] array is left
> behind.
>
> A subsequent reprobe of the same serial port causes that entry to be
> reused. Because uart->port.dev is set, uart_remove_one_port() is called
> for the half-initialized entry and bails out with an error message:
>
> bcm2835-aux-uart 3f215040.serial: Removing wrong port: (null) != (ptrval)
>
> The same happens on failure of mctrl_gpio_init() since commit
> 4a96895f74c9 ("tty/serial/8250: use mctrl_gpio helpers").
>
> Fix by zeroing the uart->port.dev pointer in the probe error path.
>
> The bug was introduced in v2.6.10 by historical commit befff6f5bf5f
> ("[SERIAL] Add new port registration/unregistration functions."):
> https://git.kernel.org/tglx/history/c/befff6f5bf5f
>
> The commit added an unconditional call to uart_remove_one_port() in
> serial8250_register_port(). In v3.7, commit 835d844d1a28 ("8250_pnp:
> do pnp probe before legacy probe") made that call conditional on
> uart->port.dev which allows me to fix the issue by zeroing that pointer
> in the error path. Thus, the present commit will fix the problem as far
> back as v3.7 whereas still older versions need to also cherry-pick
> 835d844d1a28.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe")
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # v2.6.10
> Cc: stable@xxxxxxxxxxxxxxx # v2.6.10: 835d844d1a28: 8250_pnp: do pnp probe before legacy
> ---
> Changes v1 -> v2:
> * Drop unnecessary "else" after "if ... goto" statement. (Andy)
> * Document applicability to older kernels in commit message. (Andy)
> * Add Fixes tag. (Andy)
>
> drivers/tty/serial/8250/8250_core.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 45d9117cab68..9548d3f8fc8e 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -1040,7 +1040,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> gpios = mctrl_gpio_init(&uart->port, 0);
> if (IS_ERR(gpios)) {
> ret = PTR_ERR(gpios);
> - goto out_unlock;
> + goto err;
> } else {
> uart->gpios = gpios;
> }
> @@ -1089,8 +1089,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> serial8250_apply_quirks(uart);
> ret = uart_add_one_port(&serial8250_reg,
> &uart->port);
> - if (ret == 0)
> - ret = uart->port.line;
> + if (ret)
> + goto err;
> +
> + ret = uart->port.line;
> } else {
> dev_info(uart->port.dev,
> "skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n",
> @@ -1112,10 +1114,14 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> }
> }
>
> -out_unlock:
> mutex_unlock(&serial_mutex);
>
> return ret;
> +
> +err:
> + uart->port.dev = NULL;
> + mutex_unlock(&serial_mutex);
> + return ret;
> }
> EXPORT_SYMBOL(serial8250_register_8250_port);
>
> --
> 2.26.2
>
--
With Best Regards,
Andy Shevchenko
