Re: [PATCH] clk: Guard clk_round_rate() against error pointers

[Praveen Talari]

Hi Brian,

On 23-07-2026 19:59, Brian Masney wrote:
> Hi Praveen,
>
> On Thu, Jul 23, 2026 at 11:40:47AM +0530, Praveen Talari wrote:
> > clk_round_rate() only checks for a NULL clk pointer before
> > dereferencing it, but callers such as dev_pm_opp_set_rate() can pass
> > it an error pointer (e.g. ERR_PTR(-ENOENT) left behind by
> > clk_get() when a device has no Linux clock and is instead managed by
> > firmware via a genpd/OPP performance domain).
> >
> > Dereferencing that error pointer to read clk->exclusive_count
> > crashes with an unhandled kernel NULL pointer dereference, since
> > ERR_PTR(-ENOENT) plus the field's offset lands on a small, unmapped
> > address:
> >
> >    Unable to handle kernel NULL pointer dereference at virtual
> >    address 000000000000002e
> >    ...
> >    pc : clk_round_rate+0x3c/0x188
> >    ...
> >    Call trace:
> >     clk_round_rate+0x3c/0x188 (P)
> >     dev_pm_opp_set_rate+0x114/0x33c
> >
> > Change the guard from "if (!clk)" to "if (IS_ERR_OR_NULL(clk))",
> > matching the pattern already used by other clk consumer API
> > functions such as clk_unprepare(), so an error pointer is rejected
> > the same way a NULL pointer is.
> >
> > Signed-off-by: Praveen Talari <praveen.talari@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/clk/clk.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 048adfa86a5d..8c1ad3d10284 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -1780,7 +1780,7 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
> >   	struct clk_rate_request req;
> >   	int ret;
> >   
> > -	if (!clk)
> > +	if (IS_ERR_OR_NULL(clk))
> Can you provide more details about the clk_get() call point that starts this
> error? Specifically which driver this occurs in and the exact scenario that
> triggers this.

On SA8255P platform there is no Linux
clock for the SE, and the perf domain device's OPPs are populated entirely
from firmware via devm_pm_opp_of_add_table() (through
of_genpd_add_provider_simple()/onecell()), so the perf domain's OPP table
has entries even though no clk_get() ever succeeds for it.

The clk_get(-ENOENT) case comes from _update_opp_table_clk() in
drivers/opp/core.c:

    opp_table->clk = clk_get(dev, NULL);
    ret = PTR_ERR_OR_ZERO(opp_table->clk);
    ...
    if (ret == -ENOENT) {
        /* ... no clk provided ... */
        opp_table->clk_count = 1;
        return opp_table;   /* opp_table->clk left as ERR_PTR(-ENOENT) */
    }

Because the perf domain device has no "clocks" property (its OPPs are
supplied purely as performance states by firmware/genpd), clk_get()
returns -ENOENT, and opp_table->clk is left holding that error pointer
rather than being reset to NULL.


Praveen

> Brian