Re: [PATCH v3] memory: fsl_ifc: Clear global controller pointer on probe failure and remove

[Krzysztof Kozlowski <krzk@kernel.org>]

On 25/10/2026 18:08, Reid Downing wrote:
> fsl_ifc_ctrl_dev is assigned early in fsl_ifc_ctrl_probe() to a
> devm-allocated structure. If probe fails afterwards, or when the device
> is removed, that memory is freed and the registers are unmapped, but the
> global still points at it. fsl_ifc_find() and fsl_ifc_nand_probe() only
> check it for NULL, so a later call can dereference freed memory or an
> unmapped register region.
> 
> Clear the pointer on the probe error paths and in remove, once the IRQs
> have been released. The global cannot be published later in probe
> instead, because ifc_in32()/ifc_out32() and the NAND child probed from
> of_platform_default_populate() depend on it.
> 
> Signed-off-by: Reid Downing <skibletz@gmail.com>
> ---
> Changes in v3:
>  - Use my real name in Author and Signed-off-by (earlier versions had a
>    handle appended).
>  - Generate the patch with git against drivers/memory/fsl_ifc.c; the
>    earlier versions used a path that could not be applied. Sorry for the
>    noise, and for sending two unversioned copies. Those were v1 and v2,
>    and this v3 supersedes them.
>  - Change of approach: v1 and v2 delayed publishing fsl_ifc_ctrl_dev until
>    the end of probe, but ifc_in32()/ifc_out32() and fsl_ifc_nand_probe()
>    use the global during probe, so that would break the driver. v3 keeps
>    the early assignment and instead clears the pointer on probe failure
>    and in remove.
>  - Dropped the nand_irq guard from the earlier versions; it was unrelated
>    to this fix.
> 
> Not compile-tested (no PowerPC build setup); the change is four
> assignments of NULL.

Independently I want through Sashiko reports and tried to fix this with
LLM... and then remembered about your patch. Unfortunately I am not
taking unbuilt code, especially that build testing it is trivial.


> 
>  drivers/memory/fsl_ifc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
> index e89e0c6cc..abacd90d6 100644
> --- a/drivers/memory/fsl_ifc.c
> +++ b/drivers/memory/fsl_ifc.c
> @@ -95,6 +95,8 @@ static void fsl_ifc_ctrl_remove(struct platform_device *dev)
>  	irq_dispose_mapping(ctrl->nand_irq);
>  	irq_dispose_mapping(ctrl->irq);
>  
> +	/* Children and IRQs are gone; stop lookups before unmapping. */
> +	fsl_ifc_ctrl_dev = NULL;
>  	iounmap(ctrl->gregs);
>  
>  	dev_set_drvdata(&dev->dev, NULL);
> @@ -218,6 +220,7 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
>  	fsl_ifc_ctrl_dev->gregs = of_iomap(dev->dev.of_node, 0);
>  	if (!fsl_ifc_ctrl_dev->gregs) {
>  		dev_err(&dev->dev, "failed to get memory region\n");
> +		fsl_ifc_ctrl_dev = NULL;

This is supposed to be goto error path.

>  		return -ENODEV;
>  	}
>  
> @@ -300,6 +303,7 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
>  	irq_dispose_mapping(fsl_ifc_ctrl_dev->irq);
>  err:
>  	iounmap(fsl_ifc_ctrl_dev->gregs);
> +	fsl_ifc_ctrl_dev = NULL;
>  	return ret;

Please let me know if you intend to send v4 which is going to be at
least built or I take what LLM produced me.

Best regards,
Krzysztof