diff mbox series

media: cedrus: fix memory leak in cedrus_init_ctrls()

Message ID 20260624085920.578446-1-dawei.feng@seu.edu.cn (mailing list archive)
State New
Headers
Series media: cedrus: fix memory leak in cedrus_init_ctrls() |

Commit Message

Dawei Feng June 24, 2026, 8:59 a.m. UTC 
In cedrus_init_ctrls(), the V4L2 control handler is initialized before
allocating memory for ctx->ctrls. If this allocation fails, the function
returns -ENOMEM without freeing the previously allocated handler
resources, leading to a memory leak.

Fix this by calling v4l2_ctrl_handler_free() on the ctx->ctrls allocation
failure path.

The bug was first flagged by an experimental analysis tool we are
developing for kernel memory-management bugs while analyzing
v6.13-rc1. The tool is still under development and is not yet publicly
available. Manual inspection confirms that the bug is still
present in v7.1.1.

An x86_64 allyesconfig build showed no new warnings. As we do not have an
Allwinner SoC or board with a Cedrus VPU available to test with, no
runtime testing was able to be performed.

Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
Cc: stable@vger.kernel.org
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

sashiko-bot@kernel.org June 24, 2026, 9:10 a.m. UTC | #1 
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;
> +	}
Dan Carpenter June 24, 2026, 11:50 a.m. UTC | #2 
On Wed, Jun 24, 2026 at 04:59:20PM +0800, Dawei Feng wrote:
> In cedrus_init_ctrls(), the V4L2 control handler is initialized before
> allocating memory for ctx->ctrls. If this allocation fails, the function
> returns -ENOMEM without freeing the previously allocated handler
> resources, leading to a memory leak.
> 
> Fix this by calling v4l2_ctrl_handler_free() on the ctx->ctrls allocation
> failure path.
> 
> The bug was first flagged by an experimental analysis tool we are
> developing for kernel memory-management bugs while analyzing
> v6.13-rc1. The tool is still under development and is not yet publicly
> available. Manual inspection confirms that the bug is still
> present in v7.1.1.
> 
> An x86_64 allyesconfig build showed no new warnings. As we do not have an
> Allwinner SoC or board with a Cedrus VPU available to test with, no
> runtime testing was able to be performed.
> 
> Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> ---

Looks good.

Reviewed-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter
Jernej Škrabec June 27, 2026, 6:38 a.m. UTC | #3 
Dne sreda, 24. junij 2026 ob 10:59:20 Srednjeevropski poletni čas je Dawei Feng napisal(a):
> In cedrus_init_ctrls(), the V4L2 control handler is initialized before
> allocating memory for ctx->ctrls. If this allocation fails, the function
> returns -ENOMEM without freeing the previously allocated handler
> resources, leading to a memory leak.
> 
> Fix this by calling v4l2_ctrl_handler_free() on the ctx->ctrls allocation
> failure path.
> 
> The bug was first flagged by an experimental analysis tool we are
> developing for kernel memory-management bugs while analyzing
> v6.13-rc1. The tool is still under development and is not yet publicly
> available. Manual inspection confirms that the bug is still
> present in v7.1.1.
> 
> An x86_64 allyesconfig build showed no new warnings. As we do not have an
> Allwinner SoC or board with a Cedrus VPU available to test with, no
> runtime testing was able to be performed.
> 
> Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej
diff mbox series

Patch

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index bbd186b8035b..96acd52e380c 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;
 
 	ctx->ctrls = kzalloc(ctrl_size, GFP_KERNEL);
-	if (!ctx->ctrls)
+	if (!ctx->ctrls) {
+		v4l2_ctrl_handler_free(hdl);
 		return -ENOMEM;
+	}
 
 	j = 0;
 	for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {