Re: [PATCH] media: atomisp: Fix resource leak in atomisp_pci_probe()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The v4l2 async notifier is never unregistered or cleaned up, leading to a memory leak of async subdevice connection objects and dangling notifiers resulting in a Use-After-Free.
--

commit b7444e6b542badbafff9a7d15d5037c35782ba7c
Author: Dawei Feng <dawei.feng@seu.edu.cn>

media: atomisp: Fix resource leak in atomisp_pci_probe()

This commit fixes a resource leak by unregistering subdev entities if
atomisp_register_entities() fails during atomisp_pci_probe().

> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index 900a67552d6a0..d4e4e845f66e6 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -1401,6 +1401,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
>  	err = atomisp_register_entities(isp);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "atomisp_register_entities failed (%d)\n", err);
> +		atomisp_subdev_unregister_entities(&isp->asd);
>  		goto error_uninitialize_modules;
>  	}

[Severity: High]
This is a pre-existing issue, but while looking at the error paths being
fixed here, is there also a memory leak and use-after-free risk related to
the async notifier?

In atomisp_pci_probe(), the notifier is registered via
v4l2_async_nf_register(), adding it to the global notifier_list. However,
the error paths and the driver teardown function atomisp_pci_remove()
appear to entirely omit v4l2_async_nf_unregister() and
v4l2_async_nf_cleanup():

drivers/staging/media/atomisp/pci/atomisp_v4l2.c:atomisp_pci_probe() {
    ...
	err = v4l2_async_nf_register(&isp->notifier);
	if (err) {
		dev_err(isp->dev, "failed to register async notifier : %d\n", err);
		goto error_unload_firmware;
	}

	return 0;

error_unload_firmware:
	atomisp_pm_uninit(isp);
	ia_css_unload_firmware();
    ...
}

Does this code permanently leak the memory allocated for async subdevices via
v4l2_async_nf_add_fwnode_remote() because cleanup is never called?

Additionally, since the isp struct is allocated via devm_kzalloc(), could
omitting the unregister call leave a dangling pointer in the global
notifier_list when the driver is unloaded or when the probe fails, potentially
causing a use-after-free on subsequent list iterations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608082706.3287831-1-dawei.feng@seu.edu.cn?part=1