Re: [PATCH] staging: fbtft: replace number with sizeof()

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Replacing the literal buffer size of 8 with `sizeof(u64)` for a `u8` array obfuscates the code and fails to achieve its stated goal of preventing truncation.

Pre-existing issues:
- [High] Using stack-allocated variables for SPI transfer buffers violates DMA rules and can cause memory corruption or kernel panics.
--

commit fbd50c90250b51626379149887d48d7f5eeb3c1e
Author: Manuel Ebner <manuelebnerli@mailbox.org>

staging: fbtft: replace number with sizeof()

This commit attempts to replace a literal integer with sizeof() to prevent
future truncation bugs in read_devicecode().

> diff --git a/drivers/staging/fbtft/fb_ili9320.c b/drivers/staging/fbtft/fb_ili9320.c
> index 050fc2367c122..41a3ae2f39571 100644
> --- a/drivers/staging/fbtft/fb_ili9320.c
> +++ b/drivers/staging/fbtft/fb_ili9320.c
> @@ -21,7 +21,7 @@
>  
>  static unsigned int read_devicecode(struct fbtft_par *par)
>  {
> -	u8 rxbuf[8] = {0, };
> +	u8 rxbuf[sizeof(u64)] = {0, };

[Severity: High]
This is a pre-existing issue, but rxbuf is allocated on the stack and passed
down to the SPI subsystem for DMA via par->fbtftops.read(). This leads to
fbtft_read_spi() which also allocates txbuf on the stack:

drivers/staging/fbtft/fbtft-io.c:fbtft_read_spi() {
    ...
    u8 txbuf[32] = { 0, };
    ...
    ret = spi_sync(par->spi, &m);
    ...
}

Calling spi_sync() with stack-backed SPI transfers violates DMA mapping rules
and can cause memory corruption or kernel panics, especially with
CONFIG_VMAP_STACK enabled.

[Severity: Medium]
Also, does sizing a u8 array using sizeof(u64) obfuscate the code? This seems
semantically incorrect and fails to achieve the stated goal of preventing
truncation.

>  
>  	write_reg(par, 0x0000);
>  	par->fbtftops.read(par, rxbuf, 4);

[Severity: Medium]
Does this read operation still use a hardcoded length of 4 instead of deriving
it dynamically from the buffer size (e.g., via ARRAY_SIZE())? Since the length
is hardcoded, it appears no truncation protection was actually added.

>  	return (rxbuf[2] << 8) | rxbuf[3];
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914090858.9939-2-manuelebnerli@mailbox.org?part=1