linux-kernel - Re: [PATCH 09/19] clk: divider: check validity of flags when a table is provided

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <175834587624.4354.6026619740146574818@lazor>
Date: Fri, 19 Sep 2025 22:24:36 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Benoît Monin <benoit.monin@...tlin.com>, Conor Dooley <conor+dt@...nel.org>, Gregory CLEMENT <gregory.clement@...tlin.com>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Michael Turquette <mturquette@...libre.com>, Philipp Zabel <p.zabel@...gutronix.de>, Rob Herring <robh@...nel.org>, Thomas Bogendoerfer <tsbogend@...ha.franken.de>, Théo Lebrun <theo.lebrun@...tlin.com>, Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>
Cc: Thomas Petazzoni <thomas.petazzoni@...tlin.com>, linux-mips@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org, Tawfik Bayouk <tawfik.bayouk@...ileye.com>, Sari Khoury <sari.khoury@...ileye.com>, Benoît Monin <benoit.monin@...tlin.com>
Subject: Re: [PATCH 09/19] clk: divider: check validity of flags when a table is provided

Quoting Benoît Monin (2025-09-03 05:47:16)
> If any of the flag CLK_DIVIDER_ONE_BASED, CLK_DIVIDER_POWER_OF_TWO,
> CLK_DIVIDER_MAX_AT_ZERO or CLK_DIVIDER_EVEN_INTEGERS is set, the divider
> table will be ignored in _get_div and _get_val. This can lead to subtle
> bug when a clock is registered with some flags and an optional table,
> with the clock rate and register value being computed with the wrong
> type of conversion.
> 
> Prevent this by refusing to register a divider with both the flag and
> the table set.
> 
> Signed-off-by: Benoît Monin <benoit.monin@...tlin.com>
> ---
>  drivers/clk/clk-divider.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 8e8f87024e76625f348f1d66c15a7a938fa0c4db..b4861d519bac2121dd015d094c94a5fee2480148 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -561,6 +561,13 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
>                         return ERR_PTR(-EINVAL);
>                 }
>         }

Nitpick: Prefer a newline here.

> +       if (table && (clk_divider_flags & (CLK_DIVIDER_ONE_BASED |
> +                                          CLK_DIVIDER_POWER_OF_TWO |
> +                                          CLK_DIVIDER_MAX_AT_ZERO |
> +                                          CLK_DIVIDER_EVEN_INTEGERS))) {
> +               pr_warn("divider table and flags incompatible\n");

This pr_warn() (and the one above this one) are not very helpful because
we don't know which clk is the problem. We also don't know if this is
going to cause boot failures for devices out there that have this flag
set and a table. Were all drivers audited?

I wonder if we can check this condition at compile time with some sort
of test on the clk_divider_flags expression to see if it is a compile
time constant along with the table pointer being a compile time constant
as well that isn't NULL?

> +               return ERR_PTR(-EINVAL);
> +       }