| Message ID | 1546446223-17184-1-git-send-email-peng.hao2@zte.com.cn |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v5] soc/fsl/qe: fix err handling of ucc_of_parse_tdm | expand |
Peng Hao <peng.hao2@zte.com.cn> a écrit : > From: Wen Yang <wen.yang99@zte.com.cn> > > Currently there are some issues with the ucc_of_parse_tdm function: > 1, a possible null pointer dereference in ucc_of_parse_tdm, > detected by the semantic patch deref_null.cocci, > with the following warning: > drivers/soc/fsl/qe/qe_tdm.c:177:21-24: ERROR: pdev is NULL but dereferenced. > 2, dev gets modified, so in any case that devm_iounmap() will fail > even when the new pdev is valid, because the iomap was done with a > different pdev. > 3, there is no driver bind with the "fsl,t1040-qe-si" or > "fsl,t1040-qe-siram" device. So allocating resources using devm_*() > with these devices won't provide a cleanup path for these resources > when the caller fails. > > This patch fixes them. > > Suggested-by: Li Yang <leoyang.li@nxp.com> > Suggested-by: Christophe LEROY <christophe.leroy@c-s.fr> > Signed-off-by: Wen Yang <wen.yang99@zte.com.cn> > Reviewed-by: Peng Hao <peng.hao2@zte.com.cn> > CC: Julia Lawall <julia.lawall@lip6.fr> > CC: Zhao Qiang <qiang.zhao@nxp.com> > CC: David S. Miller <davem@davemloft.net> > CC: netdev@vger.kernel.org > CC: linuxppc-dev@lists.ozlabs.org > CC: linux-kernel@vger.kernel.org > --- In order to ease review, could add the list of changes between each version of the patch ? Usually we do it at this place so that it is available for reviewers but not part of the commit text. Christophe > drivers/net/wan/fsl_ucc_hdlc.c | 62 > +++++++++++++++++++++++++++++++++++++++++- > drivers/soc/fsl/qe/qe_tdm.c | 55 ------------------------------------- > 2 files changed, 61 insertions(+), 56 deletions(-) > > diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c > index 839fa77..f30a040 100644 > --- a/drivers/net/wan/fsl_ucc_hdlc.c > +++ b/drivers/net/wan/fsl_ucc_hdlc.c > @@ -1057,6 +1057,54 @@ static const struct net_device_ops uhdlc_ops = { > .ndo_tx_timeout = uhdlc_tx_timeout, > }; > > +static int hdlc_map_iomem(char *name, int init_flag, void __iomem **ptr) > +{ > + struct device_node *np; > + struct platform_device *pdev; > + struct resource *res; > + static int siram_init_flag; > + int ret = 0; > + > + np = of_find_compatible_node(NULL, NULL, name); > + if (!np) > + return -EINVAL; > + > + pdev = of_find_device_by_node(np); > + if (!pdev) { > + pr_err("%pOFn: failed to lookup pdev\n", np); > + of_node_put(np); > + return -EINVAL; > + } > + > + of_node_put(np); > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + ret = -EINVAL; > + goto error_put_device; > + } > + *ptr = ioremap(res->start, resource_size(res)); > + if (!*ptr) { > + ret = -ENOMEM; > + goto error_put_device; > + } > + > + /* We've remapped the addresses, and we don't need the device any > + * more, so we should release it. > + */ > + put_device(&pdev->dev); > + > + if (init_flag && siram_init_flag == 0) { > + memset_io(*ptr, 0, resource_size(res)); > + siram_init_flag = 1; > + } > + return 0; > + > +error_put_device: > + put_device(&pdev->dev); > + > + return ret; > +} > + > static int ucc_hdlc_probe(struct platform_device *pdev) > { > struct device_node *np = pdev->dev.of_node; > @@ -1151,6 +1199,15 @@ static int ucc_hdlc_probe(struct > platform_device *pdev) > ret = ucc_of_parse_tdm(np, utdm, ut_info); > if (ret) > goto free_utdm; > + > + ret = hdlc_map_iomem("fsl,t1040-qe-si", 0, > + (void __iomem **)&utdm->si_regs); > + if (ret) > + goto free_utdm; > + ret = hdlc_map_iomem("fsl,t1040-qe-siram", 1, > + (void __iomem **)&utdm->siram); > + if (ret) > + goto unmap_si_regs; > } > > if (of_property_read_u16(np, "fsl,hmask", &uhdlc_priv->hmask)) > @@ -1159,7 +1216,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev) > ret = uhdlc_init(uhdlc_priv); > if (ret) { > dev_err(&pdev->dev, "Failed to init uhdlc\n"); > - goto free_utdm; > + goto undo_uhdlc_init; > } > > dev = alloc_hdlcdev(uhdlc_priv); > @@ -1188,6 +1245,9 @@ static int ucc_hdlc_probe(struct platform_device *pdev) > free_dev: > free_netdev(dev); > undo_uhdlc_init: > + iounmap(utdm->siram); > +unmap_si_regs: > + iounmap(utdm->si_regs); > free_utdm: > if (uhdlc_priv->tsa) > kfree(utdm); > diff --git a/drivers/soc/fsl/qe/qe_tdm.c b/drivers/soc/fsl/qe/qe_tdm.c > index f78c346..76480df 100644 > --- a/drivers/soc/fsl/qe/qe_tdm.c > +++ b/drivers/soc/fsl/qe/qe_tdm.c > @@ -44,10 +44,6 @@ int ucc_of_parse_tdm(struct device_node *np, > struct ucc_tdm *utdm, > const char *sprop; > int ret = 0; > u32 val; > - struct resource *res; > - struct device_node *np2; > - static int siram_init_flag; > - struct platform_device *pdev; > > sprop = of_get_property(np, "fsl,rx-sync-clock", NULL); > if (sprop) { > @@ -124,57 +120,6 @@ int ucc_of_parse_tdm(struct device_node *np, > struct ucc_tdm *utdm, > utdm->siram_entry_id = val; > > set_si_param(utdm, ut_info); > - > - np2 = of_find_compatible_node(NULL, NULL, "fsl,t1040-qe-si"); > - if (!np2) > - return -EINVAL; > - > - pdev = of_find_device_by_node(np2); > - if (!pdev) { > - pr_err("%pOFn: failed to lookup pdev\n", np2); > - of_node_put(np2); > - return -EINVAL; > - } > - > - of_node_put(np2); > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - utdm->si_regs = devm_ioremap_resource(&pdev->dev, res); > - if (IS_ERR(utdm->si_regs)) { > - ret = PTR_ERR(utdm->si_regs); > - goto err_miss_siram_property; > - } > - > - np2 = of_find_compatible_node(NULL, NULL, "fsl,t1040-qe-siram"); > - if (!np2) { > - ret = -EINVAL; > - goto err_miss_siram_property; > - } > - > - pdev = of_find_device_by_node(np2); > - if (!pdev) { > - ret = -EINVAL; > - pr_err("%pOFn: failed to lookup pdev\n", np2); > - of_node_put(np2); > - goto err_miss_siram_property; > - } > - > - of_node_put(np2); > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - utdm->siram = devm_ioremap_resource(&pdev->dev, res); > - if (IS_ERR(utdm->siram)) { > - ret = PTR_ERR(utdm->siram); > - goto err_miss_siram_property; > - } > - > - if (siram_init_flag == 0) { > - memset_io(utdm->siram, 0, resource_size(res)); > - siram_init_flag = 1; > - } > - > - return ret; > - > -err_miss_siram_property: > - devm_iounmap(&pdev->dev, utdm->si_regs); > return ret; > } > EXPORT_SYMBOL(ucc_of_parse_tdm); > -- > 2.9.5
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c index 839fa77..f30a040 100644 --- a/drivers/net/wan/fsl_ucc_hdlc.c +++ b/drivers/net/wan/fsl_ucc_hdlc.c @@ -1057,6 +1057,54 @@ static const struct net_device_ops uhdlc_ops = { .ndo_tx_timeout = uhdlc_tx_timeout, }; +static int hdlc_map_iomem(char *name, int init_flag, void __iomem **ptr) +{ + struct device_node *np; + struct platform_device *pdev; + struct resource *res; + static int siram_init_flag; + int ret = 0; + + np = of_find_compatible_node(NULL, NULL, name); + if (!np) + return -EINVAL; + + pdev = of_find_device_by_node(np); + if (!pdev) { + pr_err("%pOFn: failed to lookup pdev\n", np); + of_node_put(np); + return -EINVAL; + } + + of_node_put(np); + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { + ret = -EINVAL; + goto error_put_device; + } + *ptr = ioremap(res->start, resource_size(res)); + if (!*ptr) { + ret = -ENOMEM; + goto error_put_device; + } + + /* We've remapped the addresses, and we don't need the device any + * more, so we should release it. + */ + put_device(&pdev->dev); + + if (init_flag && siram_init_flag == 0) { + memset_io(*ptr, 0, resource_size(res)); + siram_init_flag = 1; + } + return 0; + +error_put_device: + put_device(&pdev->dev); + + return ret; +} + static int ucc_hdlc_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; @@ -1151,6 +1199,15 @@ static int ucc_hdlc_probe(struct platform_device *pdev) ret = ucc_of_parse_tdm(np, utdm, ut_info); if (ret) goto free_utdm; + + ret = hdlc_map_iomem("fsl,t1040-qe-si", 0, + (void __iomem **)&utdm->si_regs); + if (ret) + goto free_utdm; + ret = hdlc_map_iomem("fsl,t1040-qe-siram", 1, + (void __iomem **)&utdm->siram); + if (ret) + goto unmap_si_regs; } if (of_property_read_u16(np, "fsl,hmask", &uhdlc_priv->hmask)) @@ -1159,7 +1216,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev) ret = uhdlc_init(uhdlc_priv); if (ret) { dev_err(&pdev->dev, "Failed to init uhdlc\n"); - goto free_utdm; + goto undo_uhdlc_init; } dev = alloc_hdlcdev(uhdlc_priv); @@ -1188,6 +1245,9 @@ static int ucc_hdlc_probe(struct platform_device *pdev) free_dev: free_netdev(dev); undo_uhdlc_init: + iounmap(utdm->siram); +unmap_si_regs: + iounmap(utdm->si_regs); free_utdm: if (uhdlc_priv->tsa) kfree(utdm); diff --git a/drivers/soc/fsl/qe/qe_tdm.c b/drivers/soc/fsl/qe/qe_tdm.c index f78c346..76480df 100644 --- a/drivers/soc/fsl/qe/qe_tdm.c +++ b/drivers/soc/fsl/qe/qe_tdm.c @@ -44,10 +44,6 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm, const char *sprop; int ret = 0; u32 val; - struct resource *res; - struct device_node *np2; - static int siram_init_flag; - struct platform_device *pdev; sprop = of_get_property(np, "fsl,rx-sync-clock", NULL); if (sprop) { @@ -124,57 +120,6 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm, utdm->siram_entry_id = val; set_si_param(utdm, ut_info); - - np2 = of_find_compatible_node(NULL, NULL, "fsl,t1040-qe-si"); - if (!np2) - return -EINVAL; - - pdev = of_find_device_by_node(np2); - if (!pdev) { - pr_err("%pOFn: failed to lookup pdev\n", np2); - of_node_put(np2); - return -EINVAL; - } - - of_node_put(np2); - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - utdm->si_regs = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(utdm->si_regs)) { - ret = PTR_ERR(utdm->si_regs); - goto err_miss_siram_property; - } - - np2 = of_find_compatible_node(NULL, NULL, "fsl,t1040-qe-siram"); - if (!np2) { - ret = -EINVAL; - goto err_miss_siram_property; - } - - pdev = of_find_device_by_node(np2); - if (!pdev) { - ret = -EINVAL; - pr_err("%pOFn: failed to lookup pdev\n", np2); - of_node_put(np2); - goto err_miss_siram_property; - } - - of_node_put(np2); - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - utdm->siram = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(utdm->siram)) { - ret = PTR_ERR(utdm->siram); - goto err_miss_siram_property; - } - - if (siram_init_flag == 0) { - memset_io(utdm->siram, 0, resource_size(res)); - siram_init_flag = 1; - } - - return ret; - -err_miss_siram_property: - devm_iounmap(&pdev->dev, utdm->si_regs); return ret; } EXPORT_SYMBOL(ucc_of_parse_tdm);