Message ID | 1592885209-25839-1-git-send-email-haibo.chen@nxp.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | mmc: host: dereference null return value | expand |
On 23/06/20 7:06 am, haibo.chen@nxp.com wrote: > From: Haibo Chen <haibo.chen@nxp.com> > > of_match_node() has the opportunity to return NULL, so need to > dereference null return value. > This is reported by Coverity. > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> > --- > drivers/mmc/host/dw_mmc-exynos.c | 5 +++-- > drivers/mmc/host/dw_mmc-k3.c | 5 +++-- > drivers/mmc/host/dw_mmc-pltfm.c | 3 ++- > drivers/mmc/host/sdhci-of-arasan.c | 2 ++ > 4 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c > index 5e3d95b63676..27ab55abb03f 100644 > --- a/drivers/mmc/host/dw_mmc-exynos.c > +++ b/drivers/mmc/host/dw_mmc-exynos.c > @@ -545,12 +545,13 @@ MODULE_DEVICE_TABLE(of, dw_mci_exynos_match); > > static int dw_mci_exynos_probe(struct platform_device *pdev) > { > - const struct dw_mci_drv_data *drv_data; > + const struct dw_mci_drv_data *drv_data = NULL; > const struct of_device_id *match; > int ret; > > match = of_match_node(dw_mci_exynos_match, pdev->dev.of_node); > - drv_data = match->data; > + if (match) > + drv_data = match->data; Could be 1 line change: drv_data = match ? match->data : NULL; > > pm_runtime_get_noresume(&pdev->dev); > pm_runtime_set_active(&pdev->dev); > diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c > index 50977ff18074..e8a148c306b3 100644 > --- a/drivers/mmc/host/dw_mmc-k3.c > +++ b/drivers/mmc/host/dw_mmc-k3.c > @@ -451,11 +451,12 @@ MODULE_DEVICE_TABLE(of, dw_mci_k3_match); > > static int dw_mci_k3_probe(struct platform_device *pdev) > { > - const struct dw_mci_drv_data *drv_data; > + const struct dw_mci_drv_data *drv_data = NULL; > const struct of_device_id *match; > > match = of_match_node(dw_mci_k3_match, pdev->dev.of_node); > - drv_data = match->data; > + if (match) > + drv_data = match->data; Could be 1 line change: drv_data = match ? match->data : NULL; > > return dw_mci_pltfm_register(pdev, drv_data); > } > diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c > index 7de37f524a96..d3dcb96efd13 100644 > --- a/drivers/mmc/host/dw_mmc-pltfm.c > +++ b/drivers/mmc/host/dw_mmc-pltfm.c > @@ -78,7 +78,8 @@ static int dw_mci_pltfm_probe(struct platform_device *pdev) > > if (pdev->dev.of_node) { > match = of_match_node(dw_mci_pltfm_match, pdev->dev.of_node); > - drv_data = match->data; > + if (match) > + drv_data = match->data; > } > > return dw_mci_pltfm_register(pdev, drv_data); > diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c > index fb26e743e1fd..f2090f944a0e 100644 > --- a/drivers/mmc/host/sdhci-of-arasan.c > +++ b/drivers/mmc/host/sdhci-of-arasan.c > @@ -1520,6 +1520,8 @@ static int sdhci_arasan_probe(struct platform_device *pdev) > const struct sdhci_arasan_of_data *data; > > match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node); > + if (match == NULL) (!match) seems to be preferred over (match == NULL) > + return -ENOPARAM; ENOPARAM is unconventional here. ENODEV or EINVAL are better > data = match->data; > host = sdhci_pltfm_init(pdev, data->pdata, sizeof(*sdhci_arasan)); > >
Hi, On 23.06.2020 06:06, haibo.chen@nxp.com wrote: > From: Haibo Chen <haibo.chen@nxp.com> > > of_match_node() has the opportunity to return NULL, so need to > dereference null return value. > This is reported by Coverity. > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> There is no point in such check for a NULL. The driver won't be instantiated/probed if there is no matching node found first by the upper level framework. If you really want to make this code cleaner, please change it to use of_device_get_match_data(). > --- > drivers/mmc/host/dw_mmc-exynos.c | 5 +++-- > drivers/mmc/host/dw_mmc-k3.c | 5 +++-- > drivers/mmc/host/dw_mmc-pltfm.c | 3 ++- > drivers/mmc/host/sdhci-of-arasan.c | 2 ++ > 4 files changed, 10 insertions(+), 5 deletions(-) > ... Best regards
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c index 5e3d95b63676..27ab55abb03f 100644 --- a/drivers/mmc/host/dw_mmc-exynos.c +++ b/drivers/mmc/host/dw_mmc-exynos.c @@ -545,12 +545,13 @@ MODULE_DEVICE_TABLE(of, dw_mci_exynos_match); static int dw_mci_exynos_probe(struct platform_device *pdev) { - const struct dw_mci_drv_data *drv_data; + const struct dw_mci_drv_data *drv_data = NULL; const struct of_device_id *match; int ret; match = of_match_node(dw_mci_exynos_match, pdev->dev.of_node); - drv_data = match->data; + if (match) + drv_data = match->data; pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c index 50977ff18074..e8a148c306b3 100644 --- a/drivers/mmc/host/dw_mmc-k3.c +++ b/drivers/mmc/host/dw_mmc-k3.c @@ -451,11 +451,12 @@ MODULE_DEVICE_TABLE(of, dw_mci_k3_match); static int dw_mci_k3_probe(struct platform_device *pdev) { - const struct dw_mci_drv_data *drv_data; + const struct dw_mci_drv_data *drv_data = NULL; const struct of_device_id *match; match = of_match_node(dw_mci_k3_match, pdev->dev.of_node); - drv_data = match->data; + if (match) + drv_data = match->data; return dw_mci_pltfm_register(pdev, drv_data); } diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c index 7de37f524a96..d3dcb96efd13 100644 --- a/drivers/mmc/host/dw_mmc-pltfm.c +++ b/drivers/mmc/host/dw_mmc-pltfm.c @@ -78,7 +78,8 @@ static int dw_mci_pltfm_probe(struct platform_device *pdev) if (pdev->dev.of_node) { match = of_match_node(dw_mci_pltfm_match, pdev->dev.of_node); - drv_data = match->data; + if (match) + drv_data = match->data; } return dw_mci_pltfm_register(pdev, drv_data); diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c index fb26e743e1fd..f2090f944a0e 100644 --- a/drivers/mmc/host/sdhci-of-arasan.c +++ b/drivers/mmc/host/sdhci-of-arasan.c @@ -1520,6 +1520,8 @@ static int sdhci_arasan_probe(struct platform_device *pdev) const struct sdhci_arasan_of_data *data; match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node); + if (match == NULL) + return -ENOPARAM; data = match->data; host = sdhci_pltfm_init(pdev, data->pdata, sizeof(*sdhci_arasan));