Re: [PATCH v6 25/28] media: rockchip: rga: add feature flags

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing return value check for `rga_setup_ctrls` in `rga_open` leads to a Use-After-Destroy of the control handler mutex.
--

commit 0e807f6179fe82fa9a8a593d5cefb491574e313b
Author: Sven Püschel <s.pueschel@pengutronix.de>

media: rockchip: rga: add feature flags

In preparation to the RGA3 addition add feature flags, which can limit
the exposed feature set of the video device, like rotating or selection
support. This is necessary as the RGA3 doesn't initially implement the
full feature set currently exposed by the driver.

> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index 1878b4e26360b..8d60e94da32d8 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -177,17 +177,21 @@ static int rga_setup_ctrls(struct rga_ctx *ctx)
>  
>  	v4l2_ctrl_handler_init(&ctx->ctrl_handler, 4);
>  
> -	v4l2_ctrl_new_std(&ctx->ctrl_handler, &rga_ctrl_ops,
> -			  V4L2_CID_HFLIP, 0, 1, 1, 0);
> +	if (rga->hw->features & RGA_FEATURE_FLIP) {
> +		v4l2_ctrl_new_std(&ctx->ctrl_handler, &rga_ctrl_ops,
> +				  V4L2_CID_HFLIP, 0, 1, 1, 0);
>  
> -	v4l2_ctrl_new_std(&ctx->ctrl_handler, &rga_ctrl_ops,
> -			  V4L2_CID_VFLIP, 0, 1, 1, 0);
> +		v4l2_ctrl_new_std(&ctx->ctrl_handler, &rga_ctrl_ops,
> +				  V4L2_CID_VFLIP, 0, 1, 1, 0);
> +	}
>  
> -	v4l2_ctrl_new_std(&ctx->ctrl_handler, &rga_ctrl_ops,
> -			  V4L2_CID_ROTATE, 0, 270, 90, 0);
> +	if (rga->hw->features & RGA_FEATURE_ROTATE)
> +		v4l2_ctrl_new_std(&ctx->ctrl_handler, &rga_ctrl_ops,
> +				  V4L2_CID_ROTATE, 0, 270, 90, 0);
>  
> -	v4l2_ctrl_new_std(&ctx->ctrl_handler, &rga_ctrl_ops,
> -			  V4L2_CID_BG_COLOR, 0, 0xffffffff, 1, 0);
> +	if (rga->hw->features & RGA_FEATURE_BG_COLOR)
> +		v4l2_ctrl_new_std(&ctx->ctrl_handler, &rga_ctrl_ops,
> +				  V4L2_CID_BG_COLOR, 0, 0xffffffff, 1, 0);
>  
>  	if (ctx->ctrl_handler.error) {
>  		int err = ctx->ctrl_handler.error;

This issue wasn't introduced by this patch, but since rga_setup_ctrls() is
modified here, does the error handling in the caller expose a use-after-free
if rga_setup_ctrls() fails?

In rga_setup_ctrls(), if there is an error, it calls
v4l2_ctrl_handler_free(&ctx->ctrl_handler) and returns an error code.

However, in rga_open(), the return value of rga_setup_ctrls() is ignored:

drivers/media/platform/rockchip/rga/rga.c:rga_open() {
	...
	rga_setup_ctrls(ctx);

	/* Write the default values to the ctx struct */
	v4l2_ctrl_handler_setup(&ctx->ctrl_handler);
	...
}

Since v4l2_ctrl_handler_free() destroys the handler's internal mutex, does
calling v4l2_ctrl_handler_setup() subsequently attempt to lock a destroyed
mutex?

Additionally, does rga_open() returning success to userspace leave the V4L2
context in a corrupted state if rga_setup_ctrls() failed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-spu-rga3-v6-0-e547152eb9c9@pengutronix.de?part=25