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: <e2015c19-b73b-39a7-ba73-708b2c4552c7@quicinc.com>
Date:   Thu, 20 Jan 2022 16:25:26 -0800
From:   Anjelique Melendez <quic_amelende@...cinc.com>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
CC:     <dmitry.torokhov@...il.com>, <linux-input@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <collinsd@...eaurora.org>, <swboyd@...omium.org>,
        <skakit@...eaurora.org>
Subject: Re: [PATCH 3/3] input: misc: pm8941-pwrkey: avoid potential null
 pointer dereference


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@...eaurora.org>
>>
>> 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@...eaurora.org>
>> Signed-off-by: Anjelique Melendez <quic_amelende@...cinc.com>
>> ---
>>  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
>>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ