Re: [PATCH 1/2] EDAC: fsl_ddr: restore MC error registers on probe failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Calling `ddr_out32()` in the `err` cleanup path unconditionally dereferences unmapped I/O memory pointers if probe fails early, leading to a kernel crash.
- [High] Pre-initializing `orig_ddr_err_disable` and `orig_ddr_err_sbe` to 0 causes the error path to blindly overwrite hardware registers with 0 instead of their true original states on intermediate probe failures.

Pre-existing issues:
- [High] The EDAC MC object (`mci`) is freed before the shared interrupt handler is unregistered during module removal, creating a use-after-free vulnerability.
--

--- Patch [1]: [PATCH 1/2] EDAC: fsl_ddr: restore MC error registers on probe failure ---
commit 987a0eb5943a39fdf0201244bbdd7e9cdf6c562e
Author: Rosen Penev <rosenp@gmail.com>

EDAC: fsl_ddr: restore MC error registers on probe failure

fsl_mc_err_probe() clears FSL_MC_ERR_DISABLE and, in EDAC_OPSTATE_INT
mode, sets FSL_MC_ERR_INT_EN to enable error reporting. If a later
initialization step fails and jumps to the err/err2 labels, the cleanup
only frees driver memory and does not restore these hardware registers.

> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
> index e4eaec0aa81d5..7fb789e97074b 100644
> --- a/drivers/edac/fsl_ddr_edac.c
> +++ b/drivers/edac/fsl_ddr_edac.c
> @@ -519,6 +519,8 @@ int fsl_mc_err_probe(struct platform_device *op)
>  
>  	pdata = mci->pvt_info;
>  	pdata->name = "fsl_mc_err";
> +	pdata->orig_ddr_err_disable = 0;
> +	pdata->orig_ddr_err_sbe = 0;

[Severity: High]
Does pre-initializing these to 0 cause the error path to overwrite
hardware registers incorrectly?

If probe fails after I/O mapping but before the original hardware states are
read (for example, if the ECC verification check fails), the code jumps to the
err: block. 

Because orig_ddr_err_disable and orig_ddr_err_sbe are pre-initialized to 0,
the driver will write 0 back to the hardware registers rather than restoring
the bootloader's true configuration, which clears all disable bits and
forcibly enables all memory controller errors.

>  	mci->pdev = &op->dev;
>  	pdata->edac_idx = edac_mc_idx++;
>  	dev_set_drvdata(mci->pdev, mci);

[ ... ]

> @@ -646,7 +648,11 @@ int fsl_mc_err_probe(struct platform_device *op)
>  
>  err2:
>  	edac_mc_del_mc(&op->dev);
> +	ddr_out32(pdata, FSL_MC_ERR_INT_EN, 0);
>  err:
> +	ddr_out32(pdata, FSL_MC_ERR_DISABLE,
> +		  pdata->orig_ddr_err_disable);
> +	ddr_out32(pdata, FSL_MC_ERR_SBE, pdata->orig_ddr_err_sbe);

[Severity: High]
Can this unconditionally dereference unmapped I/O memory pointers if the
probe fails early?

If early initialization steps such as of_address_to_resource(),
devm_request_mem_region(), or devm_ioremap() fail in fsl_mc_err_probe(), the
code jumps to the err: label. At this point, pdata->mc_vbase is still NULL.

The cleanup path added here unconditionally calls ddr_out32(), which executes
ddr_reg_addr() to calculate the register address. This would result in an
invalid low memory address (NULL + off) being passed to iowrite32(),
triggering a fatal Data Abort.

>  	devres_release_group(&op->dev, fsl_mc_err_probe);
>  	edac_mc_free(mci);
>  	return res;


[Severity: High]
This is a pre-existing issue, but does fsl_mc_err_remove() have a
use-after-free vulnerability during module removal?

In fsl_mc_err_probe(), a shared interrupt is requested via devm_request_irq()
using mci as the dev_id. However, during module removal, edac_mc_free(mci) is
called synchronously before devres cleans up the shared IRQ:

drivers/edac/fsl_ddr_edac.c:fsl_mc_err_remove() {
    ...
	edac_mc_del_mc(&op->dev);
	edac_mc_free(mci);
}

Because the IRQ remains registered until devres cleanup runs after
fsl_mc_err_remove() returns, an interrupt on the shared IRQ line during this
window would cause fsl_mc_isr() to execute and dereference the freed mci
pointer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719194009.117532-1-rosenp@gmail.com?part=1