Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Constructing and dereferencing unaligned 16-bit pointers causes undefined behavior and potential kernel panics on strict-alignment architectures. - [High] Incorrect SPI buffer size calculation transmits uninitialized memory when multiple parameters are sent with `startbyte` enabled. -- --- Patch [1]: [PATCH 1/3] staging/fbtft: Remove prohibited space before close parenthesis --- commit b5aeca6fddf7088401c92f0dfc0d9c7d04fb863a Author: Panagiotis Gravias <[email protected]> staging/fbtft: Remove prohibited space before close parenthesis This commit cleans up a checkpatch error by removing a prohibited space before the close parenthesis in the define_fbtft_write_reg macro invocations. > diff --git a/drivers/staging/fbtft/fbtft-bus.c > b/drivers/staging/fbtft/fbtft-bus.c > index 30e436ff19e4c..38697992f5e1b 100644 > --- a/drivers/staging/fbtft/fbtft-bus.c > +++ b/drivers/staging/fbtft/fbtft-bus.c > @@ -62,9 +62,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,) > 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,) [Severity: High] This is a pre-existing issue, but does expanding this macro for 16-bit types like fbtft_write_reg16_bus16 create an unaligned pointer dereference? In the define_fbtft_write_reg() macro definition, when a startbyte is used: if (par->startbyte) { *(u8 *)par->buf = par->startbyte; buf = (buffer_type *)(par->buf + 1); offset = 1; } The pointer is advanced by 1 byte. When buffer_type is a 16-bit type, this creates an odd-aligned pointer. Later, the macro writes to it: *buf = modifier((data_type)va_arg(args, unsigned int)); This can cause unaligned memory access faults and lead to kernel panics on strict-alignment architectures. This same pattern also appears in fbtft_write_vmem16_bus8() where par->txbuf.buf + 1 is cast to a 16-bit pointer. [Severity: High] This is also a pre-existing issue within the same macro, but could the SPI buffer size calculation send uninitialized memory? When sending multiple parameters with a startbyte active, the macro packs exactly one start byte followed by the parameters. However, the transmission size is calculated as: if (len) { ... fbtft_write_buf_dc(par, par->buf, len * (sizeof(data_type) + offset), 1); } Because offset is 1, if len is greater than 1, this calculates to len * sizeof(data_type) + len, instead of the actual payload size of 1 + len * sizeof(data_type). Could this force the SPI core to transmit extra uninitialized out-of-bounds bytes and potentially corrupt the display controller's initialization state? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
