pdev struct doesn't exits for the devices whose status are disabled
from DT node, in such cases NULL is returned from 'of_find_device_by_node'
Later when we try to get drvdata from pdev struct NULL pointer dereference
is triggered.
Add a NULL check for return values to fix the issue.
We were hitting this issue when one of QUP is disabled.
Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to fix earlycon crash")
Reported-by: Sai Prakash Ranjan <[email protected]>
Signed-off-by: Akash Asthana <[email protected]>
---
drivers/soc/qcom/qcom-geni-se.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 355d503..6e5fe65 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -820,6 +820,7 @@ void geni_remove_earlycon_icc_vote(void)
struct geni_wrapper *wrapper;
struct device_node *parent;
struct device_node *child;
+ struct platform_device *wrapper_pdev;
if (!earlycon_wrapper)
return;
@@ -829,7 +830,12 @@ void geni_remove_earlycon_icc_vote(void)
for_each_child_of_node(parent, child) {
if (!of_device_is_compatible(child, "qcom,geni-se-qup"))
continue;
- wrapper = platform_get_drvdata(of_find_device_by_node(child));
+
+ wrapper_pdev = of_find_device_by_node(child);
+ if (!wrapper_pdev)
+ continue;
+
+ wrapper = platform_get_drvdata(wrapper_pdev);
icc_put(wrapper->to_core.path);
wrapper->to_core.path = NULL;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Please make sure to cc the linux-arm-msm@vger list for patches of
Qualcomm code.
On Fri, Jul 17, 2020 at 08:02:22PM +0530, Akash Asthana wrote:
> pdev struct doesn't exits for the devices whose status are disabled
s/exits/exist/
> from DT node, in such cases NULL is returned from 'of_find_device_by_node'
> Later when we try to get drvdata from pdev struct NULL pointer dereference
> is triggered.
>
> Add a NULL check for return values to fix the issue.
>
> We were hitting this issue when one of QUP is disabled.
>
> Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to fix earlycon crash")
> Reported-by: Sai Prakash Ranjan <[email protected]>
> Signed-off-by: Akash Asthana <[email protected]>
> ---
> drivers/soc/qcom/qcom-geni-se.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 355d503..6e5fe65 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -820,6 +820,7 @@ void geni_remove_earlycon_icc_vote(void)
> struct geni_wrapper *wrapper;
> struct device_node *parent;
> struct device_node *child;
> + struct platform_device *wrapper_pdev;
nit: since there is no other 'pdev' in this function you could just name
it 'pdev', which is less clunky. The variable is only used immediately
after it is assigned, so it's clear from the context that it refers to
the 'wrapper'.
Reviewed-by: Matthias Kaehlcke <[email protected]>
Hi Matthias,
On 7/17/2020 8:18 PM, Matthias Kaehlcke wrote:
> Please make sure to cc the linux-arm-msm@vger list for patches of
> Qualcomm code.
Sure.
> On Fri, Jul 17, 2020 at 08:02:22PM +0530, Akash Asthana wrote:
>> pdev struct doesn't exits for the devices whose status are disabled
> s/exits/exist/
ok
>
>> from DT node, in such cases NULL is returned from 'of_find_device_by_node'
>> Later when we try to get drvdata from pdev struct NULL pointer dereference
>> is triggered.
>>
>> Add a NULL check for return values to fix the issue.
>>
>> We were hitting this issue when one of QUP is disabled.
>>
>> Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to fix earlycon crash")
>> Reported-by: Sai Prakash Ranjan <[email protected]>
>> Signed-off-by: Akash Asthana <[email protected]>
>> ---
>> drivers/soc/qcom/qcom-geni-se.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
>> index 355d503..6e5fe65 100644
>> --- a/drivers/soc/qcom/qcom-geni-se.c
>> +++ b/drivers/soc/qcom/qcom-geni-se.c
>> @@ -820,6 +820,7 @@ void geni_remove_earlycon_icc_vote(void)
>> struct geni_wrapper *wrapper;
>> struct device_node *parent;
>> struct device_node *child;
>> + struct platform_device *wrapper_pdev;
> nit: since there is no other 'pdev' in this function you could just name
> it 'pdev', which is less clunky. The variable is only used immediately
> after it is assigned, so it's clear from the context that it refers to
> the 'wrapper'.
ok
Thankyou for review!.
regards,
Akash
>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
On Fri 17 Jul 07:32 PDT 2020, Akash Asthana wrote:
> pdev struct doesn't exits for the devices whose status are disabled
> from DT node, in such cases NULL is returned from 'of_find_device_by_node'
> Later when we try to get drvdata from pdev struct NULL pointer dereference
> is triggered.
>
> Add a NULL check for return values to fix the issue.
>
> We were hitting this issue when one of QUP is disabled.
>
> Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to fix earlycon crash")
> Reported-by: Sai Prakash Ranjan <[email protected]>
> Signed-off-by: Akash Asthana <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Tested-by: Bjorn Andersson <[email protected]>
And applied with Matthias suggested rename of wrapper_pdev to pdev.
PS. Please include [email protected] among your recipients
for future patches.
Regards,
Bjorn
> ---
> drivers/soc/qcom/qcom-geni-se.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 355d503..6e5fe65 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -820,6 +820,7 @@ void geni_remove_earlycon_icc_vote(void)
> struct geni_wrapper *wrapper;
> struct device_node *parent;
> struct device_node *child;
> + struct platform_device *wrapper_pdev;
>
> if (!earlycon_wrapper)
> return;
> @@ -829,7 +830,12 @@ void geni_remove_earlycon_icc_vote(void)
> for_each_child_of_node(parent, child) {
> if (!of_device_is_compatible(child, "qcom,geni-se-qup"))
> continue;
> - wrapper = platform_get_drvdata(of_find_device_by_node(child));
> +
> + wrapper_pdev = of_find_device_by_node(child);
> + if (!wrapper_pdev)
> + continue;
> +
> + wrapper = platform_get_drvdata(wrapper_pdev);
> icc_put(wrapper->to_core.path);
> wrapper->to_core.path = NULL;
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
>