[PATCH v1] serial: qcom-geni: Fix port_ida handling for console and probe errors

[PATCH v1] serial: qcom-geni: Fix port_ida handling for console and probe errors

[Aniket Randive]

The console port uses a fixed line number and is not allocated from
port_ida. However, qcom_geni_serial_remove() unconditionally frees the
line number for all ports, including the console port.

Skip ida_free() for console ports so that only IDs allocated from
port_ida are returned.

Also release allocated IDs from the common probe error path. Currently,
IDs are freed only on the wake-IRQ failure path, causing leaks when
probe fails after a successful allocation. Move the ida_free() call to
the common error path and guard it for console ports.

Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
 drivers/tty/serial/qcom_geni_serial.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 3633723acef8..3bfe597d2c83 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1979,7 +1979,6 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 						port->wakeup_irq);
 		if (ret) {
 			device_init_wakeup(&pdev->dev, false);
-			ida_free(&port_ida, uport->line);
 			goto error;
 		}
 	}
@@ -2003,6 +2002,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 	return 0;
 
 error:
+	if (!data->console)
+		ida_free(&port_ida, uport->line);
 	if (port->rx_dma_addr) {
 		dma_unmap_single(pdev->dev.parent, port->rx_dma_addr,
 				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
@@ -2024,7 +2025,8 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
 	irq_work_sync(&port->tx_kick);
 	dev_pm_clear_wake_irq(&pdev->dev);
 	device_init_wakeup(&pdev->dev, false);
-	ida_free(&port_ida, uport->line);
+	if (!port->dev_data->console)
+		ida_free(&port_ida, uport->line);
 	uart_remove_one_port(drv, &port->uport);
 
 	if (port->rx_dma_addr) {

---
base-commit: a8406e6c0b793ce0788019683837c40855b55995
change-id: 20260828-master-3e806ba3492d

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

Re: [PATCH v1] serial: qcom-geni: Fix port_ida handling for console and probe errors

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Manual detachment of PM domains that were managed by devres causes use-after-free and double-free in the error and remove paths.
- [High] `dma_unmap_single()` is called on an invalid DMA error address in the probe error path.
--

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

serial: qcom-geni: Fix port_ida handling for console and probe errors

The console port uses a fixed line number and is not allocated from
port_ida. However, qcom_geni_serial_remove() unconditionally frees the
line number for all ports, including the console port.

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

[ ... ]

> @@ -2003,6 +2002,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  	return 0;
>  
>  error:
> +	if (!data->console)
> +		ida_free(&port_ida, uport->line);
>  	if (port->rx_dma_addr) {
>  		dma_unmap_single(pdev->dev.parent, port->rx_dma_addr,
>  				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);

[Severity: High]
This is a pre-existing issue, but could this dma_unmap_single() call in
qcom_geni_serial_probe() be operating on an invalid DMA error address?

If dma_map_single() fails earlier in qcom_geni_serial_probe() for rx_buf, it
returns a non-zero mapping error address. Since that error address is
non-zero, this check for (port->rx_dma_addr) evaluates to true, and
dma_unmap_single() is called on the invalid address.

[Severity: Critical]
This is a pre-existing issue, but does manually calling
dev_pm_domain_detach_list() right after this block cause a double-free?

Just below this hunk in the qcom_geni_serial_probe() error path, the code
calls dev_pm_domain_detach_list(port->se.pd_list).

Since the PM domains are attached using devres-managed functions (via
port->dev_data->resources_init()), the devres cleanup will automatically
invoke devm_pm_domain_detach_list() when probe fails. Manually detaching it
in the error path would free the list, leading to a second free during the
automatic devres cleanup.

> @@ -2024,7 +2025,8 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
>  	irq_work_sync(&port->tx_kick);
>  	dev_pm_clear_wake_irq(&pdev->dev);
>  	device_init_wakeup(&pdev->dev, false);
> -	ida_free(&port_ida, uport->line);
> +	if (!port->dev_data->console)
> +		ida_free(&port_ida, uport->line);
>  	uart_remove_one_port(drv, &port->uport);
>  
>  	if (port->rx_dma_addr) {

[Severity: Critical]
This is a pre-existing issue, but does manually calling
dev_pm_domain_detach_list() in qcom_geni_serial_remove() cause a double-free?

Similar to the probe error path, just below this hunk in
qcom_geni_serial_remove(), the code calls
dev_pm_domain_detach_list(port->se.pd_list).

Since the PM domains were attached using devres-managed functions, the
devres cleanup will automatically invoke devm_pm_domain_detach_list() when
the device is removed. Manually detaching it here would free the list,
leading to a second free during the automatic devres cleanup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-master-v1-1-1f8afa5f82c2@oss.qualcomm.com?part=1

Re: [PATCH v1] serial: qcom-geni: Fix port_ida handling for console and probe errors

[Konrad Dybcio]

On 8/28/26 1:14 PM, Aniket Randive wrote:
> The console port uses a fixed line number and is not allocated from
> port_ida. However, qcom_geni_serial_remove() unconditionally frees the
> line number for all ports, including the console port.
> 
> Skip ida_free() for console ports so that only IDs allocated from
> port_ida are returned.
> 
> Also release allocated IDs from the common probe error path. Currently,
> IDs are freed only on the wake-IRQ failure path, causing leaks when
> probe fails after a successful allocation. Move the ida_free() call to
> the common error path and guard it for console ports.
> 
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 3633723acef8..3bfe597d2c83 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1979,7 +1979,6 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  						port->wakeup_irq);
>  		if (ret) {
>  			device_init_wakeup(&pdev->dev, false);
> -			ida_free(&port_ida, uport->line);
>  			goto error;
>  		}
>  	}

GPT suggested that the failure of

ret = port->dev_data->resources_init(&port->se);

should also be updated to have a 'goto error' rather than a direct
return


also that in get_port_from_line(), reordering the devm_kzalloc before
ida_alloc_range() would prevent another leak

Konrad

Re: [PATCH v1] serial: qcom-geni: Fix port_ida handling for console and probe errors

[Aniket RANDIVE]

Thanks Konrad for the review.

On 8/31/2026 12:58 PM, Konrad Dybcio wrote:
> On 8/28/26 1:14 PM, Aniket Randive wrote:
>> The console port uses a fixed line number and is not allocated from
>> port_ida. However, qcom_geni_serial_remove() unconditionally frees the
>> line number for all ports, including the console port.
>>
>> Skip ida_free() for console ports so that only IDs allocated from
>> port_ida are returned.
>>
>> Also release allocated IDs from the common probe error path. Currently,
>> IDs are freed only on the wake-IRQ failure path, causing leaks when
>> probe fails after a successful allocation. Move the ida_free() call to
>> the common error path and guard it for console ports.
>>
>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>> ---
>>   drivers/tty/serial/qcom_geni_serial.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
>> index 3633723acef8..3bfe597d2c83 100644
>> --- a/drivers/tty/serial/qcom_geni_serial.c
>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>> @@ -1979,7 +1979,6 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>>   						port->wakeup_irq);
>>   		if (ret) {
>>   			device_init_wakeup(&pdev->dev, false);
>> -			ida_free(&port_ida, uport->line);
>>   			goto error;
>>   		}
>>   	}
> 
> GPT suggested that the failure of
> 
> ret = port->dev_data->resources_init(&port->se);
> 
> should also be updated to have a 'goto error' rather than a direct
> return

Good catch. I will fix in next V2 patch. I will route through "error" 
now so it goes through the same ida_free() as every other failure path 
instead of skipping it.

> 
> 
> also that in get_port_from_line(), reordering the devm_kzalloc before
> ida_alloc_range() would prevent another leak

Agree. I will reorder it.

> 
> Konrad