[PATCH] soc: qcom: geni: Fix NULL pointer dereference

[PATCH] soc: qcom: geni: Fix NULL pointer dereference

[Akash Asthana]

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 <saipraka@codeaurora.org>
Signed-off-by: Akash Asthana <akashast@codeaurora.org>
---
 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

Re: [PATCH] soc: qcom: geni: Fix NULL pointer dereference

[Matthias Kaehlcke]

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 <saipraka@codeaurora.org>
> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
> ---
>  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 <mka@chromium.org>

Re: [PATCH] soc: qcom: geni: Fix NULL pointer dereference

[Akash Asthana]

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 <saipraka@codeaurora.org>
>> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
>> ---
>>   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 <mka@chromium.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

Re: [PATCH] soc: qcom: geni: Fix NULL pointer dereference

[Bjorn Andersson]

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 <saipraka@codeaurora.org>
> Signed-off-by: Akash Asthana <akashast@codeaurora.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>

And applied with Matthias suggested rename of wrapper_pdev to pdev.


PS. Please include linux-arm-msm@vger.kernel.org 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
>