Re: [PATCH 4/8] clk: clocking-wizard: Do not map the memory for static-config

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Registering the same struct notifier_block to two different clock notifiers corrupts the intrusive linked lists.
--

commit 8b915ba31e389cdcf5a743f96eb5c3695bc448c3
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

clk: clocking-wizard: Do not map the memory for static-config

This patch prevents the memory mapping of the AXI register block if the
device is configured with xlnx,static-config. It achieves this by moving
devm_platform_ioremap_resource() under the non-static-config conditional
branch, since only the dynamic path requires the AXI register block.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index 4a0136349f71a..e082051221bef 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c

[ ... ]

> @@ -1183,6 +1179,10 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  	}
>  
>  	if (!of_property_present(np, "xlnx,static-config")) {
> +		clk_wzrd->base = devm_platform_ioremap_resource(pdev, 0);
> +		if (IS_ERR(clk_wzrd->base))
> +			return PTR_ERR(clk_wzrd->base);
> +
>  		ret = of_property_read_u32(np, "xlnx,speed-grade", &clk_wzrd->speed_grade);
>  		if (!ret) {
>  			if (clk_wzrd->speed_grade < 1 || clk_wzrd->speed_grade > 3) {

[ ... ]

> 		if (clk_wzrd->speed_grade) {
> 			clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;
> 
> 			ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->clk_in1,
> 							 &clk_wzrd->nb);
> 			if (ret)
> 				dev_warn(&pdev->dev,
> 					 "unable to register clock notifier\n");
> 
> 			ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->axi_clk,
> 							 &clk_wzrd->nb);

[Severity: High]
This isn't a bug introduced by this patch, but it is a pre-existing issue.

Does registering the same struct notifier_block (&clk_wzrd->nb) to two
different clocks in clk_wzrd_probe() corrupt the SRCU notifier chains?

The clock notifier framework uses srcu_notifier_chain_register(), which
links the provided struct notifier_block into an intrusive linked list by
modifying its next pointer. Because clk_in1 and axi_clk are distinct clocks
with separate notifier chains, adding the exact same notifier_block instance
to the second chain overwrites its next pointer.

Could this merge the two chains, causing events from the first clock to
traverse into the second clock's chain? Furthermore, when the notifiers are
unregistered, would the unregistration logic leave the first clock's chain
pointing to elements from the second clock's chain, leading to a potential
use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615034845.3320286-1-shubhrajyoti.datta@amd.com?part=4