Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The newly added `check_mul_overflow` check is completely ineffective because it stores the result in a `u64` variable. Since both operands (`pitch` and `sizes->surface_height`) are `u32`, their product can never overflow a 64-bit integer, meaning the check always passes. The `u64` size is then implicitly truncated to 32 bits when passed to `armada_gem_alloc_private_object()`, which expects a `size_t` argument. This completely bypasses the intended overflow protection. -- commit 9304c78ca806a472129099acb5c200bab2e752a0 Author: Thomas Zimmermann <[email protected]> drm/armada: fbdev: Calculate buffer geometry with format helpers This commit replaces the geometry and size calculation in armada's fbdev emulation with DRM format helpers. It uses a 4CC lookup, pitch calculation, and allocates the GEM buffer object for the framebuffer memory aligned to PAGE_SIZE. > diff --git a/drivers/gpu/drm/armada/armada_fbdev.c > b/drivers/gpu/drm/armada/armada_fbdev.c > index 8bbae94804f8b..f95658091acf2 100644 > --- a/drivers/gpu/drm/armada/armada_fbdev.c > +++ b/drivers/gpu/drm/armada/armada_fbdev.c > @@ -45,20 +45,30 @@ int armada_fbdev_driver_fbdev_probe(struct drm_fb_helper > *fbh, > { [ ... ] > + if (check_mul_overflow(pitch, sizes->surface_height, &size)) > + return -EINVAL; > + size = ALIGN(size, PAGE_SIZE); > + if (size < PAGE_SIZE) > + return -EINVAL; > > - size = mode.pitches[0] * mode.height; > obj = armada_gem_alloc_private_object(dev, size); [Severity: High] This isn't a bug introduced by this patch, but does this check_mul_overflow() actually prevent overflow here? Since pitch and sizes->surface_height are 32-bit unsigned integers, and size is a u64, the multiplication will never overflow the 64-bit target, so the check will always pass. However, when size is passed to armada_gem_alloc_private_object(), which takes a size_t argument, couldn't this silently truncate to 32 bits since the Armada DRM driver operates on a 32-bit architecture? For example, if pitch * height evaluates to 0xFFFFFFFF, ALIGN() expands it to 0x100000000. The check size < PAGE_SIZE evaluates to false on the u64, but when passed to armada_gem_alloc_private_object(), the size truncates to 0, resulting in an undersized buffer allocation. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
