Re: [PATCH 2/7] clk: meson: pll: Support PLLs without a pre-divider

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] clk: meson: 32-bit integer overflow in
  meson_clk_get_pll_range_index() [1]
--

commit 241035ecfdb206acdcf290a5e288104a056a3f8b
Author: Carlo Caione <ccaione@baylibre.com>

clk: meson: pll: Support PLLs without a pre-divider

This commit makes the PLL pre-divider parameter optional in the common Meson
PLL driver, defaulting the divider to 1 when the parameter is not applicable.

> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index 1ea6579a760f3..44b33051405f5 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c

[ ... ]

> @@ -174,7 +176,7 @@ static int meson_clk_get_pll_range_index(unsigned long rate,
>  	*n = index + 1;
>  
>  	/* Check the predivider range */
> -	if (*n >= (1 << pll->n.width))
> +	if (*n >= (MESON_PARM_APPLICABLE(&pll->n) ? 1 << pll->n.width : 2))
>  		return -EINVAL;
>  
>  	if (*n == 1) {

[Severity: High]
This problem wasn't introduced by this patch, but can `pll->range->max *
parent_rate` in meson_clk_get_pll_range_index() and `parent_rate * m` in
__pll_params_with_frac() overflow 32-bit unsigned long arithmetic [1]?

drivers/clk/meson/clk-pll.c:meson_clk_get_pll_range_index() {
	...
	if (*n == 1) {
		/* Get the boundaries out the way */
		if (rate <= pll->range->min * parent_rate) {
			*m = pll->range->min;
			return -ENODATA;
		} else if (rate >= pll->range->max * parent_rate) {
			*m = pll->range->max;
			return -ENODATA;
		}
	}
	...
}

drivers/clk/meson/clk-pll.c:__pll_params_with_frac() {
	...
	/* Bail out if we are already over the requested rate */
	if (rate < parent_rate * m / n)
		return 0;
	...
}

On 32-bit architectures, multiplying a 24 MHz `parent_rate` by a typical
multiplier (such as `pll->range->max` of 250) reaches 6,000,000,000 and wraps
around 32-bit `ULONG_MAX`.  This causes the range check in
meson_clk_get_pll_range_index() to miscalculate `m` and the guard in
__pll_params_with_frac() to be bypassed, underflowing `val -= m * frac_max`
and writing corrupted multiplier and fractional values to the PLL hardware.

[1] https://sashiko.dev/#/bug/linux-8a5ec432-efbd-4d3f-83c7-2a50fc2850c7

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-upstream-s7-clk-v1-0-7d501d62621f@baylibre.com?part=2

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