Re: [PATCH 1/3] staging/fbtft: Remove prohibited space before close parenthesis

[sashiko-bot@kernel.org]

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 <graviaspanagiotis1@gmail.com>

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/20260714190958.219437-2-graviaspanagiotis1@gmail.com?part=1