linux-kernel - Re: [PATCH] extcon: palmas: Fail gracefully if invalid configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cd35e82d-3a68-204e-99c6-9c4dc3cba394@ti.com>
Date:   Thu, 8 Dec 2016 10:26:50 +0200
From:   Roger Quadros <rogerq@...com>
To:     Chanwoo Choi <cw00.choi@...sung.com>, <myungjoo.ham@...sung.com>
CC:     <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] extcon: palmas: Fail gracefully if invalid configuration

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@...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