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

Pre-existing issues:
- [High] Integer overflow in `vmem_size` calculation leads to a small 
allocation, causing subsequent out-of-bounds reads/writes.
--

--- Patch [1]: [PATCH v2] staging: fbtft: prefer snprintf over sprintf in 
fbtft-core.c ---
commit c1e6190003fe313b23e0964b72df871f88f45da7
Author: Tomasz Unger <[email protected]>

staging: fbtft: prefer snprintf over sprintf in fbtft-core.c

This commit replaces sprintf with snprintf for formatting stack buffers 
text1 and text2 in fbtft-core.c. This prevents potential buffer overflows 
even though the arguments theoretically fit within the 50-byte limits.

> diff --git a/drivers/staging/fbtft/fbtft-core.c 
> b/drivers/staging/fbtft/fbtft-core.c
> index ca0c38221c16b..9ea0442a337e5 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -784,10 +784,10 @@ int fbtft_register_framebuffer(struct fb_info *fb_info)
>       fbtft_sysfs_init(par);
>  
>       if (par->txbuf.buf && par->txbuf.len >= 1024)
> -             sprintf(text1, ", %zu KiB buffer memory", par->txbuf.len >> 10);
> +             snprintf(text1, sizeof(text1), ", %zu KiB buffer memory", 
> par->txbuf.len >> 10);

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the memory
safety improvements in this file, I noticed a potential integer overflow in
fbtft_framebuffer_alloc() prior to memory allocation:

drivers/staging/fbtft/fbtft-core.c:fbtft_framebuffer_alloc() {
    ...
    vmem_size = display->width * display->height * bpp / 8;
    vmem = vzalloc(vmem_size);
    if (!vmem)
        goto release_framebuf;
    ...
}

Could this calculation wrap around 32-bit arithmetic if sufficiently large
dimensions (e.g., width=65544, height=4096, bpp=16) are provided from the
device tree?

If display->width * display->height * bpp overflows, vmem_size could end up
as a small positive integer, leading to a small buffer allocation by vzalloc().

Later in fbtft_update_display(), the line length is calculated as
width * bpp / 8, which easily fits in 32 bits without overflowing. Does this
mean operations might attempt to read or write lengths larger than the
allocated vmem buffer, causing an out-of-bounds access?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to