[PATCH] memory: fsl_ifc: fix stale global pointer on probe failure/remove

[Reid D]

memory: fsl_ifc: fix stale global pointer on probe failure/remove

fsl_ifc_ctrl_dev is a global pointer. It was set at the start of
fsl_ifc_ctrl_probe(), before anything could fail. If probe failed
partway through, or the device was later removed, the global was
never cleared. fsl_ifc_find() only checks it for non-NULL, so it
could end up dereferencing freed memory or an unmapped register
region.

Use a local variable while setting up the controller struct in
probe, and only publish it to the global once probe has fully
succeeded. Clear the global as the first step of remove, before
any teardown runs. Also guard the free_irq() of the optional
nand_irq, which may be zero.

Signed-off-by: Reid Downing (b1nv0id) <skibletz@xxxxxxxxx>
---
diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
index e89e0c6..ff52cae 100644
--- a/drivers/memory/fsl_ifc.c
+++ b/drivers/memory/fsl_ifc.c
@@ -88,8 +88,11 @@ static void fsl_ifc_ctrl_remove(struct platform_device *dev)
 {
     struct fsl_ifc_ctrl *ctrl = dev_get_drvdata(&dev->dev);

+    fsl_ifc_ctrl_dev = NULL;
+
     of_platform_depopulate(&dev->dev);
-    free_irq(ctrl->nand_irq, ctrl);
+    if (ctrl->nand_irq)
+        free_irq(ctrl->nand_irq, ctrl);
     free_irq(ctrl->irq, ctrl);

     irq_dispose_mapping(ctrl->nand_irq);
@@ -201,85 +204,82 @@ static irqreturn_t fsl_ifc_ctrl_irq(int irqno, void *data)
  */
 static int fsl_ifc_ctrl_probe(struct platform_device *dev)
 {
+    struct fsl_ifc_ctrl *ctrl;
     int ret = 0;
     int version, banks;
     void __iomem *addr;

     dev_info(&dev->dev, "Freescale Integrated Flash Controller\n");

-    fsl_ifc_ctrl_dev = devm_kzalloc(&dev->dev, sizeof(*fsl_ifc_ctrl_dev),
-                    GFP_KERNEL);
-    if (!fsl_ifc_ctrl_dev)
+    ctrl = devm_kzalloc(&dev->dev, sizeof(*ctrl), GFP_KERNEL);
+    if (!ctrl)
         return -ENOMEM;

-    dev_set_drvdata(&dev->dev, fsl_ifc_ctrl_dev);
+    dev_set_drvdata(&dev->dev, ctrl);

     /* IOMAP the entire IFC region */
-    fsl_ifc_ctrl_dev->gregs = of_iomap(dev->dev.of_node, 0);
-    if (!fsl_ifc_ctrl_dev->gregs) {
+    ctrl->gregs = of_iomap(dev->dev.of_node, 0);
+    if (!ctrl->gregs) {
         dev_err(&dev->dev, "failed to get memory region\n");
         return -ENODEV;
     }

     if (of_property_read_bool(dev->dev.of_node, "little-endian")) {
-        fsl_ifc_ctrl_dev->little_endian = true;
+        ctrl->little_endian = true;
         dev_dbg(&dev->dev, "IFC REGISTERS are LITTLE endian\n");
     } else {
-        fsl_ifc_ctrl_dev->little_endian = false;
+        ctrl->little_endian = false;
         dev_dbg(&dev->dev, "IFC REGISTERS are BIG endian\n");
     }

-    version = ifc_in32(&fsl_ifc_ctrl_dev->gregs->ifc_rev) &
-            FSL_IFC_VERSION_MASK;
+    version = ifc_in32(&ctrl->gregs->ifc_rev) & FSL_IFC_VERSION_MASK;

     banks = (version == FSL_IFC_VERSION_1_0_0) ? 4 : 8;
     dev_info(&dev->dev, "IFC version %d.%d, %d banks\n",
         version >> 24, (version >> 16) & 0xf, banks);

-    fsl_ifc_ctrl_dev->version = version;
-    fsl_ifc_ctrl_dev->banks = banks;
+    ctrl->version = version;
+    ctrl->banks = banks;

-    addr = fsl_ifc_ctrl_dev->gregs;
+    addr = ctrl->gregs;
     if (version >= FSL_IFC_VERSION_2_0_0)
         addr += PGOFFSET_64K;
     else
         addr += PGOFFSET_4K;
-    fsl_ifc_ctrl_dev->rregs = addr;
+    ctrl->rregs = addr;

     /* get the Controller level irq */
-    fsl_ifc_ctrl_dev->irq = irq_of_parse_and_map(dev->dev.of_node, 0);
-    if (fsl_ifc_ctrl_dev->irq == 0) {
+    ctrl->irq = irq_of_parse_and_map(dev->dev.of_node, 0);
+    if (ctrl->irq == 0) {
         dev_err(&dev->dev, "failed to get irq resource for IFC\n");
         ret = -ENODEV;
         goto err;
     }

     /* get the nand machine irq */
-    fsl_ifc_ctrl_dev->nand_irq =
-            irq_of_parse_and_map(dev->dev.of_node, 1);
+    ctrl->nand_irq = irq_of_parse_and_map(dev->dev.of_node, 1);

-    fsl_ifc_ctrl_dev->dev = &dev->dev;
+    ctrl->dev = &dev->dev;

-    ret = fsl_ifc_ctrl_init(fsl_ifc_ctrl_dev);
+    ret = fsl_ifc_ctrl_init(ctrl);
     if (ret < 0)
         goto err_unmap_nandirq;

-    init_waitqueue_head(&fsl_ifc_ctrl_dev->nand_wait);
+    init_waitqueue_head(&ctrl->nand_wait);

-    ret = request_irq(fsl_ifc_ctrl_dev->irq, fsl_ifc_ctrl_irq, IRQF_SHARED,
-              "fsl-ifc", fsl_ifc_ctrl_dev);
+    ret = request_irq(ctrl->irq, fsl_ifc_ctrl_irq, IRQF_SHARED,
+              "fsl-ifc", ctrl);
     if (ret != 0) {
-        dev_err(&dev->dev, "failed to install irq (%d)\n",
-            fsl_ifc_ctrl_dev->irq);
+        dev_err(&dev->dev, "failed to install irq (%d)\n", ctrl->irq);
         goto err_unmap_nandirq;
     }

-    if (fsl_ifc_ctrl_dev->nand_irq) {
-        ret = request_irq(fsl_ifc_ctrl_dev->nand_irq, fsl_ifc_nand_irq,
-                0, "fsl-ifc-nand", fsl_ifc_ctrl_dev);
+    if (ctrl->nand_irq) {
+        ret = request_irq(ctrl->nand_irq, fsl_ifc_nand_irq,
+                0, "fsl-ifc-nand", ctrl);
         if (ret != 0) {
             dev_err(&dev->dev, "failed to install irq (%d)\n",
-                fsl_ifc_ctrl_dev->nand_irq);
+                ctrl->nand_irq);
             goto err_free_irq;
         }
     }
@@ -289,17 +289,21 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
     if (ret)
         goto err_free_nandirq;

+    /* Only publish the global once probe has fully succeeded. */
+    fsl_ifc_ctrl_dev = ctrl;
+
     return 0;

 err_free_nandirq:
-    free_irq(fsl_ifc_ctrl_dev->nand_irq, fsl_ifc_ctrl_dev);
+    if (ctrl->nand_irq)
+        free_irq(ctrl->nand_irq, ctrl);
 err_free_irq:
-    free_irq(fsl_ifc_ctrl_dev->irq, fsl_ifc_ctrl_dev);
+    free_irq(ctrl->irq, ctrl);
 err_unmap_nandirq:
-    irq_dispose_mapping(fsl_ifc_ctrl_dev->nand_irq);
-    irq_dispose_mapping(fsl_ifc_ctrl_dev->irq);
+    irq_dispose_mapping(ctrl->nand_irq);
+    irq_dispose_mapping(ctrl->irq);
 err:
-    iounmap(fsl_ifc_ctrl_dev->gregs);
+    iounmap(ctrl->gregs);
     return ret;
 }