Re: [PATCH] video: fbdev: kyro: Validate overlay viewport coordinates

[listdansp]

On 24/07/2026 03:54, sashiko-...@kernel.org wrote:

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

Pre-existing issues:
- [High] Missing validation for ulHeight == 1 causes a division by zero panic 
in SetOverlayViewPort.
--

--- Patch [1]: [PATCH] video: fbdev: kyro: Validate overlay viewport 
coordinates ---
commit eccd0fc9ac01567bf3583eb92ff9c8b899a5e6b3
Author: Danila Chernetsov <listda...@mail.ru>

video: fbdev: kyro: Validate overlay viewport coordinates

This commit adds coordinate validation in kyro_dev_overlay_viewport_set to
prevent 32-bit wraparound before passing the values to SetOverlayViewPort.
It explicitly checks for overflows when adding dimensions and restricts the
adjusted coordinates to fit into 16-bit register fields.

diff --git a/drivers/video/fbdev/kyro/fbdev.c b/drivers/video/fbdev/kyro/fbdev.c
--- a/drivers/video/fbdev/kyro/fbdev.c
+++ b/drivers/video/fbdev/kyro/fbdev.c

[ ... ]

@@ -378,11 +381,30 @@ static int kyro_dev_overlay_viewport_set(u32 x, u32 y, 
u32 ulWidth, u32 ulHeight
            (x < 2 && ulWidth + 2 == 0))
                return -EINVAL;

+ /*

+        * SetOverlayViewPort() adjusts X coordinates by +2 (left) and +1
+        * (right) before packing them into 16-bit register fields.
+        */
+       if (x > U16_MAX - 2 || y > U16_MAX)
+               return -EINVAL;
+
+       right = x + ulWidth;
+       bottom = y + ulHeight;
+
+       if (right < x || bottom < y)
+               return -EINVAL;
+
+       right--;
+       bottom--;
+
+       if (right > U16_MAX - 1 || bottom > U16_MAX)
+               return -EINVAL;
+

[Severity: High]
This is a pre-existing issue, but does this validation block still allow a
divide-by-zero panic if ulHeight is 1?

If a user passes ySize = 1 via the KYRO_IOCTL_OVERLAY_VIEWPORT_SET ioctl,
ulHeight will be 1. The code calculates bottom = y + ulHeight, then
bottom--, which results in bottom == y.

When these values are passed to SetOverlayViewPort(), it calculates ulDest
as the difference between the y coordinates (bottom - top). Since they are
equal, ulDest evaluates to 0:

drivers/video/fbdev/kyro/STG4000OverlayDevice.c:SetOverlayViewPort() {
     ...
     ulDest = srcDest.lDstY2 - srcDest.lDstY1;
     ...
     ulFxScale = (ulDest << 11) / ulSrc;
     ...
     ulSrcBottom = ulSrcBottom - (ulFxOffset / ulFxScale);
     ...
}

Because ulDest is 0, ulFxScale evaluates to 0. The subsequent division by
ulFxScale then causes a kernel panic.

Should ulHeight == 1 be rejected alongside the ulHeight == 0 check earlier
in kyro_dev_overlay_viewport_set()?

Thanks for the review. The reported division-by-zero issue is pre-existing
in the codebase and is completely unrelated to my changes. This patch does
not introduce, touch, or modify this logic.

I request the maintainers to review my patch as is.

Best regards,
Danila Chernetsov