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

[Krzysztof Kozlowski <krzk@kernel.org>]

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