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

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

[Reid Downing]

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.

 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;
 		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;
 }
 
-- 
2.53.0

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

[Krzysztof Kozlowski]

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

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

[Krzysztof Kozlowski]

On 30/10/2026 15:10, Krzysztof Kozlowski wrote:
> 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.
> 

If v4, then please credit also Sashiko:
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-47538f77-fb50-4a06-b49b-92dbee8b538b

Best regards,
Krzysztof

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

[Reid Downing]

Hi Krzysztof,

I completely understand. I wasn't able to test the patch, so please feel free to disregard my submission and proceed with your fix.

Thank you for taking the time to review it.

Reid Downing

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

[Krzysztof Kozlowski]

On 30/10/2026 15:51, Reid Downing wrote:
> Hi Krzysztof,
> 
> I completely understand. I wasn't able to test the patch, so please feel free to disregard my submission and proceed with your fix.

Well, you can still build it - just setup proper cross compile.

Best regards,
Krzysztof

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

[Markus Elfring]

…
> 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
…
                          ???

Is there a need to refine the change description another bit?


…
> ---
> Changes in v3:
…
> Not compile-tested (no PowerPC build setup); the change is four
> assignments of NULL.

It seems that I am counting differently.


…
> +++ b/drivers/memory/fsl_ifc.c
…
> @@ -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;
>  		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;
>  }

How do you think about to avoid duplicate source code a bit more?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v7.3#n526
https://elixir.bootlin.com/linux/v7.3/source/drivers/memory/fsl_ifc.c#L194-L304

Regards,
Markus