Re: [PATCH v3 1/4] media: qcom: camss: populate child platform devices

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing use-after-free vulnerability due to mixing `devm_kzalloc` with asynchronous V4L2 video device lifecycle.
--

commit 9fa1d355926fe655802b3d013baabb22bd939e3e
Author: Atanas Filipov <atanas.filipov@oss.qualcomm.com>

media: qcom: camss: populate child platform devices

Call of_platform_populate() at the end of camss_probe() so that child
nodes of the CAMSS DT node are registered as platform devices and
probed via their own platform_driver.

Call of_platform_depopulate() at the start of camss_remove() to tear
down child devices before the parent unwinds its own resources.

Link: https://patchwork.linuxtv.org/project/linux-media/patch/20260326-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v11-4-5b93415be6dd@linaro.org/

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

[ ... ]

> @@ -5497,6 +5500,7 @@ static void camss_remove(struct platform_device *pdev)
>  {
>  	struct camss *camss = platform_get_drvdata(pdev);
>  
> +	of_platform_depopulate(&pdev->dev);
>  	v4l2_async_nf_unregister(&camss->notifier);
>  	v4l2_async_nf_cleanup(&camss->notifier);
>  	camss_unregister_entities(camss);

[Severity: High]
This is a pre-existing issue, but does camss_remove() lead to a use-after-free
if a userspace process holds the V4L2 video node open during unbind?

In camss_probe(), the camss structure and its sub-structures are allocated
using devm_kzalloc() and devm_kcalloc():

camss_probe() {
    ...
    camss = devm_kzalloc(dev, sizeof(*camss), GFP_KERNEL);
    ...
    camss->vfe = devm_kcalloc(dev, camss->res->vfe_num,
                              sizeof(*camss->vfe), GFP_KERNEL);
    ...
}

If the device is unbound while the video device is kept open by userspace,
camss_remove() checks atomic_read(&camss->ref_count). If the count is greater
than zero, it skips camss_delete() but still returns. The driver core then
automatically frees the devres-managed memory for the camss structure.

Later, when userspace closes the file descriptor, the V4L2 core invokes the
deferred msm_video_release() callback:

drivers/media/platform/qcom/camss/camss-video.c:msm_video_release() {
    struct camss_video *video = video_get_drvdata(vdev);

    media_entity_cleanup(&vdev->entity);

    mutex_destroy(&video->q_lock);
    mutex_destroy(&video->lock);

    if (atomic_dec_and_test(&video->camss->ref_count))
        camss_delete(video->camss);
}

Will this asynchronous callback access the already-freed video and
video->camss structures when trying to destroy the mutexes and decrement
the reference count?

Is it safe to use devm allocators for structures embedding V4L2 video devices,
or should their lifetimes be strictly tied to the V4L2 release callback?

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