[PATCH] clk: tenstorrent: Assign .num before accessing .hws

[PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Aamir Ahmed]

Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with
__counted_by") annotated the hws member of 'struct clk_hw_onecell_data'
with __counted_by, which informs the bounds sanitizer (UBSAN_BOUNDS)
about the number of elements in .hws[], so that it can warn when .hws[]
is accessed out of bounds. As noted in that change, the __counted_by
member must be initialized with the number of elements before the first
array access happens, otherwise there will be a warning from each access
prior to the initialization because the number of elements is zero.
This occurs in atlantis_prcm_clocks_register() due to .num being
assigned only after every clock has been stored in .hws[]. With
CONFIG_UBSAN_BOUNDS and a compiler that implements __counted_by (GCC
15.1+ or Clang 20.1+), this triggers an array-index-out-of-bounds report
during probe, and with CONFIG_UBSAN_TRAP the first store traps so the
clock provider never registers.

Move the .num initialization to right after the allocation.

Cc: stable@vger.kernel.org
Fixes: 23c8ebc95284 ("clk: tenstorrent: Add Atlantis clock controller driver")
Assisted-by: LLM
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---
Found while auditing the remaining clk_hw_onecell_data users that assign
.num only after touching .hws[], following the fixes already merged for
clk-s2mps11 (3e14c7207a97), exynos-clkout (cf33f0b7df13) and
clk-raspberrypi (6dc445c19050). The audit, the fix and this changelog
were drafted with an LLM assistant and reviewed by hand.

Compile-tested only (W=1, no warnings) on x86_64 with GCC 13.3, via
COMPILE_TEST with CONFIG_TENSTORRENT_ATLANTIS_PRCM=m. GCC 13.3 does not
implement __counted_by (CC_HAS_COUNTED_BY needs GCC 15.1+ or Clang
20.1+), so the build only confirms that the change compiles; the
sanitizer path was not exercised. I do not have the hardware, so this is
not runtime-tested and no UBSAN report was captured.

Based on v7.3-rc1.

 drivers/clk/tenstorrent/atlantis-prcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/tenstorrent/atlantis-prcm.c b/drivers/clk/tenstorrent/atlantis-prcm.c
index 6d4386eeb7d..a68534a295c 100644
--- a/drivers/clk/tenstorrent/atlantis-prcm.c
+++ b/drivers/clk/tenstorrent/atlantis-prcm.c
@@ -796,6 +796,8 @@ static int atlantis_prcm_clocks_register(struct device *dev,
 	if (!clk_data)
 		return -ENOMEM;
 
+	clk_data->num = num_clks;
+
 	for (i = 0; i < data->num; i++) {
 		struct clk_hw *hw = data->hws[i];
 		struct atlantis_clk_common *common =
@@ -809,8 +811,6 @@ static int atlantis_prcm_clocks_register(struct device *dev,
 		clk_data->hws[common->clkid] = hw;
 	}
 
-	clk_data->num = num_clks;
-
 	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
 }
 

base-commit: 654ae5d73c05bd2943d65636ce6cd0aa46e62f18
-- 
2.53.0.windows.1

[PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Aamir Ahmed]

Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with
__counted_by") annotated the hws member of 'struct clk_hw_onecell_data'
with __counted_by, which informs the bounds sanitizer (UBSAN_BOUNDS)
about the number of elements in .hws[], so that it can warn when .hws[]
is accessed out of bounds. As noted in that change, the __counted_by
member must be initialized with the number of elements before the first
array access happens, otherwise there will be a warning from each access
prior to the initialization because the number of elements is zero.
This occurs in atlantis_prcm_clocks_register() due to .num being
assigned only after every clock has been stored in .hws[]. With
CONFIG_UBSAN_BOUNDS and a compiler that implements __counted_by (GCC
15.1+ or Clang 20.1+), this triggers an array-index-out-of-bounds report
during probe, and with CONFIG_UBSAN_TRAP the first store traps so the
clock provider never registers.

Move the .num initialization to right after the allocation.

Cc: stable@vger.kernel.org
Fixes: 23c8ebc95284 ("clk: tenstorrent: Add Atlantis clock controller driver")
Assisted-by: LLM
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---
Found while auditing the remaining clk_hw_onecell_data users that assign
.num only after touching .hws[], following the fixes already merged for
clk-s2mps11 (3e14c7207a97), exynos-clkout (cf33f0b7df13) and
clk-raspberrypi (6dc445c19050). The audit, the fix and this changelog
were drafted with an LLM assistant and reviewed by hand.

Compile-tested only (W=1, no warnings) on x86_64 with GCC 13.3, via
COMPILE_TEST with CONFIG_TENSTORRENT_ATLANTIS_PRCM=m. GCC 13.3 does not
implement __counted_by (CC_HAS_COUNTED_BY needs GCC 15.1+ or Clang
20.1+), so the build only confirms that the change compiles; the
sanitizer path was not exercised. I do not have the hardware, so this is
not runtime-tested and no UBSAN report was captured.

Based on v7.3-rc1.

 drivers/clk/tenstorrent/atlantis-prcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/tenstorrent/atlantis-prcm.c b/drivers/clk/tenstorrent/atlantis-prcm.c
index 6d4386eeb7d..a68534a295c 100644
--- a/drivers/clk/tenstorrent/atlantis-prcm.c
+++ b/drivers/clk/tenstorrent/atlantis-prcm.c
@@ -796,6 +796,8 @@ static int atlantis_prcm_clocks_register(struct device *dev,
 	if (!clk_data)
 		return -ENOMEM;
 
+	clk_data->num = num_clks;
+
 	for (i = 0; i < data->num; i++) {
 		struct clk_hw *hw = data->hws[i];
 		struct atlantis_clk_common *common =
@@ -809,8 +811,6 @@ static int atlantis_prcm_clocks_register(struct device *dev,
 		clk_data->hws[common->clkid] = hw;
 	}
 
-	clk_data->num = num_clks;
-
 	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
 }
 

base-commit: 654ae5d73c05bd2943d65636ce6cd0aa46e62f18
-- 
2.53.0.windows.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing validation of the match data pointer causes a direct NULL pointer dereference on probe.
- [High] Shared clock gate refcount is not rolled back when hardware enable fails, leading to silent hardware enablement failures.
- [High] PLL disable routine fails to close the downstream clock gate, continuing to leak a bypass clock signal.
--

commit f278096bfd5e9454d658c8bb4c16b0c59b91f633
Author: Aamir Ahmed <elb12345@hotmail.co.uk>

clk: tenstorrent: Assign .num before accessing .hws

Commit f316cdff8d67 annotated the hws member of struct clk_hw_onecell_data
with __counted_by, which informs bounds sanitizers about the number of
elements in .hws[]. This commit fixes a warning during probe by initializing
the .num member with the number of elements before the first array access
happens.

> diff --git a/drivers/clk/tenstorrent/atlantis-prcm.c b/drivers/clk/tenstorrent/atlantis-prcm.c
> index 6d4386eeb7dae..a68534a295ca1 100644
> --- a/drivers/clk/tenstorrent/atlantis-prcm.c
> +++ b/drivers/clk/tenstorrent/atlantis-prcm.c
> @@ -796,6 +796,8 @@ static int atlantis_prcm_clocks_register(struct device *dev,
>  	if (!clk_data)
>  		return -ENOMEM;
>  
> +	clk_data->num = num_clks;
> +
>  	for (i = 0; i < data->num; i++) {

[Severity: High]
This isn't a bug introduced by this patch, but can data be NULL here?

If the device is bound manually via sysfs without a matching device tree
node, of_device_get_match_data() can return NULL in atlantis_prcm_probe():

drivers/clk/tenstorrent/atlantis-prcm.c:atlantis_prcm_probe() {
    ...
	data = of_device_get_match_data(dev);

	ret = atlantis_prcm_clocks_register(dev, regmap, data);
    ...
}

Which is then passed into atlantis_prcm_clocks_register() and dereferenced:

drivers/clk/tenstorrent/atlantis-prcm.c:atlantis_prcm_clocks_register() {
    ...
	size_t num_clks = data->num;
    ...
}

Should data be validated before dereferencing it to avoid a NULL pointer
dereference?

>  		struct clk_hw *hw = data->hws[i];
>  		struct atlantis_clk_common *common =
>  			hw_to_atlantis_clk_common(hw);
>  		common->regmap = regmap;
>  
>  		ret = devm_clk_hw_register(dev, hw);
>  		if (ret)
>  			return ret;
>  
>  		clk_data->hws[common->clkid] = hw;
>  	}
>  
> -	clk_data->num = num_clks;
> -
>  	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
>  }

[Severity: High]
This is a pre-existing issue, but does this code leave the shared gate
refcount incremented if the hardware enable fails?

drivers/clk/tenstorrent/atlantis-prcm.c:atlantis_clk_gate_shared_enable() {
    ...
	if (need_enable) {
		if (!regmap_test_bits(gate->common.regmap,
				      gate->config.reg_offset,
				      gate->config.enable)) {
			pr_warn("%s: gate enable %d failed to enable\n",
				clk_hw_get_name(hw), gate->config.enable);
			return -EIO;
		}
	}
    ...
}

If regmap_test_bits() detects that the hardware bit failed to set, it
returns -EIO but leaves gate->config.share_count incremented. If the
consumer retries enabling the clock, the driver sees the refcount > 0, skips
the hardware write, and falsely returns success (0).

Should the error path re-acquire the lock and roll back the share_count?


[Severity: High]
This is also a pre-existing issue, but does the PLL disable routine fail to
close the downstream clock gate?

drivers/clk/tenstorrent/atlantis-prcm.c:atlantis_clk_pll_enable() {
    ...
	/* Enable Gate clk at PLL Output */
	return regmap_update_bits(pll->common.regmap, pll->config.cg_reg_offset,
				  pll->config.cg_reg_enable,
				  pll->config.cg_reg_enable);
}

drivers/clk/tenstorrent/atlantis-prcm.c:atlantis_clk_pll_disable() {
    ...
	/* Step 1: Switch to bypass mode before disabling */
	regmap_update_bits(pll->common.regmap, pll->config.reg_offset,
			   PLL_CFG_BYPASS_BIT, PLL_CFG_BYPASS_BIT);
	/* Step 2: Power down PLL */
	regmap_update_bits(pll->common.regmap, pll->config.en_reg_offset,
			   PLL_CFG_EN_BIT, 0);
}

When a consumer calls clk_disable() on the PLL, the driver switches to
bypass and clears the enable bit, but fails to clear
pll->config.cg_reg_enable symmetrically with atlantis_clk_pll_enable().

Because disabling the PLL also switches it to bypass mode, does the raw
bypass clock continuously propagate to downstream consumers that expect the
clock to be fully stopped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/AS8P251MB00013BD9A093C06D2D60E10EC8B42@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM?part=1

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Gustavo A. R. Silva]

On 9/6/26 05:48, Aamir Ahmed wrote:
> Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with
> __counted_by") annotated the hws member of 'struct clk_hw_onecell_data'
> with __counted_by, which informs the bounds sanitizer (UBSAN_BOUNDS)
> about the number of elements in .hws[], so that it can warn when .hws[]
> is accessed out of bounds. As noted in that change, the __counted_by
> member must be initialized with the number of elements before the first
> array access happens, otherwise there will be a warning from each access
> prior to the initialization because the number of elements is zero.
> This occurs in atlantis_prcm_clocks_register() due to .num being
> assigned only after every clock has been stored in .hws[]. With
> CONFIG_UBSAN_BOUNDS and a compiler that implements __counted_by (GCC
> 15.1+ or Clang 20.1+), this triggers an array-index-out-of-bounds report
> during probe, and with CONFIG_UBSAN_TRAP the first store traps so the
> clock provider never registers.
> 
> Move the .num initialization to right after the allocation.
> 
> Cc: stable@vger.kernel.org
> Fixes: 23c8ebc95284 ("clk: tenstorrent: Add Atlantis clock controller driver")
> Assisted-by: LLM
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>

LGTM:

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Also, see more comments below...

> diff --git a/drivers/clk/tenstorrent/atlantis-prcm.c b/drivers/clk/tenstorrent/atlantis-prcm.c
> index 6d4386eeb7d..a68534a295c 100644
> --- a/drivers/clk/tenstorrent/atlantis-prcm.c
> +++ b/drivers/clk/tenstorrent/atlantis-prcm.c
> @@ -796,6 +796,8 @@ static int atlantis_prcm_clocks_register(struct device *dev,
>   	if (!clk_data)
>   		return -ENOMEM;
>   
> +	clk_data->num = num_clks;

Looks like variable num_clks can be removed entirely, and just do:

clk_data->num = data->num;

Thanks
-Gustavo

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Gustavo A. R. Silva]

On 9/6/26 05:48, Aamir Ahmed wrote:
> Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with
> __counted_by") annotated the hws member of 'struct clk_hw_onecell_data'
> with __counted_by, which informs the bounds sanitizer (UBSAN_BOUNDS)
> about the number of elements in .hws[], so that it can warn when .hws[]
> is accessed out of bounds. As noted in that change, the __counted_by
> member must be initialized with the number of elements before the first
> array access happens, otherwise there will be a warning from each access
> prior to the initialization because the number of elements is zero.
> This occurs in atlantis_prcm_clocks_register() due to .num being
> assigned only after every clock has been stored in .hws[]. With
> CONFIG_UBSAN_BOUNDS and a compiler that implements __counted_by (GCC
> 15.1+ or Clang 20.1+), this triggers an array-index-out-of-bounds report
> during probe, and with CONFIG_UBSAN_TRAP the first store traps so the
> clock provider never registers.
> 
> Move the .num initialization to right after the allocation.
> 
> Cc: stable@vger.kernel.org
> Fixes: 23c8ebc95284 ("clk: tenstorrent: Add Atlantis clock controller driver")
> Assisted-by: LLM
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>

LGTM:

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Also, see more comments below...

> diff --git a/drivers/clk/tenstorrent/atlantis-prcm.c b/drivers/clk/tenstorrent/atlantis-prcm.c
> index 6d4386eeb7d..a68534a295c 100644
> --- a/drivers/clk/tenstorrent/atlantis-prcm.c
> +++ b/drivers/clk/tenstorrent/atlantis-prcm.c
> @@ -796,6 +796,8 @@ static int atlantis_prcm_clocks_register(struct device *dev,
>   	if (!clk_data)
>   		return -ENOMEM;
>   
> +	clk_data->num = num_clks;

Looks like variable num_clks can be removed entirely, and just do:

clk_data->num = data->num;

Thanks
-Gustavo

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Anirudh Srinivasan]

On Sat, Sep 05, 2026 at 09:48:35PM +0100, Aamir Ahmed wrote:
> Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with
> __counted_by") annotated the hws member of 'struct clk_hw_onecell_data'
> with __counted_by, which informs the bounds sanitizer (UBSAN_BOUNDS)
> about the number of elements in .hws[], so that it can warn when .hws[]
> is accessed out of bounds. As noted in that change, the __counted_by
> member must be initialized with the number of elements before the first
> array access happens, otherwise there will be a warning from each access
> prior to the initialization because the number of elements is zero.
> This occurs in atlantis_prcm_clocks_register() due to .num being
> assigned only after every clock has been stored in .hws[]. With
> CONFIG_UBSAN_BOUNDS and a compiler that implements __counted_by (GCC
> 15.1+ or Clang 20.1+), this triggers an array-index-out-of-bounds report
> during probe, and with CONFIG_UBSAN_TRAP the first store traps so the
> clock provider never registers.
> 
> Move the .num initialization to right after the allocation.
> 
> Cc: stable@vger.kernel.org
> Fixes: 23c8ebc95284 ("clk: tenstorrent: Add Atlantis clock controller driver")
> Assisted-by: LLM
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
> ---
> Found while auditing the remaining clk_hw_onecell_data users that assign
> .num only after touching .hws[], following the fixes already merged for
> clk-s2mps11 (3e14c7207a97), exynos-clkout (cf33f0b7df13) and
> clk-raspberrypi (6dc445c19050). The audit, the fix and this changelog
> were drafted with an LLM assistant and reviewed by hand.
> 
> Compile-tested only (W=1, no warnings) on x86_64 with GCC 13.3, via
> COMPILE_TEST with CONFIG_TENSTORRENT_ATLANTIS_PRCM=m. GCC 13.3 does not
> implement __counted_by (CC_HAS_COUNTED_BY needs GCC 15.1+ or Clang
> 20.1+), so the build only confirms that the change compiles; the
> sanitizer path was not exercised. I do not have the hardware, so this is
> not runtime-tested and no UBSAN report was captured.

With CONFIG_UBSAN_TRAP, probe fails like this in the QEMU tt-atlantis
machine

[    0.236357] Kernel BUG [#1]
[    0.236483] Modules linked in:
[    0.236699] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.1.0-00145-g42d8a0e21d1a #2 PREEMPTLAZY 
[    0.236778] Hardware name: Tenstorrent Atlantis RISC-V Machine (DT)
[    0.236833] epc : atlantis_prcm_probe+0x18e/0x194
[    0.237107]  ra : atlantis_prcm_probe+0xbe/0x194
[    0.237123] epc : ffffffff8060ed7a ra : ffffffff8060ecaa sp : ff20000000043b30
[    0.237134]  gp : ffffffff8173c808 tp : ff600001801f0cc0 t0 : ffffffff812db9e8
[    0.237143]  t1 : 8d795c92de4feea0 t2 : 0000000000000000 s0 : ff20000000043b90
[    0.237153]  s1 : ffffffff816d3b88 a0 : 0000000000000000 a1 : 0000000000000002
[    0.237161]  a2 : 0000000000000000 a3 : ffffffff81777080 a4 : 0000000000000000
[    0.237170]  a5 : 0000000000000000 a6 : ff60000180ee8000 a7 : ff20000000043954
[    0.237178]  s2 : ff60000180380810 s3 : ff60000180eedc40 s4 : ff60000180ee8000
[    0.237187]  s5 : ff60000180eedc48 s6 : 0000000000000038 s7 : 0000000000000000
[    0.237196]  s8 : ffffffff810a4b78 s9 : 0000000000000000 s10: 0000000000000000
[    0.237204]  s11: 0000000000000000 t3 : 0000000000000020 t4 : ffffffffffffffff
[    0.237223]  t5 : ff20000000600000 t6 : ff6000018040793e ssp : 0000000000000000
[    0.237232] status: 0000000200000120 badaddr: ffffffff8060ed7a cause: 0000000000000003
[    0.237288] [<ffffffff8060ed7a>] atlantis_prcm_probe+0x18e/0x194
[    0.237346] [<ffffffff806924f2>] platform_probe+0x66/0xac
[    0.237356] [<ffffffff8068f9ec>] really_probe+0x8c/0x23c
[    0.237367] [<ffffffff8068fc0c>] __driver_probe_device+0x70/0x110
[    0.237376] [<ffffffff8068fd82>] driver_probe_device+0x2e/0x108
[    0.237385] [<ffffffff8068ffc6>] __driver_attach+0x6a/0x168
[    0.237394] [<ffffffff8068d944>] bus_for_each_dev+0x60/0xb0
[    0.237403] [<ffffffff8068f48e>] driver_attach+0x1a/0x24
[    0.237412] [<ffffffff8068ec92>] bus_add_driver+0xca/0x1ec
[    0.237423] [<ffffffff80690c96>] driver_register+0x3e/0xdc
[    0.237432] [<ffffffff80691c2c>] __platform_driver_register+0x1c/0x24
[    0.237457] [<ffffffff80c30f16>] atlantis_prcm_driver_init+0x1a/0x24
[    0.237473] [<ffffffff8001012c>] do_one_initcall+0x5c/0x1c8
[    0.237483] [<ffffffff80c01396>] kernel_init_freeable+0x236/0x2cc
[    0.237493] [<ffffffff80ad82dc>] kernel_init+0x1c/0x14c
[    0.237503] [<ffffffff80011c74>] ret_from_fork_kernel+0x18/0x170
[    0.237511] [<ffffffff80ae2342>] ret_from_fork_kernel_asm+0x16/0x18
[    0.237598] Code: bf6d 55fd b5dd 854a 9617 00cc 0613 9326 55b5 b799 (9002) 57d1 
[    0.237743] ---[ end trace 0000000000000000 ]---
[    0.237956] Kernel panic - not syncing: Fatal exception in interrupt

Reviewed-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>

Regards
Anirudh Srinivasan

>
> Based on v7.3-rc1.
> 
>  drivers/clk/tenstorrent/atlantis-prcm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/tenstorrent/atlantis-prcm.c b/drivers/clk/tenstorrent/atlantis-prcm.c
> index 6d4386eeb7d..a68534a295c 100644
> --- a/drivers/clk/tenstorrent/atlantis-prcm.c
> +++ b/drivers/clk/tenstorrent/atlantis-prcm.c
> @@ -796,6 +796,8 @@ static int atlantis_prcm_clocks_register(struct device *dev,
>  	if (!clk_data)
>  		return -ENOMEM;
>  
> +	clk_data->num = num_clks;
> +
>  	for (i = 0; i < data->num; i++) {
>  		struct clk_hw *hw = data->hws[i];
>  		struct atlantis_clk_common *common =
> @@ -809,8 +811,6 @@ static int atlantis_prcm_clocks_register(struct device *dev,
>  		clk_data->hws[common->clkid] = hw;
>  	}
>  
> -	clk_data->num = num_clks;
> -
>  	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
>  }
>  
> 
> base-commit: 654ae5d73c05bd2943d65636ce6cd0aa46e62f18
> -- 
> 2.53.0.windows.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Anirudh Srinivasan]

On Sat, Sep 05, 2026 at 09:48:35PM +0100, Aamir Ahmed wrote:
> Commit f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with
> __counted_by") annotated the hws member of 'struct clk_hw_onecell_data'
> with __counted_by, which informs the bounds sanitizer (UBSAN_BOUNDS)
> about the number of elements in .hws[], so that it can warn when .hws[]
> is accessed out of bounds. As noted in that change, the __counted_by
> member must be initialized with the number of elements before the first
> array access happens, otherwise there will be a warning from each access
> prior to the initialization because the number of elements is zero.
> This occurs in atlantis_prcm_clocks_register() due to .num being
> assigned only after every clock has been stored in .hws[]. With
> CONFIG_UBSAN_BOUNDS and a compiler that implements __counted_by (GCC
> 15.1+ or Clang 20.1+), this triggers an array-index-out-of-bounds report
> during probe, and with CONFIG_UBSAN_TRAP the first store traps so the
> clock provider never registers.
> 
> Move the .num initialization to right after the allocation.
> 
> Cc: stable@vger.kernel.org
> Fixes: 23c8ebc95284 ("clk: tenstorrent: Add Atlantis clock controller driver")
> Assisted-by: LLM
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
> ---
> Found while auditing the remaining clk_hw_onecell_data users that assign
> .num only after touching .hws[], following the fixes already merged for
> clk-s2mps11 (3e14c7207a97), exynos-clkout (cf33f0b7df13) and
> clk-raspberrypi (6dc445c19050). The audit, the fix and this changelog
> were drafted with an LLM assistant and reviewed by hand.
> 
> Compile-tested only (W=1, no warnings) on x86_64 with GCC 13.3, via
> COMPILE_TEST with CONFIG_TENSTORRENT_ATLANTIS_PRCM=m. GCC 13.3 does not
> implement __counted_by (CC_HAS_COUNTED_BY needs GCC 15.1+ or Clang
> 20.1+), so the build only confirms that the change compiles; the
> sanitizer path was not exercised. I do not have the hardware, so this is
> not runtime-tested and no UBSAN report was captured.

With CONFIG_UBSAN_TRAP, probe fails like this in the QEMU tt-atlantis
machine

[    0.236357] Kernel BUG [#1]
[    0.236483] Modules linked in:
[    0.236699] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.1.0-00145-g42d8a0e21d1a #2 PREEMPTLAZY 
[    0.236778] Hardware name: Tenstorrent Atlantis RISC-V Machine (DT)
[    0.236833] epc : atlantis_prcm_probe+0x18e/0x194
[    0.237107]  ra : atlantis_prcm_probe+0xbe/0x194
[    0.237123] epc : ffffffff8060ed7a ra : ffffffff8060ecaa sp : ff20000000043b30
[    0.237134]  gp : ffffffff8173c808 tp : ff600001801f0cc0 t0 : ffffffff812db9e8
[    0.237143]  t1 : 8d795c92de4feea0 t2 : 0000000000000000 s0 : ff20000000043b90
[    0.237153]  s1 : ffffffff816d3b88 a0 : 0000000000000000 a1 : 0000000000000002
[    0.237161]  a2 : 0000000000000000 a3 : ffffffff81777080 a4 : 0000000000000000
[    0.237170]  a5 : 0000000000000000 a6 : ff60000180ee8000 a7 : ff20000000043954
[    0.237178]  s2 : ff60000180380810 s3 : ff60000180eedc40 s4 : ff60000180ee8000
[    0.237187]  s5 : ff60000180eedc48 s6 : 0000000000000038 s7 : 0000000000000000
[    0.237196]  s8 : ffffffff810a4b78 s9 : 0000000000000000 s10: 0000000000000000
[    0.237204]  s11: 0000000000000000 t3 : 0000000000000020 t4 : ffffffffffffffff
[    0.237223]  t5 : ff20000000600000 t6 : ff6000018040793e ssp : 0000000000000000
[    0.237232] status: 0000000200000120 badaddr: ffffffff8060ed7a cause: 0000000000000003
[    0.237288] [<ffffffff8060ed7a>] atlantis_prcm_probe+0x18e/0x194
[    0.237346] [<ffffffff806924f2>] platform_probe+0x66/0xac
[    0.237356] [<ffffffff8068f9ec>] really_probe+0x8c/0x23c
[    0.237367] [<ffffffff8068fc0c>] __driver_probe_device+0x70/0x110
[    0.237376] [<ffffffff8068fd82>] driver_probe_device+0x2e/0x108
[    0.237385] [<ffffffff8068ffc6>] __driver_attach+0x6a/0x168
[    0.237394] [<ffffffff8068d944>] bus_for_each_dev+0x60/0xb0
[    0.237403] [<ffffffff8068f48e>] driver_attach+0x1a/0x24
[    0.237412] [<ffffffff8068ec92>] bus_add_driver+0xca/0x1ec
[    0.237423] [<ffffffff80690c96>] driver_register+0x3e/0xdc
[    0.237432] [<ffffffff80691c2c>] __platform_driver_register+0x1c/0x24
[    0.237457] [<ffffffff80c30f16>] atlantis_prcm_driver_init+0x1a/0x24
[    0.237473] [<ffffffff8001012c>] do_one_initcall+0x5c/0x1c8
[    0.237483] [<ffffffff80c01396>] kernel_init_freeable+0x236/0x2cc
[    0.237493] [<ffffffff80ad82dc>] kernel_init+0x1c/0x14c
[    0.237503] [<ffffffff80011c74>] ret_from_fork_kernel+0x18/0x170
[    0.237511] [<ffffffff80ae2342>] ret_from_fork_kernel_asm+0x16/0x18
[    0.237598] Code: bf6d 55fd b5dd 854a 9617 00cc 0613 9326 55b5 b799 (9002) 57d1 
[    0.237743] ---[ end trace 0000000000000000 ]---
[    0.237956] Kernel panic - not syncing: Fatal exception in interrupt

Reviewed-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>

Regards
Anirudh Srinivasan

>
> Based on v7.3-rc1.
> 
>  drivers/clk/tenstorrent/atlantis-prcm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/tenstorrent/atlantis-prcm.c b/drivers/clk/tenstorrent/atlantis-prcm.c
> index 6d4386eeb7d..a68534a295c 100644
> --- a/drivers/clk/tenstorrent/atlantis-prcm.c
> +++ b/drivers/clk/tenstorrent/atlantis-prcm.c
> @@ -796,6 +796,8 @@ static int atlantis_prcm_clocks_register(struct device *dev,
>  	if (!clk_data)
>  		return -ENOMEM;
>  
> +	clk_data->num = num_clks;
> +
>  	for (i = 0; i < data->num; i++) {
>  		struct clk_hw *hw = data->hws[i];
>  		struct atlantis_clk_common *common =
> @@ -809,8 +811,6 @@ static int atlantis_prcm_clocks_register(struct device *dev,
>  		clk_data->hws[common->clkid] = hw;
>  	}
>  
> -	clk_data->num = num_clks;
> -
>  	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
>  }
>  
> 
> base-commit: 654ae5d73c05bd2943d65636ce6cd0aa46e62f18
> -- 
> 2.53.0.windows.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Aamir Ahmed]

On Tue, Sep 08, 2026 at 11:56:12AM +0200, Gustavo A. R. Silva wrote:
> Looks like variable num_clks can be removed entirely, and just do:
>
> clk_data->num = data->num;

Yes - it is only read once and data is const.

Thanks for reviewing, and thanks Anirudh for testing. For the redundant
variable, since this has a stable Cc, perhaps we can keep the diff
minimal here and I can send the cleanup as a follow-up once it lands?

Kind Regards

Aamir A.

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Aamir Ahmed]

On Tue, Sep 08, 2026 at 11:56:12AM +0200, Gustavo A. R. Silva wrote:
> Looks like variable num_clks can be removed entirely, and just do:
>
> clk_data->num = data->num;

Yes - it is only read once and data is const.

Thanks for reviewing, and thanks Anirudh for testing. For the redundant
variable, since this has a stable Cc, perhaps we can keep the diff
minimal here and I can send the cleanup as a follow-up once it lands?

Kind Regards

Aamir A.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Drew Fustini]

On Thu, Sep 10, 2026 at 01:47:31AM +0100, Aamir Ahmed wrote:
> On Tue, Sep 08, 2026 at 11:56:12AM +0200, Gustavo A. R. Silva wrote:
> > Looks like variable num_clks can be removed entirely, and just do:
> >
> > clk_data->num = data->num;
> 
> Yes - it is only read once and data is const.
> 
> Thanks for reviewing, and thanks Anirudh for testing. For the redundant
> variable, since this has a stable Cc, perhaps we can keep the diff
> minimal here and I can send the cleanup as a follow-up once it lands?
> 
> Kind Regards
> 
> Aamir A.

Thanks for sending the patch.

I would prefer a v2 with this change in order to avoid a trivial patch
later. We are still in the process of upstreaming support for the
Atlantis platform, so there are no users yet that would be affected by
this issue.

Thanks,
Drew

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Drew Fustini]

On Thu, Sep 10, 2026 at 01:47:31AM +0100, Aamir Ahmed wrote:
> On Tue, Sep 08, 2026 at 11:56:12AM +0200, Gustavo A. R. Silva wrote:
> > Looks like variable num_clks can be removed entirely, and just do:
> >
> > clk_data->num = data->num;
> 
> Yes - it is only read once and data is const.
> 
> Thanks for reviewing, and thanks Anirudh for testing. For the redundant
> variable, since this has a stable Cc, perhaps we can keep the diff
> minimal here and I can send the cleanup as a follow-up once it lands?
> 
> Kind Regards
> 
> Aamir A.

Thanks for sending the patch.

I would prefer a v2 with this change in order to avoid a trivial patch
later. We are still in the process of upstreaming support for the
Atlantis platform, so there are no users yet that would be affected by
this issue.

Thanks,
Drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Drew Fustini]

On Wed, Sep 23, 2026 at 07:13:22PM -0700, Drew Fustini wrote:
> On Thu, Sep 10, 2026 at 01:47:31AM +0100, Aamir Ahmed wrote:
> > On Tue, Sep 08, 2026 at 11:56:12AM +0200, Gustavo A. R. Silva wrote:
> > > Looks like variable num_clks can be removed entirely, and just do:
> > >
> > > clk_data->num = data->num;
> > 
> > Yes - it is only read once and data is const.
> > 
> > Thanks for reviewing, and thanks Anirudh for testing. For the redundant
> > variable, since this has a stable Cc, perhaps we can keep the diff
> > minimal here and I can send the cleanup as a follow-up once it lands?
> > 
> > Kind Regards
> > 
> > Aamir A.
> 
> Thanks for sending the patch.
> 
> I would prefer a v2 with this change in order to avoid a trivial patch
> later. We are still in the process of upstreaming support for the
> Atlantis platform, so there are no users yet that would be affected by
> this issue.

Applied to tenstorrent-clk-for-next with that fixup so there is no need
for a v2:

  da9f1312b06e clk: tenstorrent: Assign .num before accessing .hws

Thanks,
Drew

Re: [PATCH] clk: tenstorrent: Assign .num before accessing .hws

[Drew Fustini]

On Wed, Sep 23, 2026 at 07:13:22PM -0700, Drew Fustini wrote:
> On Thu, Sep 10, 2026 at 01:47:31AM +0100, Aamir Ahmed wrote:
> > On Tue, Sep 08, 2026 at 11:56:12AM +0200, Gustavo A. R. Silva wrote:
> > > Looks like variable num_clks can be removed entirely, and just do:
> > >
> > > clk_data->num = data->num;
> > 
> > Yes - it is only read once and data is const.
> > 
> > Thanks for reviewing, and thanks Anirudh for testing. For the redundant
> > variable, since this has a stable Cc, perhaps we can keep the diff
> > minimal here and I can send the cleanup as a follow-up once it lands?
> > 
> > Kind Regards
> > 
> > Aamir A.
> 
> Thanks for sending the patch.
> 
> I would prefer a v2 with this change in order to avoid a trivial patch
> later. We are still in the process of upstreaming support for the
> Atlantis platform, so there are no users yet that would be affected by
> this issue.

Applied to tenstorrent-clk-for-next with that fixup so there is no need
for a v2:

  da9f1312b06e clk: tenstorrent: Assign .num before accessing .hws

Thanks,
Drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv