Re: [PATCH v6 04/18] media: iris: Filter UBWC raw formats based on hardware capabilities

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing locking in several V4L2 ioctl handlers leads to data races on shared instance state like `inst->crop` and `inst->fmt_dst`.
--

--- Patch [4]: [PATCH v6 04/18] media: iris: Filter UBWC raw formats based on hardware capabilities ---
commit aa7fe2abc39f47e09e6b8087a439350fc380a98c
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>

media: iris: Filter UBWC raw formats based on hardware capabilities

The raw formats supported by Iris were previously advertised
unconditionally, assuming UBWC support on all platforms. However, some
platforms do not support UBWC which results in incorrect format
capability exposure.

Use the UBWC configuration provided by the platform to dynamically
filter raw formats at runtime. If UBWC is not supported, UBWC-based
formats are omitted from the advertised capability list, while linear
formats remain available.

> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index 4c8bc7aac1359..9fee5f28097d9 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
[ ... ]
> @@ -80,8 +87,13 @@ static bool check_format(struct iris_inst *inst, u32 pixfmt, u32 type)
>  		size = inst->core->iris_platform_data->inst_iris_fmts_size;
>  		break;
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> -		fmt = iris_vdec_formats_cap;
> -		size = ARRAY_SIZE(iris_vdec_formats_cap);
> +		if (ubwc->ubwc_enc_version) {
> +			fmt = iris_vdec_formats_cap;
> +			size = ARRAY_SIZE(iris_vdec_formats_cap);
> +		} else {
> +			fmt = iris_vdec_formats_noubwc_cap;
> +			size = ARRAY_SIZE(iris_vdec_formats_noubwc_cap);
> +		}
>  		break;
>  	default:
>  		return false;

[Severity: High]
This is a pre-existing issue, but since check_format() accesses shared
instance state like inst->fw_caps, is it safe to call it without inst->lock?

For example, when called from iris_enum_framesizes() in iris_vidc.c:

drivers/media/platform/qcom/iris/iris_vidc.c:iris_enum_framesizes() {
	...
	if (inst->domain == DECODER)
		ret = iris_vdec_validate_format(inst, fsize->pixel_format);
	...
}

This path doesn't acquire inst->lock before calling into check_format().

Similarly, several other V4L2 ioctl handlers appear to be missing
inst->lock, allowing concurrent ioctls to race. For instance:

drivers/media/platform/qcom/iris/iris_vidc.c:iris_s_selection() {
	...
	else if (inst->domain == ENCODER)
		return iris_venc_s_selection(inst, s);
	...
}

drivers/media/platform/qcom/iris/iris_vidc.c:iris_s_parm() {
	...
	if (inst->domain == ENCODER)
		return iris_venc_s_param(inst, a);
	...
}

Without the lock, concurrent userspace calls can race to modify shared state
like inst->crop, inst->fmt_dst->fmt.pix_mp.width, frame_rate, and
operating_rate, potentially causing data corruption or torn internal state.

Since video_device->lock is NULL, the driver is relying on manual
serialization. Should these ioctls acquire inst->lock to protect against
unprivileged userspace races?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-iris-ar50lt-v6-0-374f0a46c23b@oss.qualcomm.com?part=4