Re: [PATCH v14 2/5] media: qcom: camss: Add Kaanapali compatible camss driver

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unconditional dereference of `camss->genpd` during error handling and driver unbind when the PM domain was not successfully attached.
- [High] devm_ managed lifetime of driver structures conflicts with V4L2 video device file descriptor lifetime, causing a Use-After-Free on sysfs unbind.
--

commit d11af4da6f46342576374344948cc4914a3b8685
Author: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>

media: qcom: camss: Add Kaanapali compatible camss driver
    
Add support for Kaanapali in the camss driver. Add high level resource
information along with the bus bandwidth votes. Module level detailed
resource information will be enumerated in the following patches of the
series.

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

[ ... ]

> @@ -5507,6 +5521,13 @@ static void camss_remove(struct platform_device *pdev)
>  	camss_genpd_cleanup(camss);

[Severity: High]
This isn't a bug introduced by this patch, but does this unconditionally
dereference camss->genpd during error handling and driver unbind when the power
domain was not successfully attached?

In camss_configure_pd(), if camss->genpd_num > 1 but camss->genpd_num <=
vfepd_num, it returns 0 early without attaching top power domains, leaving
camss->genpd as null:

drivers/media/platform/qcom/camss/camss.c:camss_configure_pd() {
    ...
	if (!(camss->genpd_num > vfepd_num))
		return 0;
    ...
}

Later, during cleanup in camss_genpd_cleanup(), the code skips cleanup only if
camss->genpd_num == 1:

drivers/media/platform/qcom/camss/camss.c:camss_genpd_cleanup() {
    ...
	if (camss->genpd_num == 1)
		return;
    ...
	dev_pm_domain_detach(camss->genpd, true);
    ...
}

dev_pm_domain_detach() does not check for a null device pointer and immediately
dereferences dev->pm_domain, which can cause a panic on probe failure or driver
unbind for valid hardware configurations where genpd_num > 1.

[Severity: High]
This is a pre-existing issue, but does the devm_ managed lifetime of driver
structures conflict with the v4l2 video device file descriptor lifetime here?

The driver allocates its core state using devm_kzalloc and devm_kcalloc:

drivers/media/platform/qcom/camss/camss.c:camss_probe() {
    ...
	camss = devm_kzalloc(dev, sizeof(*camss), GFP_KERNEL);
    ...
	camss->vfe = devm_kcalloc(dev, camss->res->vfe_num,
				  sizeof(*camss->vfe), GFP_KERNEL);
    ...
}

When the driver is unbound via sysfs, camss_remove() executes and returns,
causing devres to immediately free all devm_ memory. However, if a user process
holds an open file descriptor to the video node during unbind, the v4l2 core
retains a reference to the video_device.

When the user later closes the file descriptor, v4l2_release() calls
msm_video_release(), which accesses video->camss->ref_count within the already
freed memory:

drivers/media/platform/qcom/camss/camss-video.c:msm_video_release() {
    ...
	if (atomic_dec_and_test(&video->camss->ref_count))
		camss_delete(video->camss);
    ...
}

Can this lead to a use-after-free?

>  }
>  
> +static const struct camss_resources kaanapali_resources = {
> +	.version = CAMSS_KAANAPALI,
> +	.pd_name = "top",
> +	.icc_res = icc_res_kaanapali,
> +	.icc_path_num = ARRAY_SIZE(icc_res_kaanapali),
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-kaanapali-camss-v14-0-e76f26aa6691@oss.qualcomm.com?part=2