Re: [PATCH 6/6] drm: Avoid undefined behavior on u16 multiplication

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 08 Sep 2025, Krzysztof Karas <krzysztof.karas@intel.com> wrote:
> Fields hdiplay and vdisplay are defined as u16. Their
> multiplication causes implicit promotion to signed 32-bit value,
> which may overflow and cause undefined behavior.
>
> The same goes for vpos, which is multiplied by signed integer.
>
> Prevent possible undefined behavior by explicitly casting one of
> the operands to (unsigned int) type.
>
> Fixes: 3ed4351a83ca ("drm: Extract drm_vblank.[hc]")
> Fixes: cc4312127108 ("drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats()")
> Fixes: 80f7c3f77697 ("drm/vram: Add helpers to validate a display mode's memory requirements")
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Simona Vetter <simona.vetter@ffwll.ch>
> Cc: <stable@vger.kernel.org> # v4.13+
> Cc: <stable@vger.kernel.org> # v5.4+
> Cc: <stable@vger.kernel.org> # v5.7+

The multitude of files, Fixes and subsequent Cc: stables with different
target kernels is just an indication this should be three separate
patches.

The cause may be the same, but they're still unrelated fixes.

Moreover, imagine the consequences of one of these fixes ending up being
incorrect, and you have to revert or fix it. It's a logistics PITA.


BR,
Jani.


> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
>  drivers/gpu/drm/drm_mipi_dbi.c        | 2 +-
>  drivers/gpu/drm/drm_vblank.c          | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index b04cde4a60e7..4b144e9603b8 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -967,7 +967,7 @@ drm_vram_helper_mode_valid_internal(struct drm_device *dev,
>  
>  	max_fbpages = (vmm->vram_size / 2) >> PAGE_SHIFT;
>  
> -	fbsize = mode->hdisplay * mode->vdisplay * max_bpp;
> +	fbsize = (unsigned int)mode->hdisplay * mode->vdisplay * max_bpp;
>  	fbpages = DIV_ROUND_UP(fbsize, PAGE_SIZE);
>  
>  	if (fbpages > max_fbpages)
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index e33c78fc8fbd..536741dd7690 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -691,7 +691,7 @@ int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev,
>  		      const struct drm_simple_display_pipe_funcs *funcs,
>  		      const struct drm_display_mode *mode, unsigned int rotation)
>  {
> -	size_t bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16);
> +	size_t bufsize = (unsigned int)mode->vdisplay * mode->hdisplay * sizeof(u16);
>  
>  	dbidev->drm.mode_config.preferred_depth = 16;
>  
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 46f59883183d..8a3a82962494 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -779,7 +779,7 @@ drm_crtc_vblank_helper_get_vblank_timestamp_internal(
>  	 * since start of scanout at first display scanline. delta_ns
>  	 * can be negative if start of scanout hasn't happened yet.
>  	 */
> -	delta_ns = div_s64(1000000LL * (vpos * mode->crtc_htotal + hpos),
> +	delta_ns = div_s64(1000000LL * ((unsigned int)vpos * mode->crtc_htotal + hpos),
>  			   mode->crtc_clock);
>  
>  	/* Subtract time delta from raw timestamp to get final

-- 
Jani Nikula, Intel