Re: [bug report] Input: add support for Azoteq IQS7222A/B/C

[Jeff LaBundy <jeff@labundy.com>]

Hi Dan,

Thank you for reaching out.

On Tue, Apr 12, 2022 at 11:01:23AM +0300, Dan Carpenter wrote:
> There are a couple other warnings as well:
> 
> drivers/input/misc/iqs7222.c:2214 iqs7222_parse_all() error: NULL dereference inside function 'iqs7222_parse_props()
> drivers/input/misc/iqs7222.c:2234 iqs7222_parse_all() error: NULL dereference inside function 'iqs7222_parse_props()

All three of these code paths are tested and do not lead to a crash; I
am suspicious that the warnings are false positives.

> 
> regards,
> dan carpenter
> 
> On Tue, Apr 12, 2022 at 10:59:37AM +0300, Dan Carpenter wrote:
> > Hello Jeff LaBundy,
> > 
> > The patch e505edaedcb9: "Input: add support for Azoteq IQS7222A/B/C"
> > from Apr 8, 2022, leads to the following Smatch static checker
> > warning:
> > 
> > 	drivers/input/misc/iqs7222.c:2166 iqs7222_parse_all()
> > 	error: NULL dereference inside function 'iqs7222_parse_props()'
> > 
> > drivers/input/misc/iqs7222.c
> >     2150 static int iqs7222_parse_all(struct iqs7222_private *iqs7222)
> >     2151 {
> >     2152         const struct iqs7222_dev_desc *dev_desc = iqs7222->dev_desc;
> >     2153         const struct iqs7222_reg_grp_desc *reg_grps = dev_desc->reg_grps;
> >     2154         u16 *sys_setup = iqs7222->sys_setup;
> >     2155         int error, i;
> >     2156 
> >     2157         if (dev_desc->event_offset)
> >     2158                 sys_setup[dev_desc->event_offset] = IQS7222_EVENT_MASK_ATI;
> >     2159 
> >     2160         for (i = 0; i < reg_grps[IQS7222_REG_GRP_CYCLE].num_row; i++) {
> >     2161                 error = iqs7222_parse_cycle(iqs7222, i);
> >     2162                 if (error)
> >     2163                         return error;
> >     2164         }
> >     2165 
> > --> 2166         error = iqs7222_parse_props(iqs7222, NULL, 0, IQS7222_REG_GRP_GLBL,
> >                                                       ^^^^
> > This NULL is dereferenced inside iqs7222_parse_props() so this will
> > crash.

By design, all calls to iqs7222_parse_props() with **child_node equal
to NULL are accompanied by a value of reg_grp that prevents the pointer
from being dereferenced.

For these specific cases, the pointer is reassigned by way of a switch
block at line 1542 before being dereferenced by fwnode_property_*().

> > 
> >     2167                                     IQS7222_REG_KEY_NONE);
> >     2168         if (error)
> >     2169                 return error;
> >     2170 
> >     2171         for (i = 0; i < reg_grps[IQS7222_REG_GRP_GPIO].num_row; i++) {
> >     2172                 struct fwnode_handle *gpio_node = NULL;
> > 
> > regards,
> > dan carpenter

Please let me know in case I have misunderstood or you feel that I can
make any improvements.

Kind regards,
Jeff LaBundy