diff mbox series

[for,4.14,4.19] serial: 8250: Avoid error message on reprobe

Message ID 20200925093109.388661-1-nobuhiro1.iwamatsu@toshiba.co.jp
State Superseded
Headers show
Series [for,4.14,4.19] serial: 8250: Avoid error message on reprobe | expand

Commit Message

Nobuhiro Iwamatsu Sept. 25, 2020, 9:31 a.m. UTC
From: Lukas Wunner <lukas@wunner.de>

commit e0a851fe6b9b619527bd928aa93caaddd003f70c upstream.

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.

Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v2.6.10
Cc: stable@vger.kernel.org # v2.6.10: 835d844d1a28: 8250_pnp: do pnp probe before legacy
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/b4a072013ee1a1d13ee06b4325afb19bda57ca1b.1589285873.git.lukas@wunner.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[iwamatsu: Backported to 4.14, 4.19: adjust context]
Signed-off-by: Nobuhiro Iwamatsu (CIP) <nobuhiro1.iwamatsu@toshiba.co.jp>
---
 drivers/tty/serial/8250/8250_core.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Greg KH Sept. 25, 2020, 10:58 a.m. UTC | #1
On Fri, Sep 25, 2020 at 06:31:09PM +0900, Nobuhiro Iwamatsu wrote:
> From: Lukas Wunner <lukas@wunner.de>

> 

> commit e0a851fe6b9b619527bd928aa93caaddd003f70c upstream.

> 

> 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.

> 

> Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe")

> Signed-off-by: Lukas Wunner <lukas@wunner.de>

> Cc: stable@vger.kernel.org # v2.6.10

> Cc: stable@vger.kernel.org # v2.6.10: 835d844d1a28: 8250_pnp: do pnp probe before legacy

> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Link: https://lore.kernel.org/r/b4a072013ee1a1d13ee06b4325afb19bda57ca1b.1589285873.git.lukas@wunner.de

> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> [iwamatsu: Backported to 4.14, 4.19: adjust context]

> Signed-off-by: Nobuhiro Iwamatsu (CIP) <nobuhiro1.iwamatsu@toshiba.co.jp>


Thanks, now queue dup.

greg k-h
diff mbox series

Patch

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index d6b790510c94..8d46bd612888 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1065,8 +1065,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",
@@ -1091,6 +1093,11 @@  int serial8250_register_8250_port(struct uart_8250_port *up)
 	mutex_unlock(&serial_mutex);
 
 	return ret;
+
+err:
+	uart->port.dev = NULL;
+	mutex_unlock(&serial_mutex);
+	return ret;
 }
 EXPORT_SYMBOL(serial8250_register_8250_port);