Re: [CI v4 01/14] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `u64 supported_fm` bitmask is incorrectly validated using the 32-bit `BIT()` macro, causing the validation to ignore invalid high bits on 32-bit systems.
--

--- Patch [1]: [CI v4 01/14] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX ---
commit c09a55528ad3801e2bc4cab04c676078a00cc4e2
Author: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>

drm/colorop: Add DRM_COLOROP_FIXED_MATRIX

Introduce DRM_COLOROP_FIXED_MATRIX, a new colorop type representing a
hardware that performs a fixed matrix operation. Userspace selects one of
the predefined hardware modes via a new FIXED_MATRIX_TYPE enum property.

> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index 682fcc6515259..483defad699c0 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c

[ ... ]

> @@ -453,6 +462,80 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *col
>  }
>  EXPORT_SYMBOL(drm_plane_colorop_3dlut_init);
>  
> +/**
> + * drm_plane_colorop_fixed_matrix_init - Initialize a DRM_COLOROP_FIXED_MATRIX
> + *
> + * @dev: DRM device
> + * @colorop: The drm_colorop object to initialize
> + * @plane: The associated drm_plane
> + * @funcs: control functions for the new colorop
> + * @supported_fm: A bitfield of supported drm_colorop_fixed_matrix_type enum values,
> + *               created using BIT(fixed_matrix_type) and combined with the OR '|'
> + *               operator.
> + * @flags: bitmask of misc, see DRM_COLOROP_FLAG_* defines.
> + * @return zero on success, -E value on failure
> + */
> +int drm_plane_colorop_fixed_matrix_init(struct drm_device *dev, struct drm_colorop *colorop,
> +					struct drm_plane *plane,
> +					const struct drm_colorop_funcs *funcs,
> +					u64 supported_fm, uint32_t flags)
> +{
> +	struct drm_prop_enum_list enum_list[DRM_COLOROP_FM_COUNT];
> +	int i, len;
> +	struct drm_property *prop;
> +	int ret;
> +
> +	if (!supported_fm) {
> +		drm_err(dev,
> +			"No supported FM type op for new Fixed Matrix colorop on [PLANE:%d:%s]\n",
> +			plane->base.id, plane->name);
> +		return -EINVAL;
> +	}
> +
> +	if ((supported_fm & -BIT(DRM_COLOROP_FM_COUNT)) != 0) {

[Severity: Medium]
Will this check ignore invalid high bits in supported_fm on 32-bit systems?

Since supported_fm is a u64, using the 32-bit BIT() macro on a 32-bit
architecture means -BIT(DRM_COLOROP_FM_COUNT) will result in a 32-bit
unsigned long (e.g., 0xFFFFFFE0UL). When bitwise-ANDed with a u64, this mask
zero-extends to 0x00000000FFFFFFE0ULL, causing any invalid bits set in the
upper 32 bits to be silently ignored instead of returning -EINVAL.

Would it be safer to use BIT_ULL() or GENMASK_ULL() here?

> +		drm_err(dev, "Unknown Fixed Matrix provided on [PLANE:%d:%s]\n",
> +			plane->base.id, plane->name);
> +		return -EINVAL;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713051516.4087420-1-chaitanya.kumar.borah@intel.com?part=1