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

[email protected]
Newsgroups gmane.linux.serial
Message-ID <[email protected]>
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 <[email protected]>

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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.