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

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

[Praveen Talari]

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@oss.qualcomm.com>
---
 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))
 		return 0;
 
 	clk_prepare_lock();

---
base-commit: b4515cf4156356e8f4fe6e0fdc17f59adab9772f
change-id: 20260723-fix_ptr_check_on_clk-603605c3e350

Best regards,
--  
Praveen Talari <praveen.talari@oss.qualcomm.com>

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

[Brian Masney]

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@oss.qualcomm.com>
> ---
>  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.

Brian

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@oss.qualcomm.com>
>> ---
>>   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
>

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

[Brian Masney]

Hi Praveen,

On Thu, Jul 23, 2026 at 10:10:06PM +0530, Praveen Talari wrote:
> On 23-07-2026 19:59, Brian Masney wrote:
> > 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@oss.qualcomm.com>
> > > ---
> > >   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.

This looks to be a reasonable change. Thanks.

Reviewed-by: Brian Masney <bmasney@redhat.com>

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 4506 bytes --]

Hi,

On Fri, Jul 24, 2026 at 11:44:55AM -0400, Brian Masney wrote:
> Hi Praveen,
> 
> On Thu, Jul 23, 2026 at 10:10:06PM +0530, Praveen Talari wrote:
> > On 23-07-2026 19:59, Brian Masney wrote:
> > > 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@oss.qualcomm.com>
> > > > ---
> > > >   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.
> 
> This looks to be a reasonable change. Thanks.
> 
> Reviewed-by: Brian Masney <bmasney@redhat.com>

I suggest to instead change the OPP code, so that it calls
clk_get_optional() instead of clk_get() and thus properly
"document" that the clock is optional and use the NULL dummy
clock (it's also shorter):

/*
 * There are few platforms which don't want the OPP core to
 * manage device's clock settings. In such cases neither the
 * platform provides the clks explicitly to us, nor the DT
 * contains a valid clk entry. The OPP nodes in DT may still
 * contain "opp-hz" property though, which we need to parse and
 * allow the platform to find an OPP based on freq later on.
 *
 * This is a simple solution to take care of such corner cases,
 * i.e. make the clk_count 1, which lets us allocate space for
 * frequency in opp->rates and also parse the entries in DT.
 */
opp_table->clk = clk_get_optional(dev, NULL);

ret = PTR_ERR_OR_ZERO(opp_table->clk);
if (ret) {
    dev_pm_opp_put_opp_table(opp_table);
    dev_err_probe(dev, ret, "Couldn't find clock\n");
    return ERR_PTR(ret);
}

if (opp_table->clk)
    opp_table->config_clks = _opp_config_clk_single;
opp_table->clk_count = 1;
return opp_table;

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]