Re: [PATCH v2 3/9] soc: qcom: geni-se: Add helper to set SE clock rate via OPP

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Double free and use-after-free of `port->se.pd_list` via devres cleanup in `qcom_geni_serial_probe` error path and `qcom_geni_serial_remove`.
--

commit 6ff6aaccb1d04a9b68e796b58d89553fda0f613b
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

soc: qcom: geni-se: Add helper to set SE clock rate via OPP

GENI protocol drivers need a common way to scale the SE source clock
through the OPP framework. The new helper automatically selects the
performance-domain device when power domains are attached and falls back
to the SE device otherwise.

> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 447b9cc6babf6..04f58a6f26fff 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -1149,6 +1149,28 @@ int geni_se_set_perf_opp(struct geni_se *se, unsigned long clk_freq)
>  }
>  EXPORT_SYMBOL_GPL(geni_se_set_perf_opp);
>  
[ ... ]

[Severity: Critical]
This isn't a bug introduced by this patch, but could the manual detach calls
in drivers/tty/serial/qcom_geni_serial.c result in a double free and
use-after-free?

In qcom_geni_serial_probe(), the code calls port->dev_data->resources_init().
For configurations that map this to geni_se_domain_attach(), it allocates and
attaches power domains using the managed API devm_pm_domain_attach_list().
This registers a cleanup action that will automatically detach and free the
list on probe failure or driver unbind.

However, the error path manually frees the list:

drivers/tty/serial/qcom_geni_serial.c:qcom_geni_serial_probe()
	...
error:
	if (port->rx_dma_addr) {
		dma_unmap_single(pdev->dev.parent, port->rx_dma_addr,
				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
		port->rx_dma_addr = 0;
	}
	dev_pm_domain_detach_list(port->se.pd_list);
	return ret;
}

The manual call to dev_pm_domain_detach_list() immediately frees the list,
but since the manual detach bypasses devres, the devres subsystem will
automatically invoke its cleanup action on the exact same, already-freed
pointer when the probe returns an error.

The same manual detach is also present in qcom_geni_serial_remove():

drivers/tty/serial/qcom_geni_serial.c:qcom_geni_serial_remove()
	...
	if (port->rx_dma_addr) {
		dma_unmap_single(pdev->dev.parent, port->rx_dma_addr,
				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
		port->rx_dma_addr = 0;
	}

	dev_pm_domain_detach_list(port->se.pd_list);
}

Could this cause memory corruption or a kernel panic when devres double-frees
the list as soon as the remove function finishes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-derive_clk_perf_tbl_from_perf_domain_opp_table-v2-0-091697dbeb02@oss.qualcomm.com?part=3