soc/fsl/qe: fix err handling of ucc_of_parse_tdm
authorWen Yang <wen.yang99@zte.com.cn>
Wed, 2 Jan 2019 17:09:53 +0000 (01:09 +0800)
committerDavid S. Miller <davem@davemloft.net>
Fri, 4 Jan 2019 20:50:43 +0000 (12:50 -0800)
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
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/wan/fsl_ucc_hdlc.c
drivers/soc/fsl/qe/qe_tdm.c

index 839fa77..f30a040 100644 (file)
@@ -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);
index f78c346..76480df 100644 (file)
@@ -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);