[-next] iommu/exynos: Fix return value check in exynos_iommu_of_setup()
diff mbox

Message ID 1467807354-26503-1-git-send-email-weiyj_lk@163.com
State Not Applicable
Headers show

Commit Message

weiyj_lk@163.com July 6, 2016, 12:15 p.m. UTC
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

In case of error, the function of_platform_device_create() returns
NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
check should be replaced with NULL test.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/iommu/exynos-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Dennis Chen July 6, 2016, 12:47 p.m. UTC | #1
On Wed, Jul 06, 2016 at 12:15:54PM +0000, weiyj_lk@163.com wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> In case of error, the function of_platform_device_create() returns
> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
> check should be replaced with NULL test.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
>  drivers/iommu/exynos-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 33dcc29..9b23059 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
>               exynos_iommu_init();
>
>       pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> -     if (IS_ERR(pdev))
> -             return PTR_ERR(pdev);
> +     if (!pdev)
> +             return -ENOMEM;
>
Indeed!If of_platform_device_create only returns NULL in case of failure, then IS_ERR() will be
false, so
Reviewed-by: Dennis Chen <dennis.chen@arm.com>

Thanks,
Dennis
>
>       /*
>        * use the first registered sysmmu device for performing
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski July 6, 2016, 1:09 p.m. UTC | #2
On 07/06/2016 02:15 PM, weiyj_lk@163.com wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> In case of error, the function of_platform_device_create() returns
> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
> check should be replaced with NULL test.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
>  drivers/iommu/exynos-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 33dcc29..9b23059 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
>  		exynos_iommu_init();
>  
>  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> -	if (IS_ERR(pdev))
> -		return PTR_ERR(pdev);
> +	if (!pdev)
> +		return -ENOMEM;
>  
>  	/*
>  	 * use the first registered sysmmu device for performing
> 

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>


Which in case of first sysmmu device and of_platform_device_create()
failure will lead to NULL pointer exception, so I think it is
appropriate to:
Fixes: 8ed55c812fa8 ("iommu/exynos: Init from dt-specific callback
instead of initcall")
Cc: <stable@vger.kernel.org>

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andi Shyti July 7, 2016, 2:34 a.m. UTC | #3
Hi,

just a question,

> >  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> > -	if (IS_ERR(pdev))
> > -		return PTR_ERR(pdev);
> > +	if (!pdev)
> > +		return -ENOMEM;
> >  
> >  	/*
> >  	 * use the first registered sysmmu device for performing
> > 
> 

is ENOMEM the right return value? of_platform_device_create()
doesn't fail only in case of malloc failure.

Shouldn't it be ENODEV instead, which might also mean with some
imagination that there is no memory available for that device?

Thanks,
Andi

> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
weiyj_lk@163.com July 7, 2016, 2:50 a.m. UTC | #4
Hi,

> just a question,
>
>>>  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
>>> -	if (IS_ERR(pdev))
>>> -		return PTR_ERR(pdev);
>>> +	if (!pdev)
>>> +		return -ENOMEM;
>>>  
>>>  	/*
>>>  	 * use the first registered sysmmu device for performing
>>>
> is ENOMEM the right return value? of_platform_device_create()
> doesn't fail only in case of malloc failure.
>
> Shouldn't it be ENODEV instead, which might also mean with some
> imagination that there is no memory available for that device?
>
Your are right, I will change the error to -ENODEV and resend the patch.

Thanks,
Wei Yongjun




--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Szyprowski July 18, 2016, 7:34 a.m. UTC | #5
Hello,


On 2016-07-06 15:09, Krzysztof Kozlowski wrote:
> On 07/06/2016 02:15 PM, weiyj_lk@163.com wrote:
>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>
>> In case of error, the function of_platform_device_create() returns
>> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
>> check should be replaced with NULL test.
>>
>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>> ---
>>   drivers/iommu/exynos-iommu.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
>> index 33dcc29..9b23059 100644
>> --- a/drivers/iommu/exynos-iommu.c
>> +++ b/drivers/iommu/exynos-iommu.c
>> @@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
>>   		exynos_iommu_init();
>>   
>>   	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
>> -	if (IS_ERR(pdev))
>> -		return PTR_ERR(pdev);
>> +	if (!pdev)
>> +		return -ENOMEM;
>>   
>>   	/*
>>   	 * use the first registered sysmmu device for performing
>>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>
>
> Which in case of first sysmmu device and of_platform_device_create()
> failure will lead to NULL pointer exception, so I think it is
> appropriate to:
> Fixes: 8ed55c812fa8 ("iommu/exynos: Init from dt-specific callback
> instead of initcall")
> Cc: <stable@vger.kernel.org>

Thanks for spotting this issue.

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

Best regards

Patch
diff mbox

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 33dcc29..9b23059 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1345,8 +1345,8 @@  static int __init exynos_iommu_of_setup(struct device_node *np)
 		exynos_iommu_init();
 
 	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
+	if (!pdev)
+		return -ENOMEM;
 
 	/*
 	 * use the first registered sysmmu device for performing