mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] rtc: ac100: Assign .num before accessing .hws
@ 2026-09-05 18:38 Aamir Ahmed
  2026-09-13 15:45 ` Chen-Yu Tsai
  2026-09-14  5:46 ` Gustavo A. R. Silva
  0 siblings, 2 replies; 4+ messages in thread
From: Aamir Ahmed @ 2026-09-05 18:38 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Chen-Yu Tsai, linux-sunxi, Kees Cook,
	linux-hardening, Aamir Ahmed, stable

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 ac100_rtc_register_clks() due to .num being assigned only
after every clkout 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.

Initialize .num with AC100_CLKOUT_NUM, the number of elements .hws[] was
allocated with, right after the allocation. That is the value the loop
counter ends up at on the success path anyway, so the provider's
behaviour is unchanged.

Cc: stable@vger.kernel.org
Fixes: f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with __counted_by")
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, with
CONFIG_RTC_DRV_AC100=m forced on the make command line because the
driver has no COMPILE_TEST option. 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/rtc/rtc-ac100.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ac100.c b/drivers/rtc/rtc-ac100.c
index bba7115ff3a..a2f465438fd 100644
--- a/drivers/rtc/rtc-ac100.c
+++ b/drivers/rtc/rtc-ac100.c
@@ -317,6 +317,8 @@ static int ac100_rtc_register_clks(struct ac100_rtc_dev *chip)
 	if (!chip->clk_data)
 		return -ENOMEM;
 
+	chip->clk_data->num = AC100_CLKOUT_NUM;
+
 	chip->rtc_32k_clk = clk_hw_register_fixed_rate(chip->dev,
 						       AC100_RTC_32K_NAME,
 						       NULL, 0,
@@ -360,7 +362,6 @@ static int ac100_rtc_register_clks(struct ac100_rtc_dev *chip)
 		chip->clk_data->hws[i] = &clk->hw;
 	}
 
-	chip->clk_data->num = i;
 	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, chip->clk_data);
 	if (ret)
 		goto err_unregister_rtc_32k;

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] rtc: ac100: Assign .num before accessing .hws
  2026-09-05 18:38 [PATCH] rtc: ac100: Assign .num before accessing .hws Aamir Ahmed
@ 2026-09-13 15:45 ` Chen-Yu Tsai
  2026-09-14  5:46 ` Gustavo A. R. Silva
  1 sibling, 0 replies; 4+ messages in thread
From: Chen-Yu Tsai @ 2026-09-13 15:45 UTC (permalink / raw)
  To: Aamir Ahmed, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, linux-sunxi, Kees Cook, linux-hardening, stable

On Sun, Sep 6, 2026 at 2:40 AM Aamir Ahmed <elb12345@hotmail.co.uk> 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 ac100_rtc_register_clks() due to .num being assigned only
> after every clkout 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.
>
> Initialize .num with AC100_CLKOUT_NUM, the number of elements .hws[] was
> allocated with, right after the allocation. That is the value the loop
> counter ends up at on the success path anyway, so the provider's
> behaviour is unchanged.
>
> Cc: stable@vger.kernel.org
> Fixes: f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with __counted_by")
> Assisted-by: LLM
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>


Reviewed-by: Chen-Yu Tsai <wens@kernel.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] rtc: ac100: Assign .num before accessing .hws
  2026-09-05 18:38 [PATCH] rtc: ac100: Assign .num before accessing .hws Aamir Ahmed
  2026-09-13 15:45 ` Chen-Yu Tsai
@ 2026-09-14  5:46 ` Gustavo A. R. Silva
  1 sibling, 0 replies; 4+ messages in thread
From: Gustavo A. R. Silva @ 2026-09-14  5:46 UTC (permalink / raw)
  To: Aamir Ahmed, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Chen-Yu Tsai, linux-sunxi, Kees Cook,
	linux-hardening, stable



On 9/6/26 03:38, 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 ac100_rtc_register_clks() due to .num being assigned only
> after every clkout 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.
> 
> Initialize .num with AC100_CLKOUT_NUM, the number of elements .hws[] was
> allocated with, right after the allocation. That is the value the loop
> counter ends up at on the success path anyway, so the provider's
> behaviour is unchanged.
> 
> Cc: stable@vger.kernel.org
> Fixes: f316cdff8d67 ("clk: Annotate struct clk_hw_onecell_data with __counted_by")
> Assisted-by: LLM
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>

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

Thanks
-Gustavo

> ---
> 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, with
> CONFIG_RTC_DRV_AC100=m forced on the make command line because the
> driver has no COMPILE_TEST option. 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/rtc/rtc-ac100.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/rtc-ac100.c b/drivers/rtc/rtc-ac100.c
> index bba7115ff3a..a2f465438fd 100644
> --- a/drivers/rtc/rtc-ac100.c
> +++ b/drivers/rtc/rtc-ac100.c
> @@ -317,6 +317,8 @@ static int ac100_rtc_register_clks(struct ac100_rtc_dev *chip)
>   	if (!chip->clk_data)
>   		return -ENOMEM;
>   
> +	chip->clk_data->num = AC100_CLKOUT_NUM;
> +
>   	chip->rtc_32k_clk = clk_hw_register_fixed_rate(chip->dev,
>   						       AC100_RTC_32K_NAME,
>   						       NULL, 0,
> @@ -360,7 +362,6 @@ static int ac100_rtc_register_clks(struct ac100_rtc_dev *chip)
>   		chip->clk_data->hws[i] = &clk->hw;
>   	}
>   
> -	chip->clk_data->num = i;
>   	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, chip->clk_data);
>   	if (ret)
>   		goto err_unregister_rtc_32k;
> 
> base-commit: 654ae5d73c05bd2943d65636ce6cd0aa46e62f18


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] rtc: ac100: Assign .num before accessing .hws
       [not found] <20260905184936.155E11F00A3A@smtp.kernel.org>
@ 2026-09-05 20:45 ` Aamir Ahmed
  0 siblings, 0 replies; 4+ messages in thread
From: Aamir Ahmed @ 2026-09-05 20:45 UTC (permalink / raw)
  To: linux-rtc; +Cc: Alexandre Belloni, linux-kernel, Aamir Ahmed

On Sat, Sep 05, 2026 at 06:49:35PM +0000, Sashiko AI wrote:
> [Severity: High]
> This isn't a bug introduced by this patch, but does the lack of cleanup
> in ac100_rtc_probe() lead to a use-after-free and resource leak?
[...]
> Can any subsequent attempt to query a clock via device tree traverse the
> provider list, dereference the freed pointer, and trigger a crash?

Yes, I think that is correct, thanks for pointing it out.

ac100_rtc_register_clks() registers the RTC-32k clock and the clock
provider without devres, and both are released only from
ac100_rtc_remove(). really_probe() reaches probe_failed below the
device_remove() call, so remove() does not run when probe() fails, and
chip->clk_data is then freed by devres_release_all() while the provider
still points at it. A later lookup on that node would read freed memory
in of_clk_hw_onecell_get(). There are real consumers: the wifi power
sequence nodes on sun8i-a83t-bananapi-m3 and sun8i-a83t-cubietruck-plus
take <&ac100_rtc 1>, and the sun9i-a80 boards route osc32k through
<&ac100_rtc 0>.

The RTC-32k clock is leaked on the same path, and since __clk_register()
rejects a duplicate name with -EEXIST that also makes a later probe of
the same device fail.

The -EINVAL path taken when the ADDA 4M parent clock cannot be found
leaks the RTC-32k clock too, but no provider has been registered at that
point, so that one is a leak rather than a use-after-free. It also is
not reachable with any in-tree device tree, since every board points the
rtc node at the codec node.

On the rating: I do not think High is right. devm_rtc_register_device()
can only fail with -ENOMEM here, nothing on that path is influenced by
an unprivileged user, and re-opening the window needs a root-initiated
rebind. Per Documentation/process/threat-model.rst I would treat it as a
regular error-path bug rather than a vulnerability, which is also how
the equivalent fixes in rtc-jz4740 and rtc-pcf8563 were handled.

I have sent a separate patch converting both registrations to
devm_clk_hw_register_fixed_rate() and devm_of_clk_add_hw_provider(),
which also lets ac100_rtc_unregister_clks() and the remove callback go
away:

  rtc: ac100: Fix clock provider use-after-free on probe failure

It applies on top of this one. It is compile-tested only, as I do not
have AC100 hardware.

Thanks,
Aamir

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-14  5:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-05 18:38 [PATCH] rtc: ac100: Assign .num before accessing .hws Aamir Ahmed
2026-09-13 15:45 ` Chen-Yu Tsai
2026-09-14  5:46 ` Gustavo A. R. Silva
     [not found] <20260905184936.155E11F00A3A@smtp.kernel.org>
2026-09-05 20:45 ` Aamir Ahmed

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®