Re: [PATCH] media: cedrus: fix memory leak in cedrus_init_ctrls()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect array size calculation for `ctx->ctrls` allocates insufficient space for the NULL terminator, leading to a KASAN slab-out-of-bounds read and possible wild pointer dereference.
--

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

media: cedrus: fix memory leak in cedrus_init_ctrls()

This commit fixes a memory leak in cedrus_init_ctrls() by freeing the V4L2
control handler resources if the memory allocation for the controls array
fails.

> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index bbd186b8035b2..96acd52e380c8 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -285,8 +285,10 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
>  	ctrl_size = sizeof(ctrl) * CEDRUS_CONTROLS_COUNT + 1;

[Severity: High]
This isn't a bug introduced by this patch, but does this correctly allocate
enough space for the NULL terminator?

It appears to add 1 byte instead of sizeof(ctrl), allocating insufficient
space. The remaining bytes of the terminator pointer could be left
uninitialized.

When checking the array bounds in cedrus_find_control_data():

	for (i = 0; ctx->ctrls[i]; i++)
		if (ctx->ctrls[i]->id == id)

Could this read the improperly sized terminator, leading to a KASAN
slab-out-of-bounds read and a wild pointer dereference?

>  
>  	ctx->ctrls = kzalloc(ctrl_size, GFP_KERNEL);
> -	if (!ctx->ctrls)
> +	if (!ctx->ctrls) {
> +		v4l2_ctrl_handler_free(hdl);
>  		return -ENOMEM;
> +	}

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