Re: [PATCH v5 02/38] drm/blend: Get a rotation name from it's bitfield

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm_get_rotation_name() incorrectly treats a rotation bitmask argument as a direct array index, returning wrong strings for valid rotation masks.
--

commit b39dab0752eb4becf3196f5e38118e4db280556f
Author: Louis Chauvet <louis.chauvet@bootlin.com>

drm/blend: Get a rotation name from it's bitfield

This commit extracts the rotation property table and implements
drm_get_rotation_name() to return the name from a rotation value. It
also introduces a macro for formatting multiple rotation masks.

> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 2f0d1ba285be8..eecdaf623c1e2 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
[ ... ]
> +/**
> + * drm_get_rotation_name - Return the name of a rotation
> + * @rotation: The rotation mask (DRM_MODE_ROTATE_* | DRM_MODE_REFLECT_*)
> + *
> + * Returns: the name of the rotation type (unknown) if rotation is not
> + * a known rotation/reflection
> + */
> +const char *drm_get_rotation_name(unsigned int rotation)
> +{
> +	if (rotation < ARRAY_SIZE(rotation_props))
> +		return rotation_props[rotation].name;

[Severity: Medium]
Since the kernel-doc states the rotation parameter is a mask, won't this 
return incorrect strings?

DRM_MODE_ROTATE_90 is 2, so this would index rotation_props[2].name and 
return "rotate-180" instead of "rotate-90". For higher masks like 
DRM_MODE_REFLECT_X which evaluates to 16, this fails the bounds check 
and returns "(unknown)".

Should this extract the bit index using __builtin_ffs before indexing the 
array?

> +
> +	return "(unknown)";
> +}
> +EXPORT_SYMBOL(drm_get_rotation_name);
[ ... ]
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index bebbb77a8f214..33329170aafe8 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
[ ... ]
> +#define DRM_ROTATION_FMT "%s%s%s%s%s%s%s%s%s%s%s%s"
> +#define DRM_ROTATION_FMT_ARGS(rot) \
> +	(rot) & DRM_MODE_ROTATE_0  ? drm_get_rotation_name(0) : "", \
> +	(rot) & DRM_MODE_ROTATE_0  ? " " : "", \
> +	(rot) & DRM_MODE_ROTATE_90 ? drm_get_rotation_name(1) : "", \

[Severity: Medium]
Does passing array indices like 0, 1, 2 directly to drm_get_rotation_name() 
contradict the documented API contract?

The function's documentation states it expects rotation masks. Passing 
literal indices works around the indexing bug in the function implementation 
but leaves the exported API broken for other callers passing standard masks.

> +	(rot) & DRM_MODE_ROTATE_90 ? " " : "", \
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260627-vkms-all-config-v5-0-854aa0840926@bootlin.com?part=2