Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1537714 > unrolled thread

[PATCH] extcon: palmas: Fail gracefully if invalid configuration

Started byRoger Quadros <rogerq@ti.com>
First post2016-12-07 13:20 +0100
Last post2016-12-08 09:50 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] extcon: palmas: Fail gracefully if invalid configuration Roger Quadros <rogerq@ti.com> - 2016-12-07 13:20 +0100
    Re: [PATCH] extcon: palmas: Fail gracefully if invalid configuration Chanwoo Choi <cw00.choi@samsung.com> - 2016-12-07 13:50 +0100
      Re: [PATCH] extcon: palmas: Fail gracefully if invalid configuration Roger Quadros <rogerq@ti.com> - 2016-12-08 09:30 +0100
    [PATCH v2] extcon: palmas: Check the parent instance to prevent the  NULL Roger Quadros <rogerq@ti.com> - 2016-12-08 09:50 +0100

#1537714 — [PATCH] extcon: palmas: Fail gracefully if invalid configuration

FromRoger Quadros <rogerq@ti.com>
Date2016-12-07 13:20 +0100
Subject[PATCH] extcon: palmas: Fail gracefully if invalid configuration
Message-ID<sLJmx-8sl-11@gated-at.bofh.it>
extcon-palmas must be child of palmas and expects parent's
drvdata to be valid. Check for non NULL parent drvdata and
fail if it is NULL. Not doing so will result in a NULL
pointer dereference later in the probe() parent drvdata
is NULL (e.g. misplaced extcon-palmas node in device tree).

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/extcon/extcon-palmas.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
index 634ba70..ec987ab 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -190,6 +190,11 @@ static int palmas_usb_probe(struct platform_device *pdev)
 	struct palmas_usb *palmas_usb;
 	int status;
 
+	if (!palmas) {
+		dev_err(&pdev->dev, "device has invalid parent\n");
+		return -EINVAL;
+	}
+
 	palmas_usb = devm_kzalloc(&pdev->dev, sizeof(*palmas_usb), GFP_KERNEL);
 	if (!palmas_usb)
 		return -ENOMEM;
-- 
2.7.4

[toc] | [next] | [standalone]

#1537725

FromChanwoo Choi <cw00.choi@samsung.com>
Date2016-12-07 13:50 +0100
Message-ID<sLJPz-a6-11@gated-at.bofh.it>
In reply to#1537714
Hi Roger,

Looks good to me.
But I have some comment.

How about changing the subject as following?

- old : Fail gracefully if invalid configuration
- new : Check the parent instance to prevent the NULL pointer error

On 2016년 12월 07일 21:12, Roger Quadros wrote:
> extcon-palmas must be child of palmas and expects parent's
> drvdata to be valid. Check for non NULL parent drvdata and
> fail if it is NULL. Not doing so will result in a NULL
> pointer dereference later in the probe() parent drvdata
> is NULL (e.g. misplaced extcon-palmas node in device tree).
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/extcon/extcon-palmas.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
> index 634ba70..ec987ab 100644
> --- a/drivers/extcon/extcon-palmas.c
> +++ b/drivers/extcon/extcon-palmas.c
> @@ -190,6 +190,11 @@ static int palmas_usb_probe(struct platform_device *pdev)
>  	struct palmas_usb *palmas_usb;
>  	int status;
>  
> +	if (!palmas) {
> +		dev_err(&pdev->dev, "device has invalid parent\n");

How about changing the error message as following?
because the extcon-palmas used the 'failed to ..' style for error message.
- "failed to get valid parent"

> +		return -EINVAL;
> +	}
> +
>  	palmas_usb = devm_kzalloc(&pdev->dev, sizeof(*palmas_usb), GFP_KERNEL);
>  	if (!palmas_usb)
>  		return -ENOMEM;
> 


Best Regards,
Chanwoo Choi

[toc] | [prev] | [next] | [standalone]

#1538368

FromRoger Quadros <rogerq@ti.com>
Date2016-12-08 09:30 +0100
Message-ID<sM2fv-3Ni-19@gated-at.bofh.it>
In reply to#1537725
Hi Chanwoo,

On 07/12/16 14:46, Chanwoo Choi wrote:
> Hi Roger,
> 
> Looks good to me.
> But I have some comment.
> 
> How about changing the subject as following?
> 
> - old : Fail gracefully if invalid configuration
> - new : Check the parent instance to prevent the NULL pointer error

I'm OK with this.

> 
> On 2016년 12월 07일 21:12, Roger Quadros wrote:
>> extcon-palmas must be child of palmas and expects parent's
>> drvdata to be valid. Check for non NULL parent drvdata and
>> fail if it is NULL. Not doing so will result in a NULL
>> pointer dereference later in the probe() parent drvdata
>> is NULL (e.g. misplaced extcon-palmas node in device tree).
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  drivers/extcon/extcon-palmas.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
>> index 634ba70..ec987ab 100644
>> --- a/drivers/extcon/extcon-palmas.c
>> +++ b/drivers/extcon/extcon-palmas.c
>> @@ -190,6 +190,11 @@ static int palmas_usb_probe(struct platform_device *pdev)
>>  	struct palmas_usb *palmas_usb;
>>  	int status;
>>  
>> +	if (!palmas) {
>> +		dev_err(&pdev->dev, "device has invalid parent\n");
> 
> How about changing the error message as following?
> because the extcon-palmas used the 'failed to ..' style for error message.
> - "failed to get valid parent"

This is also fine.

I'll send a v2.

> 
>> +		return -EINVAL;
>> +	}
>> +
>>  	palmas_usb = devm_kzalloc(&pdev->dev, sizeof(*palmas_usb), GFP_KERNEL);
>>  	if (!palmas_usb)
>>  		return -ENOMEM;
>>
> 
> 

--
cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1538380 — [PATCH v2] extcon: palmas: Check the parent instance to prevent the NULL

FromRoger Quadros <rogerq@ti.com>
Date2016-12-08 09:50 +0100
Subject[PATCH v2] extcon: palmas: Check the parent instance to prevent the NULL
Message-ID<sM2yS-3Vu-7@gated-at.bofh.it>
In reply to#1537714
extcon-palmas must be child of palmas and expects parent's
drvdata to be valid. Check for non NULL parent drvdata and
fail if it is NULL. Not doing so will result in a NULL
pointer dereference later in the probe() parent drvdata
is NULL (e.g. misplaced extcon-palmas node in device tree).

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
Changelog:
v2:
- updated error message and commit message subject

 drivers/extcon/extcon-palmas.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
index 634ba70..a128fd2 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -190,6 +190,11 @@ static int palmas_usb_probe(struct platform_device *pdev)
 	struct palmas_usb *palmas_usb;
 	int status;
 
+	if (!palmas) {
+		dev_err(&pdev->dev, "failed to get valid parent\n");
+		return -EINVAL;
+	}
+
 	palmas_usb = devm_kzalloc(&pdev->dev, sizeof(*palmas_usb), GFP_KERNEL);
 	if (!palmas_usb)
 		return -ENOMEM;
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web