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 <[email protected]> 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()? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
