* [PATCH] PM / devfreq: mediatek: Fix possible dereference of null pointer
@ 2022-06-17 13:33 matthias.bgg
2022-06-20 14:21 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 3+ messages in thread
From: matthias.bgg @ 2022-06-17 13:33 UTC (permalink / raw)
To: dan.carpenter, johnson.wang; +Cc: linux-pm, linux-mediatek, Matthias Brugger
From: Matthias Brugger <matthias.bgg@gmail.com>
We dereference the driver data before checking of it's valid.
This patch fixes this, by accessing the PLL data struct after cheching
the pointer
Fixes: 07dc787be231 ("Add linux-next specific files for 20220617")
Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
drivers/devfreq/mtk-cci-devfreq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/devfreq/mtk-cci-devfreq.c b/drivers/devfreq/mtk-cci-devfreq.c
index 71abb3fbd042..77522f16c878 100644
--- a/drivers/devfreq/mtk-cci-devfreq.c
+++ b/drivers/devfreq/mtk-cci-devfreq.c
@@ -127,7 +127,7 @@ static int mtk_ccifreq_target(struct device *dev, unsigned long *freq,
u32 flags)
{
struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev);
- struct clk *cci_pll = clk_get_parent(drv->cci_clk);
+ struct clk *cci_pll;
struct dev_pm_opp *opp;
unsigned long opp_rate;
int voltage, pre_voltage, inter_voltage, target_voltage, ret;
@@ -138,6 +138,7 @@ static int mtk_ccifreq_target(struct device *dev, unsigned long *freq,
if (drv->pre_freq == *freq)
return 0;
+ cci_pll = clk_get_parent(drv->cci_clk);
inter_voltage = drv->inter_voltage;
opp_rate = *freq;
--
2.36.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] PM / devfreq: mediatek: Fix possible dereference of null pointer
2022-06-17 13:33 [PATCH] PM / devfreq: mediatek: Fix possible dereference of null pointer matthias.bgg
@ 2022-06-20 14:21 ` AngeloGioacchino Del Regno
2022-06-20 15:37 ` Matthias Brugger
0 siblings, 1 reply; 3+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-06-20 14:21 UTC (permalink / raw)
To: matthias.bgg, dan.carpenter, johnson.wang
Cc: linux-pm, linux-mediatek, Matthias Brugger
Il 17/06/22 15:33, matthias.bgg@kernel.org ha scritto:
> From: Matthias Brugger <matthias.bgg@gmail.com>
>
> We dereference the driver data before checking of it's valid.
> This patch fixes this, by accessing the PLL data struct after cheching
> the pointer
>
Hello Matthias,
honestly, I don't think that this commit is right: mtk_ccifreq_target() is the
devfreq_dev_profile's .target() callback!
Checking mtk_ccifreq_probe(), we are setting drvdata long before adding the
devfreq device so, actually, it's impossible for dev_get_drvdata(dev) to return
NULL, or whatever invalid pointer.
This means that the right thing to do in mtk_ccifreq_target() is to simply remove
the `drv` NULL check, as this can never happen! :-)
Cheers,
Angelo
> Fixes: 07dc787be231 ("Add linux-next specific files for 20220617")
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---
> drivers/devfreq/mtk-cci-devfreq.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/mtk-cci-devfreq.c b/drivers/devfreq/mtk-cci-devfreq.c
> index 71abb3fbd042..77522f16c878 100644
> --- a/drivers/devfreq/mtk-cci-devfreq.c
> +++ b/drivers/devfreq/mtk-cci-devfreq.c
> @@ -127,7 +127,7 @@ static int mtk_ccifreq_target(struct device *dev, unsigned long *freq,
> u32 flags)
> {
> struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev);
> - struct clk *cci_pll = clk_get_parent(drv->cci_clk);
> + struct clk *cci_pll;
> struct dev_pm_opp *opp;
> unsigned long opp_rate;
> int voltage, pre_voltage, inter_voltage, target_voltage, ret;
> @@ -138,6 +138,7 @@ static int mtk_ccifreq_target(struct device *dev, unsigned long *freq,
> if (drv->pre_freq == *freq)
> return 0;
>
> + cci_pll = clk_get_parent(drv->cci_clk);
> inter_voltage = drv->inter_voltage;
>
> opp_rate = *freq;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PM / devfreq: mediatek: Fix possible dereference of null pointer
2022-06-20 14:21 ` AngeloGioacchino Del Regno
@ 2022-06-20 15:37 ` Matthias Brugger
0 siblings, 0 replies; 3+ messages in thread
From: Matthias Brugger @ 2022-06-20 15:37 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, matthias.bgg, dan.carpenter, johnson.wang
Cc: linux-pm, linux-mediatek
On 20/06/2022 16:21, AngeloGioacchino Del Regno wrote:
> Il 17/06/22 15:33, matthias.bgg@kernel.org ha scritto:
>> From: Matthias Brugger <matthias.bgg@gmail.com>
>>
>> We dereference the driver data before checking of it's valid.
>> This patch fixes this, by accessing the PLL data struct after cheching
>> the pointer
>>
>
> Hello Matthias,
>
> honestly, I don't think that this commit is right: mtk_ccifreq_target() is the
> devfreq_dev_profile's .target() callback!
>
> Checking mtk_ccifreq_probe(), we are setting drvdata long before adding the
> devfreq device so, actually, it's impossible for dev_get_drvdata(dev) to return
> NULL, or whatever invalid pointer.
>
> This means that the right thing to do in mtk_ccifreq_target() is to simply remove
> the `drv` NULL check, as this can never happen! :-)
>
Yes you are right. Probe will error out if drv can't be allocated or cci_clk
isn't present. I'll send a new patch.
Thanks,
Matthias
> Cheers,
> Angelo
>
>> Fixes: 07dc787be231 ("Add linux-next specific files for 20220617")
>> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>> ---
>> drivers/devfreq/mtk-cci-devfreq.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/devfreq/mtk-cci-devfreq.c
>> b/drivers/devfreq/mtk-cci-devfreq.c
>> index 71abb3fbd042..77522f16c878 100644
>> --- a/drivers/devfreq/mtk-cci-devfreq.c
>> +++ b/drivers/devfreq/mtk-cci-devfreq.c
>> @@ -127,7 +127,7 @@ static int mtk_ccifreq_target(struct device *dev, unsigned
>> long *freq,
>> u32 flags)
>> {
>> struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev);
>> - struct clk *cci_pll = clk_get_parent(drv->cci_clk);
>> + struct clk *cci_pll;
>> struct dev_pm_opp *opp;
>> unsigned long opp_rate;
>> int voltage, pre_voltage, inter_voltage, target_voltage, ret;
>> @@ -138,6 +138,7 @@ static int mtk_ccifreq_target(struct device *dev, unsigned
>> long *freq,
>> if (drv->pre_freq == *freq)
>> return 0;
>> + cci_pll = clk_get_parent(drv->cci_clk);
>> inter_voltage = drv->inter_voltage;
>> opp_rate = *freq;
>>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-20 15:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17 13:33 [PATCH] PM / devfreq: mediatek: Fix possible dereference of null pointer matthias.bgg
2022-06-20 14:21 ` AngeloGioacchino Del Regno
2022-06-20 15:37 ` Matthias Brugger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.