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

[PATCH v2] 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.

Route resources_init() failures through the common error path so the
allocated line number is released correctly.

Allocate the port structure before calling ida_alloc_range() in
get_port_from_line(). This prevents IDs from being left allocated when
memory allocation fails.

Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
Changes in v2:
- Route port->dev_data->resources_init() failures through the common
error path so allocated line numbers are not leaked.
- Allocate the port structure before ida_alloc_range() in
get_port_from_line() to prevent ID leaks on memory-allocation
failures.
---
Link to v1: https://patch.msgid.link/20260828-master-v1-1-1f8afa5f82c2@oss.qualcomm.com
---
 drivers/tty/serial/qcom_geni_serial.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 3633723acef8..ceaf560fd8dd 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -289,6 +289,10 @@ static struct qcom_geni_serial_port *get_port_from_line(int line, bool console,
 	} else {
 		int max_alias_num = of_alias_get_highest_id("serial");
 
+		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+		if (!port)
+			return ERR_PTR(-ENOMEM);
+
 		if (line < 0 || line >= nr_ports)
 			line = ida_alloc_range(&port_ida, max_alias_num + 1,
 					       nr_ports - 1, GFP_KERNEL);
@@ -299,10 +303,6 @@ static struct qcom_geni_serial_port *get_port_from_line(int line, bool console,
 		if (line < 0)
 			return ERR_PTR(-ENXIO);
 
-		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
-		if (!port)
-			return ERR_PTR(-ENOMEM);
-
 		port->uport.iotype = UPIO_MEM;
 		port->uport.ops = &qcom_geni_uart_pops;
 		port->uport.flags = UPF_BOOT_AUTOCONF;
@@ -1897,7 +1897,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 
 	ret = port->dev_data->resources_init(&port->se);
 	if (ret)
-		return ret;
+		goto error;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -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 v2] serial: qcom-geni: Fix port_ida handling for console and probe errors

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Manual detachment of the devres-managed PM domain list during probe error handling and device removal causes a double-free.
- [High] The driver incorrectly calls `dma_unmap_single()` with a DMA mapping error address in the probe error path.
- [High] The driver frees the port IDA before unregistering the UART port, exposing a race condition.
- [High] Unconditional probe failure if no serial aliases exist in the Device Tree.
--

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

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

This commit ensures the console port line number is not allocated from or
freed back to port_ida. It also fixes leaks by moving the ida_free() call
to the common probe error path and allocating the port structure before
allocating an ID.

Link: https://patch.msgid.link/20260828-master-v1-1-1f8afa5f82c2@oss.qualcomm.com

> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 3633723acef8d..ceaf560fd8dda 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -289,6 +289,10 @@ static struct qcom_geni_serial_port *get_port_from_line(int line, bool console,
>  	} else {
>  		int max_alias_num = of_alias_get_highest_id("serial");
>  
> +		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> +		if (!port)
> +			return ERR_PTR(-ENOMEM);
> +
>  		if (line < 0 || line >= nr_ports)
>  			line = ida_alloc_range(&port_ida, max_alias_num + 1,
>  					       nr_ports - 1, GFP_KERNEL);

[Severity: High]
This is a pre-existing issue, but does this unconditionally fail the probe
if there are no serial aliases in the Device Tree?

If of_alias_get_highest_id(serial) finds no aliases and returns -ENODEV
(-19), the minimum value passed to ida_alloc_range() becomes -18. Since
ida_alloc_range() performs a signedness bounds check ((int)min < 0), it
immediately returns -ENOSPC, causing the probe to fail on systems without
DT aliases.

[ ... ]

> @@ -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 does this erroneously unmap an invalid
DMA address if dma_map_single() fails?

If dma_map_single() fails during probe, it returns DMA_MAPPING_ERROR
(usually ~0ULL) which is non-zero. The failure path directly jumps to the
error label without clearing port->rx_dma_addr, so this check considers it
valid and calls dma_unmap_single() on the error address, potentially causing
IOMMU faults or memory corruption.

[Severity: Critical]
This is a pre-existing issue, but does the manual detachment of the
devres-managed PM domain list here in the probe error path cause a double-free?

Just below in the error path, dev_pm_domain_detach_list(port->se.pd_list)
is called. Since pd_list was allocated and attached via
devm_pm_domain_attach_list(), the devres subsystem will automatically free and
detach it on probe failure. Manually calling dev_pm_domain_detach_list()
results in kfree being called twice on the same pointer, leading to heap
corruption and a kernel panic.

> @@ -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);

[Severity: High]
This is a pre-existing issue, but does freeing the port IDA before
unregistering the UART port expose a TOCTOU race for the line ID?

If qcom_geni_serial_remove() frees the line IDA while the UART port is still
registered in the TTY layer, a concurrent probe of another UART device could
allocate the exact same line ID and call uart_add_one_port(), resulting in
two active driver instances sharing the same TTY port line in the serial core.

[Severity: Critical]
This is a pre-existing issue, but does the manual detachment of the
devres-managed PM domain list in this remove function cause a double-free?

Further down in qcom_geni_serial_remove(), the driver calls
dev_pm_domain_detach_list(port->se.pd_list). Since pd_list is devm-managed,
the devres subsystem will automatically handle its cleanup when the device is
removed. Manually detaching it here causes a double-free of the pd_list memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-master-v2-1-0032ec7fd1ca@oss.qualcomm.com?part=1

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

[Konrad Dybcio]

On 8/31/26 1:12 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.
> 
> Route resources_init() failures through the common error path so the
> allocated line number is released correctly.
> 
> Allocate the port structure before calling ida_alloc_range() in
> get_port_from_line(). This prevents IDs from being left allocated when
> memory allocation fails.
> 
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
> Changes in v2:
> - Route port->dev_data->resources_init() failures through the common
> error path so allocated line numbers are not leaked.

I asked GPT what it thinks again and this time it found that this
will cause a double calling of dev_pm_domain_detach_list(), which
will mess up the refcounting..

I think the fix here is to drop the manual detaches since the common
helper uses a devres-managed version of attach.

Konrad

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

[Aniket RANDIVE]

On 8/31/2026 5:08 PM, Konrad Dybcio wrote:
> On 8/31/26 1:12 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.
>>
>> Route resources_init() failures through the common error path so the
>> allocated line number is released correctly.
>>
>> Allocate the port structure before calling ida_alloc_range() in
>> get_port_from_line(). This prevents IDs from being left allocated when
>> memory allocation fails.
>>
>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>> ---
>> Changes in v2:
>> - Route port->dev_data->resources_init() failures through the common
>> error path so allocated line numbers are not leaked.
> 
> I asked GPT what it thinks again and this time it found that this
> will cause a double calling of dev_pm_domain_detach_list(), which
> will mess up the refcounting..
> 
> I think the fix here is to drop the manual detaches since the common
> helper uses a devres-managed version of attach.
> 

Thanks for catching this.

Looking closer, this appears to be a separate issue from the port_ida 
fixes in this patch. The dev_pm_domain_detach_list() problem already 
exists in the current error path and is not introduced by this change.

Since this involves power-domain cleanup rather than port_ida handling, 
I'd prefer to address it in a separate follow-up patch to keep this fix 
focused and easier to review.

Is it OK to keep this patch limited to the port_ida fixes and handle the 
power-domain cleanup separately?

Thanks,
Aniket

> Konrad

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

[Praveen Talari]

Hi Aniket,

On 31-08-2026 21:29, Aniket RANDIVE wrote:
>
>
> On 8/31/2026 5:08 PM, Konrad Dybcio wrote:
>> On 8/31/26 1:12 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.
>>>
>>> Route resources_init() failures through the common error path so the
>>> allocated line number is released correctly.
>>>
>>> Allocate the port structure before calling ida_alloc_range() in
>>> get_port_from_line(). This prevents IDs from being left allocated when
>>> memory allocation fails.
>>>
>>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>>> ---
>>> Changes in v2:
>>> - Route port->dev_data->resources_init() failures through the common
>>> error path so allocated line numbers are not leaked.
>>
>> I asked GPT what it thinks again and this time it found that this
>> will cause a double calling of dev_pm_domain_detach_list(), which
>> will mess up the refcounting..
>>
>> I think the fix here is to drop the manual detaches since the common
>> helper uses a devres-managed version of attach.
>>
>
> Thanks for catching this.
>
> Looking closer, this appears to be a separate issue from the port_ida 
> fixes in this patch. The dev_pm_domain_detach_list() problem already 
> exists in the current error path and is not introduced by this change.
>
> Since this involves power-domain cleanup rather than port_ida 
> handling, I'd prefer to address it in a separate follow-up patch to 
> keep this fix focused and easier to review.
>
> Is it OK to keep this patch limited to the port_ida fixes and handle 
> the power-domain cleanup separately?
That should be separate patch.

For this patch.

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

Thanks,

Praveen Talari
>
> Thanks,
> Aniket
>
>> Konrad
>
>

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

[Konrad Dybcio]

On 8/31/26 5:59 PM, Aniket RANDIVE wrote:
> 
> 
> On 8/31/2026 5:08 PM, Konrad Dybcio wrote:
>> On 8/31/26 1:12 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.

[...]

> Thanks for catching this.
> 
> Looking closer, this appears to be a separate issue from the port_ida fixes in this patch. The dev_pm_domain_detach_list() problem already exists in the current error path and is not introduced by this change.
> 
> Since this involves power-domain cleanup rather than port_ida handling, I'd prefer to address it in a separate follow-up patch to keep this fix focused and easier to review.
> 
> Is it OK to keep this patch limited to the port_ida fixes and handle the power-domain cleanup separately?

Yes

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

Konrad