* [PATCH] staging: fbtft: replace number with sizeof()
@ 2026-09-14 9:08 Manuel Ebner
2026-09-14 9:16 ` sashiko-bot
2026-09-14 10:57 ` Andy Shevchenko
0 siblings, 2 replies; 3+ messages in thread
From: Manuel Ebner @ 2026-09-14 9:08 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: Manuel Ebner, dri-devel, linux-fbdev, linux-staging, linux-kernel
Replace integer number with sizeof with the goal of prevent future truncation bugs.
Signed-off-by: Manuel Ebner <manuelebnerli@mailbox.org>
---
drivers/staging/fbtft/fb_ili9320.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_ili9320.c b/drivers/staging/fbtft/fb_ili9320.c
index 0f98267a7..655778392 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, };
write_reg(par, 0x0000);
par->fbtftops.read(par, rxbuf, 4);
--
2.55.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: fbtft: replace number with sizeof()
2026-09-14 9:08 [PATCH] staging: fbtft: replace number with sizeof() Manuel Ebner
@ 2026-09-14 9:16 ` sashiko-bot
2026-09-14 10:57 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-14 9:16 UTC (permalink / raw)
To: Manuel Ebner; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: fbtft: replace number with sizeof()
2026-09-14 9:08 [PATCH] staging: fbtft: replace number with sizeof() Manuel Ebner
2026-09-14 9:16 ` sashiko-bot
@ 2026-09-14 10:57 ` Andy Shevchenko
1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-09-14 10:57 UTC (permalink / raw)
To: Manuel Ebner
Cc: Andy Shevchenko, Greg Kroah-Hartman, dri-devel, linux-fbdev,
linux-staging, linux-kernel
On Mon, Sep 14, 2026 at 12:09 PM Manuel Ebner <manuelebnerli@mailbox.org> wrote:
>
> Replace integer number with sizeof with the goal of prevent future truncation bugs.
preventing
>
> Signed-off-by: Manuel Ebner <manuelebnerli@mailbox.org>
So far I see only a single patch, what are the "future truncation
bugs" you are talking about?
...
> static unsigned int read_devicecode(struct fbtft_par *par)
> {
> - u8 rxbuf[8] = {0, };
> + u8 rxbuf[sizeof(u64)] = {0, };
This is an unclear change. Where did u64 come from? Why do we even
need a 8-byte buffer when we only do 4-byte operations?
> write_reg(par, 0x0000);
> par->fbtftops.read(par, rxbuf, 4);
Perhaps you need to read a datasheet for this panel and come up with a
better understanding?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-14 13:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14 9:08 [PATCH] staging: fbtft: replace number with sizeof() Manuel Ebner
2026-09-14 9:16 ` sashiko-bot
2026-09-14 10:57 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.