Messages in this thread | | | Date | Wed, 24 May 2017 08:22:58 +0100 | From | Lee Jones <> | Subject | Re: [mfd] question about potential null pointer dereference |
| |
On Tue, 23 May 2017, Gustavo A. R. Silva wrote:
> > Hello everybody, > > While looking into Coverity ID 1408830 I ran into the following piece of > code at drivers/mfd/wm831x-spi.c:26 > > 26static int wm831x_spi_probe(struct spi_device *spi) > 27{ > 28 struct wm831x_pdata *pdata = dev_get_platdata(&spi->dev); > 29 const struct spi_device_id *id = spi_get_device_id(spi); > 30 const struct of_device_id *of_id; > 31 struct wm831x *wm831x; > 32 enum wm831x_parent type; > 33 int ret; > 34 > 35 if (spi->dev.of_node) { > 36 of_id = of_match_device(wm831x_of_match, &spi->dev); > 37 type = (enum wm831x_parent)of_id->data; > 38 } else { > 39 type = (enum wm831x_parent)id->driver_data; > 40 } > 41 > 42 wm831x = devm_kzalloc(&spi->dev, sizeof(struct wm831x), > GFP_KERNEL); > 43 if (wm831x == NULL) > 44 return -ENOMEM; > 45 > 46 spi->mode = SPI_MODE_0; > 47 > 48 spi_set_drvdata(spi, wm831x); > 49 wm831x->dev = &spi->dev; > 50 wm831x->type = type; > 51 > 52 wm831x->regmap = devm_regmap_init_spi(spi, &wm831x_regmap_config); > 53 if (IS_ERR(wm831x->regmap)) { > 54 ret = PTR_ERR(wm831x->regmap); > 55 dev_err(wm831x->dev, "Failed to allocate register map: > %d\n", > 56 ret); > 57 return ret; > 58 } > 59 > 60 if (pdata) > 61 memcpy(&wm831x->pdata, pdata, sizeof(*pdata)); > 62 > 63 return wm831x_device_init(wm831x, spi->irq); > 64} > > The issue here is that there is a potential NULL pointer dereference at line > 37, in case function of_match_device() returns NULL. > > Maybe a patch like the following could be applied in order to avoid any > chance of a NULL pointer dereference:
I do not believe it's possible for of_match_device() to return NULL in this case.
However, if you wanted to submit a patch checking for it, it would not be rejected.
> index c332e28..7b227c9 100644 > --- a/drivers/mfd/wm831x-spi.c > +++ b/drivers/mfd/wm831x-spi.c > @@ -34,6 +34,10 @@ static int wm831x_spi_probe(struct spi_device *spi) > > if (spi->dev.of_node) { > of_id = of_match_device(wm831x_of_match, &spi->dev); > + if (!of_id) { > + dev_err(&spi->dev, "Failed to find matching id\n");
"Failed to match device"
> + return -EINVAL;
-ENODEV
> type = (enum wm831x_parent)of_id->data; > } else { > type = (enum wm831x_parent)id->driver_data; > > What do you think? > > I'd really appreciate any comment on this. > > Thank you!
-- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
| |