Re: [PATCH v2 02/13] media: stm32: dcmipp: make dcmipp_state & cmsr2 read common

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The interrupt handler `dcmipp_irq_callback` is subject to a teardown and initialization lifecycle mismatch that causes a NULL pointer dereference during probe and a Use-After-Free during removal.
--

commit 57156bb8e4502a74fba0baeca5a498fc6151d274
Author: Alain Volmat <alain.volmat@foss.st.com>

media: stm32: dcmipp: make dcmipp_state & cmsr2 read common

This commit moves the dcmipp_state enum into a common header and centralizes
the reading of the CMSR2 interrupt status register. The core interrupt
handler now reads the register and shares the value with each subdevice.

> diff --git a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
> index dd784cfcaac82..b06a4931ae951 100644
> --- a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
> +++ b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-core.c
[ ... ]
> @@ -251,10 +253,15 @@ static irqreturn_t dcmipp_irq_callback(int irq, void *arg)
>  	struct dcmipp_ent_device *ved;
>  	irqreturn_t ret = IRQ_HANDLED;
>  	unsigned int i;
> +	u32 cmsr2;
> +
> +	/* Centralized read of CMSR2 */
> +	cmsr2 = reg_read(dcmipp, DCMIPP_CMSR2);
>  
>  	/* Call irq handler of each entities of pipeline */
>  	for (i = 0; i < dcmipp->pipe_cfg->num_ents; i++) {
>  		ved = dcmipp->entity[i];
> +		ved->cmsr2 = cmsr2;

[Severity: High]
This is a pre-existing issue, but does this create a risk of a NULL pointer
dereference during probe or a use-after-free during removal?

In dcmipp_probe(), devm_request_threaded_irq() is called and unmasks the IRQ
before the dcmipp->entity array is allocated via devm_kcalloc():

	ret = devm_request_threaded_irq(&pdev->dev, irq, dcmipp_irq_callback,
					dcmipp_irq_thread, IRQF_ONESHOT,
					dev_name(&pdev->dev), dcmipp);
	...
	dcmipp->entity = devm_kcalloc(&pdev->dev, dcmipp->pipe_cfg->num_ents,
				      ...);

If an interrupt fires immediately, dcmipp->entity will be NULL, causing a crash
when evaluating dcmipp->entity[i].

Similarly, in dcmipp_remove(), the subdevice entities are released and their
memory freed while the devres-managed IRQ is still active:

	for (i = 0; i < dcmipp->pipe_cfg->num_ents; i++)
		dcmipp->pipe_cfg->ents[i].release(dcmipp->entity[i]);

If a spurious interrupt fires during or after this teardown loop, could this
assignment to ved->cmsr2 corrupt freed memory?

>  		if (ved->handler)
>  			ved->handler_ret = ved->handler(irq, ved);
>  		else if (ved->thread_fn)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-stm32-dcmipp-pixel-pipes-support-v2-0-3c76b5f93157@foss.st.com?part=2