[PATCH] memory: fsl_ifc: Clear fsl_ifc_ctrl_dev on probe failure and removal

[Krzysztof Kozlowski]

Global fsl_ifc_ctrl_dev is allocated with devm_kzalloc(), so it is freed
when probe fails or when the device is unbound, but the exported global
pointer is never reset.  Subsequent calls to fsl_ifc_find() or probing
of fsl_ifc_nand would then pass the NULL check and dereference freed
memory.  Issue is mostly theoretical, because there are no users/calls
to fsl_ifc_find().

Rework the logic, so the global fsl_ifc_ctrl_dev is assigned only after
device state is properly initialized in probe(), except requesting
interrupts.  This makes sure that early call to fsl_ifc_find() will not
use the device driver while it is not fully initialized.

Clear the global pointer on exit and error paths, to fix the possible
use-after-free.

Reported-by: Reid Downing <skibletz@xxxxxxxxx>
Closes: https://lore.kernel.org/all/20261025170833.1316443-1-skibletz@xxxxxxxxx/
Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
Closes: https://sashiko.dev/#/bug/linux-72f58ab4-9f9d-4b00-949c-00929918a3b3
Fixes: 8e0d09b1232d ("memory: fsl_ifc: fix leak of private memory on probe failure")
Fixes: a20cbdeffce2 ("powerpc/fsl: Add support for Integrated Flash Controller")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxxxxx>
---
 drivers/memory/fsl_ifc.c | 73 +++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
index 6dcc317d95d4..ab188f151c94 100644
--- a/drivers/memory/fsl_ifc.c
+++ b/drivers/memory/fsl_ifc.c
@@ -92,6 +92,7 @@ static void fsl_ifc_ctrl_remove(struct platform_device *dev)
 	if (ctrl->nand_irq)
 		free_irq(ctrl->nand_irq, ctrl);
 	free_irq(ctrl->irq, ctrl);
+	fsl_ifc_ctrl_dev = NULL;
 
 	irq_dispose_mapping(ctrl->nand_irq);
 	irq_dispose_mapping(ctrl->irq);
@@ -205,82 +206,82 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
 	int ret = 0;
 	int version, banks;
 	void __iomem *addr;
+	struct fsl_ifc_ctrl *ctrl;
 
 	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);
+	fsl_ifc_ctrl_dev = ctrl;
+
+	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);
-		goto err_unmap_nandirq;
+			ctrl->irq);
+		goto err_clear_global;
 	}
 
-	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;
 		}
 	}
@@ -293,15 +294,17 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
 	return 0;
 
 err_free_nandirq:
-	if (fsl_ifc_ctrl_dev->nand_irq)
-		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_clear_global:
+	fsl_ifc_ctrl_dev = NULL;
 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;
 }
 
-- 
2.53.0