Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list
@ 2026-09-01  9:50 Aniket Randive
  2026-09-01  9:53 ` Praveen Talari
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Aniket Randive @ 2026-09-01  9:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: konrad.dybcio, naresh.maramaina, praveen.talari, linux-arm-msm,
	linux-kernel, linux-serial, Aniket Randive

The SA8255P UART variants attach PM domains through
devm_pm_domain_attach_list(), which registers a devres cleanup action
to detach and free the PM domain list automatically on probe failure
or device removal.

Remove the manual dev_pm_domain_detach_list() calls from the probe
error path and remove callback. These calls can operate on memory
that has already been freed by the devres-managed cleanup, resulting
in a potential double-free. Also remove the unused pm_domain.h header.

Suggested-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 3633723acef8..fe961b206645 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -20,7 +20,6 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/panic_notifier.h>
-#include <linux/pm_domain.h>
 #include <linux/pm_opp.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -2008,7 +2007,6 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
 		port->rx_dma_addr = 0;
 	}
-	dev_pm_domain_detach_list(port->se.pd_list);
 	return ret;
 }
 
@@ -2032,8 +2030,6 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
 				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
 		port->rx_dma_addr = 0;
 	}
-
-	dev_pm_domain_detach_list(port->se.pd_list);
 }
 
 static int __maybe_unused qcom_geni_serial_runtime_suspend(struct device *dev)

---
base-commit: 89c07d98716a13454ec3fd9f97689e812cc71bd4
change-id: 20260901-probe_cleanup_uart-60bb7d0df5ae

Best regards,
--  
Aniket Randive <aniket.randive@oss.qualcomm.com>


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

* Re: [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list
  2026-09-01  9:50 [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list Aniket Randive
@ 2026-09-01  9:53 ` Praveen Talari
  2026-09-01  9:54 ` Konrad Dybcio
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Praveen Talari @ 2026-09-01  9:53 UTC (permalink / raw)
  To: Aniket Randive, Greg Kroah-Hartman, Jiri Slaby
  Cc: konrad.dybcio, naresh.maramaina, linux-arm-msm, linux-kernel,
	linux-serial

Hi

On 01-09-2026 15:20, Aniket Randive wrote:
> The SA8255P UART variants attach PM domains through
> devm_pm_domain_attach_list(), which registers a devres cleanup action
> to detach and free the PM domain list automatically on probe failure
> or device removal.
>
> Remove the manual dev_pm_domain_detach_list() calls from the probe
> error path and remove callback. These calls can operate on memory
> that has already been freed by the devres-managed cleanup, resulting
> in a potential double-free. Also remove the unused pm_domain.h header.
>
> Suggested-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
>   drivers/tty/serial/qcom_geni_serial.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 3633723acef8..fe961b206645 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -20,7 +20,6 @@
>   #include <linux/module.h>
>   #include <linux/of.h>
>   #include <linux/panic_notifier.h>
> -#include <linux/pm_domain.h>
>   #include <linux/pm_opp.h>
>   #include <linux/platform_device.h>
>   #include <linux/pm_runtime.h>
> @@ -2008,7 +2007,6 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>   				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
>   		port->rx_dma_addr = 0;
>   	}
> -	dev_pm_domain_detach_list(port->se.pd_list);
>   	return ret;
>   }
>   
> @@ -2032,8 +2030,6 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
>   				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
>   		port->rx_dma_addr = 0;
>   	}
> -
> -	dev_pm_domain_detach_list(port->se.pd_list);
>   }
>   

Reviewed-by: Praveen Talari <praveen.talari@oss.qualcomm.com>

Thanks,

Praveen Talari

>   static int __maybe_unused qcom_geni_serial_runtime_suspend(struct device *dev)
>
> ---
> base-commit: 89c07d98716a13454ec3fd9f97689e812cc71bd4
> change-id: 20260901-probe_cleanup_uart-60bb7d0df5ae
>
> Best regards,
> --
> Aniket Randive <aniket.randive@oss.qualcomm.com>
>

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

* Re: [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list
  2026-09-01  9:50 [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list Aniket Randive
  2026-09-01  9:53 ` Praveen Talari
@ 2026-09-01  9:54 ` Konrad Dybcio
  2026-09-01 10:02 ` sashiko-bot
  2026-09-01 14:52 ` Abel Vesa
  3 siblings, 0 replies; 5+ messages in thread
From: Konrad Dybcio @ 2026-09-01  9:54 UTC (permalink / raw)
  To: Aniket Randive, Greg Kroah-Hartman, Jiri Slaby
  Cc: naresh.maramaina, praveen.talari, linux-arm-msm, linux-kernel,
	linux-serial

On 9/1/26 11:50 AM, Aniket Randive wrote:
> The SA8255P UART variants attach PM domains through
> devm_pm_domain_attach_list(), which registers a devres cleanup action
> to detach and free the PM domain list automatically on probe failure
> or device removal.
> 
> Remove the manual dev_pm_domain_detach_list() calls from the probe
> error path and remove callback. These calls can operate on memory
> that has already been freed by the devres-managed cleanup, resulting
> in a potential double-free. Also remove the unused pm_domain.h header.
> 
> Suggested-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

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

* Re: [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list
  2026-09-01  9:50 [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list Aniket Randive
  2026-09-01  9:53 ` Praveen Talari
  2026-09-01  9:54 ` Konrad Dybcio
@ 2026-09-01 10:02 ` sashiko-bot
  2026-09-01 14:52 ` Abel Vesa
  3 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-01 10:02 UTC (permalink / raw)
  To: Aniket Randive; +Cc: linux-serial

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Asynchronous pm_runtime_put() immediately followed by devres-managed pm_runtime_disable() on probe error path.
- [High] Probe error paths leak IDA slots and dedicated wake IRQs, and ida_free() crashes on console unbind.
--

commit 70e27637a93cd696a967e44f11424854b0e2411a
Author: Aniket Randive <aniket.randive@oss.qualcomm.com>

serial: qcom-geni: Avoid double-free of PM domain list

The SA8255P UART variants attach PM domains through
devm_pm_domain_attach_list(), which registers a devres cleanup action
to detach and free the PM domain list automatically on probe failure
or device removal.

> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 3633723acef8d..fe961b206645c 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c

[ ... ]

> @@ -2008,7 +2007,6 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
>  		port->rx_dma_addr = 0;
>  	}
> -	dev_pm_domain_detach_list(port->se.pd_list);
>  	return ret;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
qcom_geni_serial_probe() fail to release several resources?

If uart_add_one_port() or pm_runtime_resume_and_get() fail in probe, the
code jumps to the error label which completely fails to call ida_free(),
dev_pm_clear_wake_irq(), and device_init_wakeup(..., false).

Additionally, if devm_kzalloc() fails earlier in get_port_from_line() in
the same file, the allocated IDA slot is never freed, resulting in a
permanent leak of global IDA slots.

[Severity: High]
This is a pre-existing issue, but does the asynchronous pm_runtime_put()
on the probe error path leave the hardware permanently powered on?

drivers/tty/serial/qcom_geni_serial.c:qcom_geni_serial_probe() {
    ...
	ret = uart_add_one_port(drv, uport);
	pm_runtime_put(uport->dev);
	if (ret)
		goto error;
    ...
}

If uart_add_one_port() fails, pm_runtime_put() queues an asynchronous
suspend right before returning. Because this is an error path, devres
will immediately invoke pm_runtime_disable() which cancels the pending
asynchronous idle/suspend work, leaking power.

> @@ -2032,8 +2030,6 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
>  				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
>  		port->rx_dma_addr = 0;
>  	}
> -
> -	dev_pm_domain_detach_list(port->se.pd_list);
>  }

[Severity: High]
This is a pre-existing issue, but does the unconditional ida_free() in
qcom_geni_serial_remove() trigger a warning when unbinding console ports?

drivers/tty/serial/qcom_geni_serial.c:qcom_geni_serial_remove() {
    ...
	ida_free(&port_ida, uport->line);
    ...
}

When unbinding the driver for a console port, ida_free() is called on a
port line that was statically assigned, not allocated from the IDA,
which causes a WARN_ON_ONCE.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-probe_cleanup_uart-v1-1-0bc2b00c970e@oss.qualcomm.com?part=1

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

* Re: [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list
  2026-09-01  9:50 [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list Aniket Randive
                   ` (2 preceding siblings ...)
  2026-09-01 10:02 ` sashiko-bot
@ 2026-09-01 14:52 ` Abel Vesa
  3 siblings, 0 replies; 5+ messages in thread
From: Abel Vesa @ 2026-09-01 14:52 UTC (permalink / raw)
  To: Aniket Randive
  Cc: Greg Kroah-Hartman, Jiri Slaby, konrad.dybcio, naresh.maramaina,
	praveen.talari, linux-arm-msm, linux-kernel, linux-serial

On 26-09-01 15:20:54, Aniket Randive wrote:
> The SA8255P UART variants attach PM domains through
> devm_pm_domain_attach_list(), which registers a devres cleanup action
> to detach and free the PM domain list automatically on probe failure
> or device removal.
> 
> Remove the manual dev_pm_domain_detach_list() calls from the probe
> error path and remove callback. These calls can operate on memory
> that has already been freed by the devres-managed cleanup, resulting
> in a potential double-free. Also remove the unused pm_domain.h header.
> 
> Suggested-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>

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

end of thread, other threads:[~2026-09-01 14:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01  9:50 [PATCH v1] serial: qcom-geni: Avoid double-free of PM domain list Aniket Randive
2026-09-01  9:53 ` Praveen Talari
2026-09-01  9:54 ` Konrad Dybcio
2026-09-01 10:02 ` sashiko-bot
2026-09-01 14:52 ` Abel Vesa

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