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

[Stephen Boyd]

Quoting Anjelique Melendez (2022-01-20 16:25:26)
>
> 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.
>

Many drivers check that their device_get_match_data() returns a valid
pointer. I'd like to see that API used in addition to checking the
return value for NULL so that we can keep the static analysis tools
happy. Yes it's an impossible case assuming the driver writer didn't
mess up but it shuts SA up and we don't really have a better solution
to tell tools that device_get_match_data() can't return NULL.