Re: [PATCH v4 1/5] media: qcom: camss: Populate CAMSS child devices via DT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The return value of `devm_of_platform_populate()` is silently ignored, potentially leaving the subsystem in a broken state without propagating the error.
- [High] Incorrect devres ordering causes child devices to be populated prematurely and depopulated too late, leading to hardware faults and Use-After-Free during teardown.
--

commit 87e3279291715684d9b1d07a1326244dc96aa857
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

media: qcom: camss: Populate CAMSS child devices via DT

Use devm_of_platform_populate() so that child nodes declared under the
CAMSS device tree node (e.g. OPE) are automatically instantiated as
platform devices.

> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 2123f6388e3d7..95e655a8b6aa0 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c

[ ... ]

> @@ -5362,6 +5363,8 @@ static int camss_probe(struct platform_device *pdev)
>  	if (!camss)
>  		return -ENOMEM;
>  
> +	devm_of_platform_populate(dev);

[Severity: High]
Does this code silently ignore initialization errors?

If devm_of_platform_populate() fails, the driver proceeds without propagating
the error. This could leave the required child devices unpopulated and the
camera subsystem in a partially initialized and functionally broken state.
Should the return value be checked here?

[Severity: High]
Could calling devm_of_platform_populate() this early in probe cause hardware
faults or use-after-free during driver teardown?

Because devm_of_platform_populate() is called at the very beginning of
camss_probe(), its devres cleanup (of_platform_depopulate()) is pushed to the
bottom of the LIFO devres stack and runs last.

During driver removal, camss_remove() manually disables PM runtime and
destroys power domains via camss_genpd_cleanup() before the devres cleanup
unregisters the child devices.

This leaves a teardown window where the child video devices remain fully
registered and active while the parent's resources, memory, and power domains
are completely destroyed. If userspace accesses the child video device during
this window, will it interact with unpowered or freed parent hardware?

> +
>  	camss->res = of_device_get_match_data(dev);
>  
>  	atomic_set(&camss->ref_count, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706071113.383215-1-atanas.filipov@oss.qualcomm.com?part=1