Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure
@ 2026-09-08  9:36 Denis Arefev
  2026-09-08  9:36 ` [PATCH 1/2 5.10] serial: 8250_dw: Use dev_err_probe() Denis Arefev
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Denis Arefev @ 2026-09-08  9:36 UTC (permalink / raw)
  To: stable, Greg Kroah-Hartman
  Cc: Andy Shevchenko, Jiri Slaby, Sasha Levin, Artem Shimko,
	Stepan Ionichev, Serge Semin, linux-serial, linux-kernel

This series cleans up the error handling in dw8250_probe() and fixes a
port leak. When clk_notifier_register() fails, probe() returns an error
but the 8250 port registered just before it stays registered: the
matching serial8250_unregister_port() lives in dw8250_remove(), which is
not called on probe failure. The port slot then stays occupied until a
rebind, and the devm-allocated driver data is freed while the port still
references it (via private_data and the serial_in/serial_out callbacks),
a use-after-free hazard.

The first patch converts the probe() error paths to dev_err_probe().
The second patch unregisters the 8250 port on the
clk_notifier_register() error path.

Andy Shevchenko (1):
  serial: 8250_dw: Use dev_err_probe()

Stepan Ionichev (1):
  serial: 8250_dw: unregister 8250 port if clk_notifier_register() fails

 drivers/tty/serial/8250/8250_dw.c | 32 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 18 deletions(-)
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2 5.10] serial: 8250_dw: Use dev_err_probe()
  2026-09-08  9:36 [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure Denis Arefev
@ 2026-09-08  9:36 ` Denis Arefev
  2026-09-08  9:36 ` [PATCH 2/2 5.10] serial: 8250_dw: unregister 8250 port if clk_notifier_register() fails Denis Arefev
  2026-09-08 22:39 ` [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure Sasha Levin
  2 siblings, 0 replies; 5+ messages in thread
From: Denis Arefev @ 2026-09-08  9:36 UTC (permalink / raw)
  To: stable, Greg Kroah-Hartman
  Cc: Andy Shevchenko, Jiri Slaby, Sasha Levin, Artem Shimko,
	Stepan Ionichev, linux-serial, linux-kernel

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

commit 57f83e5dd6a33c4696699954784f8fee789b1d0c upstream.

Simplify the error path in ->probe() a bit by using dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20220509172129.37770-2-andriy.shevchenko@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Denis Arefev <arefev@swemel.ru>
---
 drivers/tty/serial/8250/8250_dw.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 211dc88ce4e2..629eb24166b5 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -451,18 +451,17 @@ static void dw8250_reset_control_assert(void *data)
 static int dw8250_probe(struct platform_device *pdev)
 {
 	struct uart_8250_port uart = {}, *up = &uart;
-	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	struct uart_port *p = &up->port;
 	struct device *dev = &pdev->dev;
 	struct dw8250_data *data;
+	struct resource *regs;
 	int irq;
 	int err;
 	u32 val;
 
-	if (!regs) {
-		dev_err(dev, "no registers defined\n");
-		return -EINVAL;
-	}
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!regs)
+		return dev_err_probe(dev, -EINVAL, "no registers defined\n");
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -547,7 +546,7 @@ static int dw8250_probe(struct platform_device *pdev)
 
 	err = clk_prepare_enable(data->clk);
 	if (err)
-		dev_warn(dev, "could not enable optional baudclk: %d\n", err);
+		return dev_err_probe(dev, err, "could not enable optional baudclk\n");
 
 	err = devm_add_action_or_reset(dev, dw8250_clk_disable_unprepare, data->clk);
 	if (err)
@@ -557,20 +556,16 @@ static int dw8250_probe(struct platform_device *pdev)
 		p->uartclk = clk_get_rate(data->clk);
 
 	/* If no clock rate is defined, fail. */
-	if (!p->uartclk) {
-		dev_err(dev, "clock rate not defined\n");
-		return -EINVAL;
-	}
+	if (!p->uartclk)
+		return dev_err_probe(dev, -EINVAL, "clock rate not defined\n");
 
 	data->pclk = devm_clk_get_optional(dev, "apb_pclk");
 	if (IS_ERR(data->pclk))
 		return PTR_ERR(data->pclk);
 
 	err = clk_prepare_enable(data->pclk);
-	if (err) {
-		dev_err(dev, "could not enable apb_pclk\n");
-		return err;
-	}
+	if (err)
+		return dev_err_probe(dev, err, "could not enable apb_pclk\n");
 
 	err = devm_add_action_or_reset(dev, dw8250_clk_disable_unprepare, data->pclk);
 	if (err)
@@ -616,9 +611,8 @@ static int dw8250_probe(struct platform_device *pdev)
 	if (data->clk) {
 		err = clk_notifier_register(data->clk, &data->clk_notifier);
 		if (err)
-			dev_warn(p->dev, "Failed to set the clock notifier\n");
-		else
-			queue_work(system_unbound_wq, &data->clk_work);
+			return dev_err_probe(dev, err, "Failed to set the clock notifier\n");
+		queue_work(system_unbound_wq, &data->clk_work);
 	}
 
 	platform_set_drvdata(pdev, data);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2 5.10] serial: 8250_dw: unregister 8250 port if clk_notifier_register() fails
  2026-09-08  9:36 [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure Denis Arefev
  2026-09-08  9:36 ` [PATCH 1/2 5.10] serial: 8250_dw: Use dev_err_probe() Denis Arefev
@ 2026-09-08  9:36 ` Denis Arefev
  2026-09-08 22:39 ` [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure Sasha Levin
  2 siblings, 0 replies; 5+ messages in thread
From: Denis Arefev @ 2026-09-08  9:36 UTC (permalink / raw)
  To: stable, Greg Kroah-Hartman
  Cc: Andy Shevchenko, Jiri Slaby, Sasha Levin, Artem Shimko,
	Stepan Ionichev, Serge Semin, linux-serial, linux-kernel

From: Stepan Ionichev <sozdayvek@gmail.com>

commit 10fc708b4de7f86002d2d735a2dbf3b5b7f65692 upstream.

dw8250_probe() registers the 8250 port via serial8250_register_8250_port()
and then, if the device has a clock, registers a clock notifier. If
clk_notifier_register() fails, probe returns the error but leaves the
8250 port registered. The matching serial8250_unregister_port() lives
in dw8250_remove(), which is not called when probe fails, so the port
slot stays occupied until the device is rebound or the system is
rebooted. The devm-allocated driver data is freed while the port still
references it (via the saved private_data and serial_in/serial_out
callbacks), so any access to that port slot before a rebind is a
use-after-free hazard.

Unregister the port on the clk_notifier_register() error path.

Fixes: cc816969d7b5 ("serial: 8250_dw: Fix common clocks usage race condition")
Cc: stable@vger.kernel.org
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20260514143746.23671-2-sozdayvek@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Denis Arefev: adapted for 5.10:
- system_dfl_wq is not available here, use system_unbound_wq instead]
Signed-off-by: Denis Arefev <arefev@swemel.ru>
---
Backport fix for CVE-2026-53384
Link: https://nvd.nist.gov/vuln/detail/CVE-2026-53384
---

 drivers/tty/serial/8250/8250_dw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 629eb24166b5..0afcfe4ff56a 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -610,8 +610,10 @@ static int dw8250_probe(struct platform_device *pdev)
 	 */
 	if (data->clk) {
 		err = clk_notifier_register(data->clk, &data->clk_notifier);
-		if (err)
+		if (err) {
+			serial8250_unregister_port(data->data.line);
 			return dev_err_probe(dev, err, "Failed to set the clock notifier\n");
+		}
 		queue_work(system_unbound_wq, &data->clk_work);
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure
  2026-09-08  9:36 [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure Denis Arefev
  2026-09-08  9:36 ` [PATCH 1/2 5.10] serial: 8250_dw: Use dev_err_probe() Denis Arefev
  2026-09-08  9:36 ` [PATCH 2/2 5.10] serial: 8250_dw: unregister 8250 port if clk_notifier_register() fails Denis Arefev
@ 2026-09-08 22:39 ` Sasha Levin
  2026-09-08 22:59   ` Sasha Levin
  2 siblings, 1 reply; 5+ messages in thread
From: Sasha Levin @ 2026-09-08 22:39 UTC (permalink / raw)
  To: stable, Greg Kroah-Hartman
  Cc: Sasha Levin, Andy Shevchenko, Jiri Slaby, Artem Shimko,
	Stepan Ionichev, Serge Semin, linux-serial, linux-kernel,
	Denis Arefev

> This series cleans up the error handling in dw8250_probe() and fixes a
> port leak. When clk_notifier_register() fails, probe() returns an error
> but the 8250 port registered just before it stays registered: the
> matching serial8250_unregister_port() lives in dw8250_remove(), which is
> not called on probe failure.

Queued for 5.10, thanks.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure
  2026-09-08 22:39 ` [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure Sasha Levin
@ 2026-09-08 22:59   ` Sasha Levin
  0 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-09-08 22:59 UTC (permalink / raw)
  To: stable, Greg Kroah-Hartman
  Cc: Sasha Levin, Andy Shevchenko, Jiri Slaby, Artem Shimko,
	Stepan Ionichev, Serge Semin, linux-serial, linux-kernel,
	Denis Arefev

> Queued for 5.10, thanks.

I'm dropping these again...

The bug being fixed isn't reachable on 5.10. dw8250_probe() there ends the
clock notifier block with:

	if (data->clk) {
		err = clk_notifier_register(data->clk, &data->clk_notifier);
		if (err)
			dev_warn(p->dev, "Failed to set the clock notifier\n");
		else
			queue_work(system_unbound_wq, &data->clk_work);
	}

	platform_set_drvdata(pdev, data);

so a notifier registration failure warns and probe still returns 0. There is no
error return anywhere after serial8250_register_8250_port(), which is the
precondition 10fc708b4de7 ("serial: 8250_dw: unregister 8250 port if
clk_notifier_register() fails") describes when it says probe "returns the error
but leaves the 8250 port registered". No port is left behind on 5.10, so
there's nothing to unregister and no use after free.

What creates that path is patch 1/2, 57f83e5dd6a3 ("serial: 8250_dw: Use
dev_err_probe()"), which turns the warn into a return. That's a cleanup from
May 2022 with no Fixes tag and no stable Cc. It's in the base of 6.1 and newer,
which is why the fix is correct there, but 5.10 and 5.15 predate it and never
had the failing path. Taking the pair here would import the bug and its fix
together, and would also make a notifier registration failure fatal to probe on
a tree where it currently isn't, for hardware that works today.

Same reasoning applies to 5.15, so please don't send it there either.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-08 22:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08  9:36 [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure Denis Arefev
2026-09-08  9:36 ` [PATCH 1/2 5.10] serial: 8250_dw: Use dev_err_probe() Denis Arefev
2026-09-08  9:36 ` [PATCH 2/2 5.10] serial: 8250_dw: unregister 8250 port if clk_notifier_register() fails Denis Arefev
2026-09-08 22:39 ` [PATCH 0/2 5.10] serial: 8250_dw: fix port leak on clock notifier failure Sasha Levin
2026-09-08 22:59   ` Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox