Thank you for the review. These are pre-existing issues not introduced by my patch. I will investigate and send fixes for both the unaligned access and buffer size calculation.
Regards, suryasaimadhu. On Thu, 25 Jun 2026 at 15:11, <[email protected]> wrote: > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) > to consider: > > Pre-existing issues: > - [High] Incorrect buffer size calculation causes out-of-bounds read and > information leak over SPI when `startbyte` is used. > - [High] Pointer arithmetic creates an unaligned pointer for 16-bit register > writes when `startbyte` is used, causing an unaligned memory access. > -- > > commit 5efb24fc5a4ff6b0f2481a7887082d742684defc > Author: suryasaimadhu <[email protected]> > > staging: fbtft: replace empty modifier argument with no-op macro > > This commit defines an identity function fbtft_write_reg_no_modifier() and > uses it to replace empty modifier arguments in define_fbtft_write_reg() > macro calls to fix checkpatch errors. > > > diff --git a/drivers/staging/fbtft/fbtft-bus.c > > b/drivers/staging/fbtft/fbtft-bus.c > > index 30e436ff19e4c..2169f8d1d1b83 100644 > > --- a/drivers/staging/fbtft/fbtft-bus.c > > +++ b/drivers/staging/fbtft/fbtft-bus.c > > @@ -11,6 +11,7 @@ > > * > > > > *****************************************************************************/ > > > > +#define fbtft_write_reg_no_modifier(x) (x) > > #define define_fbtft_write_reg(func, buffer_type, data_type, modifier) > > \ > > void func(struct fbtft_par *par, int len, ...) > > \ > > { > > \ > > [Severity: High] > This is a pre-existing issue, but does this pointer arithmetic create an > unaligned memory access on strict-alignment architectures? > > When par->startbyte is non-zero, buf is advanced by one byte in > define_fbtft_write_reg(): > > if (par->startbyte) { > *(u8 *)par->buf = par->startbyte; > buf = (buffer_type *)(par->buf + 1); > offset = 1; > } > > Since buffer_type can be a 16-bit type like u16 or __be16 (for example, in > fbtft_write_reg16_bus8 or fbtft_write_reg16_bus16), adding 1 byte to the > aligned par->buf creates an odd address. > > The code then dereferences this unaligned pointer: > > *buf = modifier((data_type)va_arg(args, unsigned int)); > > Could this trigger an alignment fault resulting in a kernel panic on > architectures that require strict alignment? > > [ ... ] > > > @@ -62,9 +63,9 @@ out: > > \ > > } > > \ > > EXPORT_SYMBOL(func); > > > > -define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, ) > > +define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, > > fbtft_write_reg_no_modifier) > > define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, cpu_to_be16) > > -define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, ) > > +define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, > > fbtft_write_reg_no_modifier) > > [Severity: High] > This is a pre-existing issue, but is there an out-of-bounds read when > calculating the length to send over SPI? > > When startbyte is configured, offset is set to 1. The macro > define_fbtft_write_reg() uses the following size calculation for the final > transfer: > > fbtft_write_buf_dc(par, par->buf, > len * (sizeof(data_type) + offset), 1); > > Because the parenthesis includes both sizeof(data_type) and offset, the size > grows too quickly. For example, with a len of 63, a 16-bit data_type, and an > offset of 1, the size evaluates to 63 * (2 + 1) = 189 bytes. > > Does this result in reading past the end of par->buf and sending > uninitialized kernel heap memory to the display controller, potentially > causing a kernel panic if it crosses an unmapped page boundary? > > -- > Sashiko AI review ยท > https://sashiko.dev/#/patchset/[email protected]?part=1
