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 Closes: https://lore.kernel.org/all/20261025170833.1316443-1-skibletz@gmail.com/ Reported-by: Sashiko 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@vger.kernel.org Signed-off-by: Krzysztof Kozlowski --- 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