Re: [PATCH] media: renesas: vsp1: fix CLU/LUT entity leaks upon pool allocation failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] vsp1_hgo_create() and vsp1_hgt_create() trigger a kernel oops if vsp1_entity_init() fails, due to an error pointer dereference in vsp1_entity_destroy().
--

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

media: renesas: vsp1: fix CLU/LUT entity leaks upon pool allocation failure

This commit fixes resource leaks in vsp1_clu_create() and vsp1_lut_create()
by consolidating error handling. If vsp1_dl_body_pool_create() or control
initialization fails, it now uses a common error label to properly invoke
vsp1_entity_destroy().

> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_clu.c b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
> index a6e4bcab5101a..b0b85a00957ac 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_clu.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
[ ... ]
> @@ -248,11 +250,14 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device *vsp1)
>  	if (clu->ctrls.error) {
>  		dev_err(vsp1->dev, "clu: failed to initialize controls\n");
>  		ret = clu->ctrls.error;
> -		vsp1_entity_destroy(&clu->entity);
> -		return ERR_PTR(ret);
> +		goto error;
>  	}
>  
>  	v4l2_ctrl_handler_setup(&clu->ctrls);
>  
>  	return clu;
> +
> +error:
> +	vsp1_entity_destroy(&clu->entity);

[Severity: High]
This is a pre-existing issue, but does calling vsp1_entity_destroy() risk
a kernel panic if the entity state is an error pointer?

In drivers/media/platform/renesas/vsp1/vsp1_entity.c:vsp1_entity_init(),
if state allocation fails, it assigns an ERR_PTR to entity->state:

drivers/media/platform/renesas/vsp1/vsp1_entity.c:vsp1_entity_init() {
    ...
    entity->state = __v4l2_subdev_state_alloc(&entity->subdev,
                                              "vsp1:state->lock", &key);
    if (IS_ERR(entity->state)) {
        media_entity_cleanup(&entity->subdev.entity);
        return PTR_ERR(entity->state);
    }
    ...
}

If vsp1_entity_destroy() is later called, it unconditionally frees the state:

drivers/media/platform/renesas/vsp1/vsp1_entity.c:vsp1_entity_destroy() {
    ...
    __v4l2_subdev_state_free(entity->state);
    ...
}

However, __v4l2_subdev_state_free() only checks for NULL, not IS_ERR():

drivers/media/v4l2-core/v4l2-subdev.c:__v4l2_subdev_state_free() {
    if (!state)
        return;

    mutex_destroy(&state->_lock);
    ...
}

This causes a blind dereference of the error pointer.

While this patch calls vsp1_entity_destroy() after vsp1_entity_init() has
succeeded (making the state pointer valid here), callers like vsp1_hgo_create()
and vsp1_hgt_create() handle vsp1_entity_init() failures by immediately calling
vsp1_entity_destroy().

Could this cause a crash if state allocation fails during probe?

> +	return ERR_PTR(ret);
>  }

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