Re: [PATCH v3 2/2] mfd: mxs-lradc: Pass parent IRQs to child drivers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] mfd: mxs-lradc: use-after-free in release_resource() on shared
  mem resource
--

commit 976bbc1082d0f14834e0c7850fdccd2c40f13cb4
Author: Fabio Estevam <festevam@gmail.com>

mfd: mxs-lradc: Pass parent IRQs to child drivers

This commit moves IRQ resolution from the mxs-lradc child drivers into
the MFD parent and shares a single relative memory resource across both
MFD cells via devm_mfd_add_devices().

> diff --git a/drivers/mfd/mxs-lradc.c b/drivers/mfd/mxs-lradc.c
> index 30a66e2681699..d16dfea892076 100644
> --- a/drivers/mfd/mxs-lradc.c
> +++ b/drivers/mfd/mxs-lradc.c
[ ... ]
> +static const struct resource mxs_lradc_resources[] = {
> +	DEFINE_RES_MEM(0, SZ_8K),
> };
> 
> -static struct mfd_cell mx23_cells[] = {
> +static const struct mfd_cell mxs_lradc_cells[] = {
> 	{
> 		.name = "mxs-lradc-adc",
> -		.resources = mx23_adc_resources,
> -		.num_resources = ARRAY_SIZE(mx23_adc_resources),
> +		.resources = mxs_lradc_resources,
> +		.num_resources = ARRAY_SIZE(mxs_lradc_resources),
> 	},
> 	{
> 		.name = "mxs-lradc-ts",
> -		.resources = mx23_touchscreen_resources,
> -		.num_resources = ARRAY_SIZE(mx23_touchscreen_resources),
> +		.resources = mxs_lradc_resources,
> +		.num_resources = ARRAY_SIZE(mxs_lradc_resources),
> 	},
> };
[ ... ]
> @@ -187,38 +215,15 @@ static int mxs_lradc_probe(struct platform_device *pdev)
> 		goto err_clk;
> 	}
> 
> -	switch (lradc->soc) {
> -	case IMX23_LRADC:
> -		mx23_adc_resources[RES_MEM] = *res;
> -		mx23_touchscreen_resources[RES_MEM] = *res;
> -		cells = mx23_cells;
> -		break;
> -	case IMX28_LRADC:
> -		mx28_adc_resources[RES_MEM] = *res;
> -		mx28_touchscreen_resources[RES_MEM] = *res;
> -		cells = mx28_cells;
> -		break;
> -	default:
> -		dev_err(dev, "Unsupported SoC\n");
> -		ret = -ENODEV;
> +	ret = mxs_lradc_get_irqs(pdev, lradc, data);
> +	if (ret)
> 		goto err_clk;
> -	}
> -
> -	ret = devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
> -				   &cells[ADC_CELL], 1, NULL, 0, NULL);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Failed to add the ADC subdevice\n");
> -		goto err_clk;
> -	}
> -
> -	if (!lradc->touchscreen_wire)
> -		return 0;
> 
> -	ret = devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
> -				   &cells[TSC_CELL], 1, NULL, 0, NULL);
> +	num_cells = lradc->touchscreen_wire ? ARRAY_SIZE(mxs_lradc_cells) : 1;
> +	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> +				   mxs_lradc_cells, num_cells, res, 0, NULL);

[Severity: High]
Does passing the same relative IORESOURCE_MEM range in mxs_lradc_cells[]
with mem_base = res cause a use-after-free in __release_resource() when
the driver is unbound with a touchscreen enabled?

When num_cells is 2, mfd_add_device() gives both mxs-lradc-adc and
mxs-lradc-ts an IORESOURCE_MEM resource with parent = res and identical
bounds [res->start, res->start + SZ_8K - 1].  In platform_device_add(),
insert_resource() inserts the ADC resource (r_adc) as a child of res, and
then __insert_resource() places the identical touchscreen resource (r_ts)
between res and r_adc, setting r_adc->parent = r_ts.

On driver unbind, mfd_remove_devices() unregisters the children in
reverse order:

1. platform_device_unregister(pdev_ts) calls release_resource(r_ts) ->
   __release_resource(r_ts, true), which unlinks r_ts from res->child
   without reparenting r_ts->child (r_adc), and platform_device_release()
   frees r_ts while r_adc->parent still points to it.

2. platform_device_unregister(pdev_adc) then calls
   release_resource(r_adc) -> __release_resource(r_adc, true), which
   walks r_adc->parent->child and writes to the freed r_ts.

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