Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Nullifying `regmap` in `rtk_clk_remove` introduces a race condition that leads to a NULL pointer dereference if the driver is unbound via sysfs.
--
--- Patch [9]: [PATCH v12 09/11] clk: realtek: Add RTD1625-CRT clock controller driver ---
commit 9b8b892d0fe2cab055b1375b43022988ae2354a2
Author: Cheng-Yu Lee <[email protected]>
clk: realtek: Add RTD1625-CRT clock controller driver
Add support for the CRT (Clock, Reset, and Test) domain clock controller
on the Realtek RTD1625 SoC. This driver acts as the parent device and
registers the reset controller as an auxiliary device on the auxiliary bus.
> diff --git a/drivers/clk/realtek/clk-rtd1625-crt.c b/drivers/clk/realtek/clk-rtd1625-crt.c
> new file mode 100644
> index 0000000000000..982ef73785ac5
> --- /dev/null
> +++ b/drivers/clk/realtek/clk-rtd1625-crt.c
[ ... ]
> +static struct platform_driver rtd1625_crt_driver = {
> + .probe = rtd1625_crt_probe,
> + .remove = rtk_clk_remove,
[Severity: High]
Does adding this remove callback and allowing sysfs unbind expose a NULL
pointer dereference race condition?
Because the clocks are registered using devm_clk_hw_register() in
rtk_clk_probe(), they are unregistered during devres cleanup, which
happens after rtk_clk_remove() completes.
Looking at rtk_clk_remove() in clk-rtk-common.c, it explicitly sets the
regmap to NULL:
for (int i = 0; i < desc->num_clks; i++)
desc->clks[i]->regmap = NULL;
If a concurrent clock operation occurs (like reading the debugfs
clk_summary) between rtk_clk_remove() returning and the devres cleanup
finishing, will CCF invoke the clock callbacks with a NULL regmap and
cause a kernel panic in regmap_read()?
> + .driver = {
> + .name = "rtk-rtd1625-crt-clk",
> + .of_match_table = rtd1625_crt_match,
> + },
> +};
> +
> +module_platform_driver(rtd1625_crt_driver)
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
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.