| Message ID | aa54ca91f2310ecea413daa289ab882cf9f37245.1544188058.git.mchehab+samsung@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] media: pxa_camera: don't deferenciate a NULL pointer | expand |
Hi Mauro, On Fri, Dec 07, 2018 at 08:07:54AM -0500, Mauro Carvalho Chehab wrote: > As warned by smatch: > drivers/media/platform/pxa_camera.c:2400 pxa_camera_probe() error: we previously assumed 'pcdev->pdata' could be null (see line 2397) > > It would be possible that neither DT nor platform data would be > provided. This is a Kernel bug, so warn about that and bail. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > --- > drivers/media/platform/pxa_camera.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c > index 5f930560eb30..f91f8fd424c4 100644 > --- a/drivers/media/platform/pxa_camera.c > +++ b/drivers/media/platform/pxa_camera.c > @@ -2396,6 +2396,9 @@ static int pxa_camera_probe(struct platform_device *pdev) > pcdev->pdata = pdev->dev.platform_data; > if (pdev->dev.of_node && !pcdev->pdata) { > err = pxa_camera_pdata_from_dt(&pdev->dev, pcdev, &pcdev->asd); > + } else if (!pcdev->pdata) { This fixes the issue, but the current checks remain a bit odd. The driver seems to prefer platform data over OF. I wonder if that's intentional or not. In that case, I'd roughly write this as: if (pcdev->pdata) { ...; } else if (pdev->dev.of_node) { ...; } else { return -ENODEV; } I'm not sure WARN_ON(1) is necessary. A lot of drivers simply do it this way without WARN_ON(). > + WARN_ON(1); > + return -ENODEV; > } else { > pcdev->platform_flags = pcdev->pdata->flags; > pcdev->mclk = pcdev->pdata->mclk_10khz * 10000;
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c index 5f930560eb30..f91f8fd424c4 100644 --- a/drivers/media/platform/pxa_camera.c +++ b/drivers/media/platform/pxa_camera.c @@ -2396,6 +2396,9 @@ static int pxa_camera_probe(struct platform_device *pdev) pcdev->pdata = pdev->dev.platform_data; if (pdev->dev.of_node && !pcdev->pdata) { err = pxa_camera_pdata_from_dt(&pdev->dev, pcdev, &pcdev->asd); + } else if (!pcdev->pdata) { + WARN_ON(1); + return -ENODEV; } else { pcdev->platform_flags = pcdev->pdata->flags; pcdev->mclk = pcdev->pdata->mclk_10khz * 10000;
As warned by smatch: drivers/media/platform/pxa_camera.c:2400 pxa_camera_probe() error: we previously assumed 'pcdev->pdata' could be null (see line 2397) It would be possible that neither DT nor platform data would be provided. This is a Kernel bug, so warn about that and bail. Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> --- drivers/media/platform/pxa_camera.c | 3 +++ 1 file changed, 3 insertions(+)