Re: [PATCH v6 6/9] dt-bindings: media: add i.MX parallel CPI support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unhandled resource leaks (memory and fwnode references) and double-free risk in `__media_async_register_subdev` and its cleanup path.
--

commit 050e10af8571659357b0d6bb808eab4e58871636
Author: Frank Li <Frank.Li@nxp.com>

media: subdev: Add media_async_register_subdev() helper

This commit adds media_async_register_subdev(), a helper to register a V4L2
sub-device with the asynchronous sub-device framework. It parses the firmware
graph, creates media pads for endpoints, and registers asynchronous notifiers.
This centralizes common initialization and link creation to reduce boilerplate.

> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 62a3a452f7884..169059654478f 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -1302,6 +1303,160 @@ int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *m
>  }
>  EXPORT_SYMBOL_GPL(__v4l2_async_register_subdev_sensor);
>  
[ ... ]
> +void media_async_subdev_cleanup(struct v4l2_subdev *sd)
> +{
> +	v4l2_async_unregister_subdev(sd);
> +	v4l2_subdev_cleanup(sd);
> +	media_entity_cleanup(&sd->entity);
> +	v4l2_async_nf_unregister(sd->subdev_notifier);
> +	v4l2_async_nf_cleanup(sd->subdev_notifier);
> +	kfree(sd->entity.pads);
> +}
> +EXPORT_SYMBOL_GPL(media_async_subdev_cleanup);

[Severity: High]
Does this cleanup function leak fwnode references?

v4l2_async_nf_parse_fwnode() parses endpoints directly into pad->vep, taking
an fwnode reference via fwnode_handle_get(). It appears this cleanup function
fails to iterate over the pads to call v4l2_fwnode_endpoint_free(&pad->vep) to
release these references.

Also, if a driver explicitly invokes this cleanup helper to recover after
__media_async_register_subdev() fails, will kfree(sd->entity.pads) cause a
double-free panic?

> +
> +int __media_async_register_subdev(struct v4l2_subdev *sd, struct module *module)
> +{
> +	struct device *dev = sd->dev;
> +	u32 ep_count;
> +	int ret;
> +
[ ... ]
> +	struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad, ep_count);
> +	if (!pads)
> +		return -ENOMEM;
> +
> +	ret = v4l2_async_nf_parse_fwnode(dev, pads, notifier);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = media_entity_pads_init(&sd->entity, ep_count, pads);
> +	if (ret)
> +		goto out_cleanup;
> +
> +	ret = v4l2_async_nf_register(notifier);
> +	if (ret < 0)
> +		goto out_cleanup;
> +
> +	ret = v4l2_subdev_init_finalize(sd);
> +	if (ret)
> +		goto out_unregister;
> +
> +	ret = __v4l2_async_register_subdev(sd, module);
> +	if (ret < 0)
> +		goto out_unregister;
> +
> +	sd->subdev_notifier = no_free_ptr(notifier);
> +	retain_and_null_ptr(pads);
> +
> +	return 0;
> +
> +out_unregister:
> +	v4l2_async_nf_unregister(notifier);
> +
> +out_cleanup:
> +	v4l2_async_nf_cleanup(notifier);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(__media_async_register_subdev);

[Severity: High]
If __v4l2_async_register_subdev() fails, the error path returns without
calling v4l2_subdev_cleanup(sd) to free the active state allocated by
v4l2_subdev_init_finalize(). Can this leak sd->active_state?

Additionally, pads is allocated with __free(kfree) and linked to
sd->entity.pads. On a registration failure, pads is automatically freed when
the function returns, but sd->entity.pads is not cleared.

Could this leave a dangling pointer that causes a double-free if error
recovery is attempted?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=6