Re: [PATCH 3/3] input: misc: pm8941-pwrkey: avoid potential null pointer dereference

[Anjelique Melendez]

On 1/20/2022 3:01 PM, Bjorn Andersson wrote:
> On Thu 20 Jan 12:41 PST 2022, Anjelique Melendez wrote:
>
>> From: David Collins <collinsd@xxxxxxxxxxxxxx>
>>
>> Add a null check for the pwrkey->data pointer after it is assigned
>> in pm8941_pwrkey_probe().  This avoids a potential null pointer
>> dereference when pwrkey->data->has_pon_pbs is accessed later in
>> the probe function.
>>
>> Change-Id: I589c4851e544d79a1863fd110b32a0b45ac03caf
>> Signed-off-by: David Collins <collinsd@xxxxxxxxxxxxxx>
>> Signed-off-by: Anjelique Melendez <quic_amelende@xxxxxxxxxxx>
>> ---
>>  drivers/input/misc/pm8941-pwrkey.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
>> index 0ce00736e695..ac08ed025802 100644
>> --- a/drivers/input/misc/pm8941-pwrkey.c
>> +++ b/drivers/input/misc/pm8941-pwrkey.c
>> @@ -263,6 +263,10 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
>>  
>>  	pwrkey->dev = &pdev->dev;
>>  	pwrkey->data = of_device_get_match_data(&pdev->dev);
>> +	if (!pwrkey->data) {
> The only way this can happen is if you add a new compatible and forget
> to specify data and when that happens you will get a print in the log
> somewhere, which once you realize that you don't have your pwrkey you
> might be able to find among all the other prints.
>
> If you instead don't NULL check this pointer you will get a large splat
> in the log, with callstack and all, immediately hinting you that
> pwrkey->data is NULL.
>
>
> In other words, there's already a print, a much larger print and I don't
> think there's value in handling this mistake gracefully.
>
> Regards,
> Bjorn


We would like to the null pointer check in place to avoid static analysis

warnings that can be easily fixed.


>
>> +		dev_err(&pdev->dev, "match data not found\n");
>> +		return -ENODEV;
>> +	}
>>  
>>  	parent = pdev->dev.parent;
>>  	regmap_node = pdev->dev.of_node;
>> -- 
>> 2.34.1
>>