Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v1] OPP: Fix crashing when current OPP has unsupportable voltage
@ 2019-06-23 17:50 Dmitry Osipenko
  2019-06-24  7:18 ` Viresh Kumar
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 17:50 UTC (permalink / raw)
  To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Marc Dietrich
  Cc: linux-pm, linux-tegra, linux-kernel

Fix NULL dereference caused by a typo in the code. In particular it
happens when CPU is running on a frequency which has unsupportable voltage
(by regulator) defined in the OPP table and a custom set_opp() callback is
being used. The problem was spotted during of testing of upcoming update
for the NVIDIA Tegra CPUFreq driver.

Cc: stable <stable@vger.kernel.org>
Fixes: 7e535993fa4f ("OPP: Separate out custom OPP handler specific code")
Reported-by: Marc Dietrich <marvin24@gmx.de>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/opp/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 9fda9a0ec016..89ec6aa220cf 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -685,7 +685,7 @@ static int _set_opp_custom(const struct opp_table *opp_table,
 
 	data->old_opp.rate = old_freq;
 	size = sizeof(*old_supply) * opp_table->regulator_count;
-	if (IS_ERR(old_supply))
+	if (!old_supply)
 		memset(data->old_opp.supplies, 0, size);
 	else
 		memcpy(data->old_opp.supplies, old_supply, size);
-- 
2.22.0


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

* Re: [PATCH v1] OPP: Fix crashing when current OPP has unsupportable voltage
  2019-06-23 17:50 [PATCH v1] OPP: Fix crashing when current OPP has unsupportable voltage Dmitry Osipenko
@ 2019-06-24  7:18 ` Viresh Kumar
  2019-06-24 10:18   ` Dmitry Osipenko
  0 siblings, 1 reply; 3+ messages in thread
From: Viresh Kumar @ 2019-06-24  7:18 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Marc Dietrich,
	linux-pm, linux-tegra, linux-kernel

On 23-06-19, 20:50, Dmitry Osipenko wrote:
> Fix NULL dereference caused by a typo in the code. In particular it
> happens when CPU is running on a frequency which has unsupportable voltage
> (by regulator) defined in the OPP table and a custom set_opp() callback is
> being used. The problem was spotted during of testing of upcoming update
> for the NVIDIA Tegra CPUFreq driver.
> 
> Cc: stable <stable@vger.kernel.org>
> Fixes: 7e535993fa4f ("OPP: Separate out custom OPP handler specific code")
> Reported-by: Marc Dietrich <marvin24@gmx.de>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/opp/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 9fda9a0ec016..89ec6aa220cf 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -685,7 +685,7 @@ static int _set_opp_custom(const struct opp_table *opp_table,
>  
>  	data->old_opp.rate = old_freq;
>  	size = sizeof(*old_supply) * opp_table->regulator_count;
> -	if (IS_ERR(old_supply))
> +	if (!old_supply)
>  		memset(data->old_opp.supplies, 0, size);
>  	else
>  		memcpy(data->old_opp.supplies, old_supply, size);

While the change is fine, the commit log isn't. It isn't about
unsupportable voltage but frequency. The frequency the CPU is
currently running at, is not present in the OPP table and so there is
no corresponding OPP, hence no voltage supplies.

I have applied this patch with following change log:

commit 560d1bcad715c215e7ffe5d7cffe045974b623d0 (HEAD -> opp/linux-next)
Author: Dmitry Osipenko <digetx@gmail.com>
Date:   Sun Jun 23 20:50:53 2019 +0300

    opp: Don't use IS_ERR on invalid supplies
    
    _set_opp_custom() receives a set of OPP supplies as its arguments and
    the caller of it passes NULL when the supplies are not valid. But
    _set_opp_custom(), by mistake, checks for error by performing
    IS_ERR(old_supply) on it which will always evaluate to false.
    
    The problem was spotted during of testing of upcoming update for the
    NVIDIA Tegra CPUFreq driver.
    
    Cc: stable <stable@vger.kernel.org>
    Fixes: 7e535993fa4f ("OPP: Separate out custom OPP handler specific code")
    Reported-by: Marc Dietrich <marvin24@gmx.de>
    Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
    [ Viresh: Massaged changelog ]
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/opp/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


-- 
viresh

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

* Re: [PATCH v1] OPP: Fix crashing when current OPP has unsupportable voltage
  2019-06-24  7:18 ` Viresh Kumar
@ 2019-06-24 10:18   ` Dmitry Osipenko
  0 siblings, 0 replies; 3+ messages in thread
From: Dmitry Osipenko @ 2019-06-24 10:18 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Marc Dietrich,
	linux-pm, linux-tegra, linux-kernel

24.06.2019 10:18, Viresh Kumar пишет:
> On 23-06-19, 20:50, Dmitry Osipenko wrote:
>> Fix NULL dereference caused by a typo in the code. In particular it
>> happens when CPU is running on a frequency which has unsupportable voltage
>> (by regulator) defined in the OPP table and a custom set_opp() callback is
>> being used. The problem was spotted during of testing of upcoming update
>> for the NVIDIA Tegra CPUFreq driver.
>>
>> Cc: stable <stable@vger.kernel.org>
>> Fixes: 7e535993fa4f ("OPP: Separate out custom OPP handler specific code")
>> Reported-by: Marc Dietrich <marvin24@gmx.de>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/opp/core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 9fda9a0ec016..89ec6aa220cf 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -685,7 +685,7 @@ static int _set_opp_custom(const struct opp_table *opp_table,
>>  
>>  	data->old_opp.rate = old_freq;
>>  	size = sizeof(*old_supply) * opp_table->regulator_count;
>> -	if (IS_ERR(old_supply))
>> +	if (!old_supply)
>>  		memset(data->old_opp.supplies, 0, size);
>>  	else
>>  		memcpy(data->old_opp.supplies, old_supply, size);
> 
> While the change is fine, the commit log isn't. It isn't about
> unsupportable voltage but frequency. The frequency the CPU is
> currently running at, is not present in the OPP table and so there is
> no corresponding OPP, hence no voltage supplies.

Ah, indeed! Looks like the reason for old OPP not being found was caused
by the appropriate OPP being disabled because of unsupportable voltage.
The offending higher "unsupportable" CPU freq was left after bootloader.

> I have applied this patch with following change log:
> 
> commit 560d1bcad715c215e7ffe5d7cffe045974b623d0 (HEAD -> opp/linux-next)
> Author: Dmitry Osipenko <digetx@gmail.com>
> Date:   Sun Jun 23 20:50:53 2019 +0300
> 
>     opp: Don't use IS_ERR on invalid supplies
>     
>     _set_opp_custom() receives a set of OPP supplies as its arguments and
>     the caller of it passes NULL when the supplies are not valid. But
>     _set_opp_custom(), by mistake, checks for error by performing
>     IS_ERR(old_supply) on it which will always evaluate to false.
>     
>     The problem was spotted during of testing of upcoming update for the
>     NVIDIA Tegra CPUFreq driver.
>     
>     Cc: stable <stable@vger.kernel.org>
>     Fixes: 7e535993fa4f ("OPP: Separate out custom OPP handler specific code")
>     Reported-by: Marc Dietrich <marvin24@gmx.de>
>     Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>     [ Viresh: Massaged changelog ]
>     Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/opp/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thank you very much!

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-23 17:50 [PATCH v1] OPP: Fix crashing when current OPP has unsupportable voltage Dmitry Osipenko
2019-06-24  7:18 ` Viresh Kumar
2019-06-24 10:18   ` Dmitry Osipenko

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org linux-pm@archiver.kernel.org
	public-inbox-index linux-pm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox