[PATCH] staging: fbtft: fix unaligned access and buffer size when startbyte is used

[PATCH] staging: fbtft: fix unaligned access and buffer size when startbyte is used

[suryasaimadhu]

When par->startbyte is non-zero, buf is advanced by one byte creating
an unaligned pointer for 16-bit types (u16, __be16). Dereferencing this
unaligned pointer can cause a kernel panic on strict-alignment
architectures.

Fix by using put_unaligned() instead of direct pointer dereference.

Also fix incorrect buffer size calculation in fbtft_write_buf_dc() call:
  len * (sizeof(data_type) + offset)  /* wrong: multiplies offset by len */
  len * sizeof(data_type) + offset    /* correct: one startbyte +
                                         len items */

Signed-off-by: suryasaimadhu <suryasaimadhu369@gmail.com>
---
 drivers/staging/fbtft/fbtft-bus.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index 2169f8d1d..cfcf4d7e7 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -4,6 +4,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/spi/spi.h>
 #include "fbtft.h"
+#include <linux/unaligned.h>
 
 /*****************************************************************************
  *
@@ -40,7 +41,7 @@ void func(struct fbtft_par *par, int len, ...)                                \
 		offset = 1;                                                   \
 	}                                                                     \
 									      \
-	*buf = modifier((data_type)va_arg(args, unsigned int));               \
+	put_unaligned(modifier((data_type)va_arg(args, unsigned int)), buf);  \
 	ret = fbtft_write_buf_dc(par, par->buf, sizeof(data_type) + offset,   \
 				 0);                                          \
 	if (ret < 0)							      \
@@ -52,11 +53,13 @@ void func(struct fbtft_par *par, int len, ...)                                \
 									      \
 	if (len) {                                                            \
 		i = len;                                                      \
-		while (i--)						      \
-			*buf++ = modifier((data_type)va_arg(args,             \
-							    unsigned int));   \
+		while (i--) {                                                 \
+			put_unaligned(modifier((data_type)va_arg(args,        \
+					       unsigned int)), buf);          \
+			buf++;                                                \
+		}                                                             \
 		fbtft_write_buf_dc(par, par->buf,			      \
-				   len * (sizeof(data_type) + offset), 1);    \
+				   len * sizeof(data_type) + offset, 1);      \
 	}                                                                     \
 out:									      \
 	va_end(args);                                                         \
-- 
2.47.3

Re: [PATCH] staging: fbtft: fix unaligned access and buffer size when startbyte is used

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fixes unaligned pointer dereferences in `fbtft_write_reg` macros but misses identical unaligned accesses in `fbtft_write_vmem16_bus8()` and `write_vmem16_bus8()` (in `fb_ra8875.c`).
--

commit 41bf3a5882f6fd09bf60fb5460097488d5497b06
Author: suryasaimadhu <suryasaimadhu369@gmail.com>

staging: fbtft: fix unaligned access and buffer size when startbyte is used

This commit addresses an unaligned memory access in fbtft-bus.c macros
by replacing direct pointer dereferences with put_unaligned() when
par->startbyte is used. It also corrects a buffer size calculation.

> diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
> index 30e436ff19e4c..0314e450799de 100644
> --- a/drivers/staging/fbtft/fbtft-bus.c
> +++ b/drivers/staging/fbtft/fbtft-bus.c
[ ... ]
> @@ -39,7 +40,7 @@ void func(struct fbtft_par *par, int len, ...)                                \
>  		offset = 1;                                                   \
>  	}                                                                     \
>  									      \
> -	*buf = modifier((data_type)va_arg(args, unsigned int));               \
> +	put_unaligned(modifier((data_type)va_arg(args, unsigned int)), buf);  \
>  	ret = fbtft_write_buf_dc(par, par->buf, sizeof(data_type) + offset,   \
>  				 0);                                          \

[Severity: High]
This is a pre-existing issue, but does this same unaligned access pattern
exist in other functions that use startbyte?

Looking at fbtft_write_vmem16_bus8(), when par->startbyte is non-zero,
txbuf16 is offset by 1 byte and directly dereferenced:

drivers/staging/fbtft/fbtft-bus.c:fbtft_write_vmem16_bus8() {
        ...
        txbuf16 = par->txbuf.buf + 1;
        ...
        for (i = 0; i < to_copy; i++)
                txbuf16[i] = cpu_to_be16(vmem16[i]);
        ...
}

Could this direct 16-bit store to an odd-aligned address still trigger
kernel panics on strict-alignment architectures during video memory updates?

A similar pattern also appears to exist in the ra8875 driver where
txbuf16 is unconditionally offset by 1 byte:

drivers/staging/fbtft/fb_ra8875.c:write_vmem16_bus8() {
        ...
        txbuf16 = par->txbuf.buf + 1;
        ...
        for (i = 0; i < to_copy; i++)
                txbuf16[i] = cpu_to_be16(vmem16[i]);
        ...
}

Should these locations also be updated to use put_unaligned() to ensure
all unaligned accesses are caught by this fix?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625103041.281190-1-suryasaimadhu369@gmail.com?part=1

[PATCH] staging: fbtft: fix unaligned vmem writes when txbuf is byte-offset

[suryasaimadhu]

fbtft_write_vmem16_bus8() and fb_ra8875's write_vmem16_bus8() offset
txbuf16 by one byte for a command/start prefix, then store 16-bit pixel
data via txbuf16[i]. On strict-alignment architectures this can fault
the same way as the write_reg path fixed in the previous patch.

Use put_unaligned() for these stores.

Signed-off-by: suryasaimadhu <suryasaimadhu369@gmail.com>
---
 drivers/staging/fbtft/fb_ra8875.c | 3 ++-
 drivers/staging/fbtft/fbtft-bus.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/fb_ra8875.c b/drivers/staging/fbtft/fb_ra8875.c
index 0ab1de664..5b95b0095 100644
--- a/drivers/staging/fbtft/fb_ra8875.c
+++ b/drivers/staging/fbtft/fb_ra8875.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 
 #include <linux/gpio/consumer.h>
+#include <linux/unaligned.h>
 #include "fbtft.h"
 
 #define DRVNAME "fb_ra8875"
@@ -262,7 +263,7 @@ static int write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
 			to_copy, remain - to_copy);
 
 		for (i = 0; i < to_copy; i++)
-			txbuf16[i] = cpu_to_be16(vmem16[i]);
+			put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);
 
 		vmem16 = vmem16 + to_copy;
 		ret = par->fbtftops.write(par, par->txbuf.buf,
diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index cfcf4d7e7..fc3faab7d 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -158,7 +158,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
 			to_copy, remain - to_copy);
 
 		for (i = 0; i < to_copy; i++)
-			txbuf16[i] = cpu_to_be16(vmem16[i]);
+			put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);
 
 		vmem16 = vmem16 + to_copy;
 		ret = par->fbtftops.write(par, par->txbuf.buf,
-- 
2.47.3

Re: [PATCH] staging: fbtft: fix unaligned vmem writes when txbuf is byte-offset

[sashiko-bot]

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

New issues:
- [Low] Missing `#include <linux/unaligned.h>` in `fbtft-bus.c`.

Pre-existing issues:
- [High] Unaligned memory stores in `define_fbtft_write_reg` macro when `startbyte` is used.
- [High] An integer underflow in `tx_array_size` leads to an out-of-bounds heap write if the `txbuflen` device property is set to a small value.
- [High] Missing NULL check for `par->txbuf.buf` in `fb_ra8875.c` leads to a NULL pointer dereference on Big Endian systems.
--

commit 57041cf8757e7586efa49957106ff7e0611f087c
Author: suryasaimadhu <suryasaimadhu369@gmail.com>

staging: fbtft: fix unaligned vmem writes when txbuf is byte-offset

This commit addresses unaligned memory access in the fbtft driver. It
updates write_vmem16_bus8() in both fb_ra8875.c and fbtft-bus.c to use
put_unaligned() when storing 16-bit pixel data.

> diff --git a/drivers/staging/fbtft/fb_ra8875.c b/drivers/staging/fbtft/fb_ra8875.c
> index 0ab1de6647d07..5b95b00953994 100644
> --- a/drivers/staging/fbtft/fb_ra8875.c
> +++ b/drivers/staging/fbtft/fb_ra8875.c
> @@ -10,6 +10,7 @@
>  #include <linux/delay.h>
>  
>  #include <linux/gpio/consumer.h>
> +#include <linux/unaligned.h>
>  #include "fbtft.h"
>  
>  #define DRVNAME "fb_ra8875"

[Severity: High]
This is a pre-existing issue, but does write_vmem16_bus8() have a potential
null pointer dereference here?

fb_ra8875.c:write_vmem16_bus8() {
    ...
    txbuf16 = par->txbuf.buf + 1;
    tx_array_size -= 2;
    *(u8 *)(par->txbuf.buf) = 0x00;
    ...
}

If the txbuflen device property is 0, par->txbuf.buf remains null on big
endian systems because the fallback allocation is gated by a little endian
check.

Could this cause a crash when assigning 0x00 unconditionally?


[Severity: High]
This is a pre-existing issue, but could tx_array_size underflow and cause
an out-of-bounds heap write?

fb_ra8875.c:write_vmem16_bus8() {
    ...
    tx_array_size = par->txbuf.len / 2;
    txbuf16 = par->txbuf.buf + 1;
    tx_array_size -= 2;
    ...
    while (remain) {
        to_copy = min(tx_array_size, remain);
    ...
}

If par->txbuf.len is set to a small value like 2 via the device tree,
tx_array_size evaluates to 1. Subtracting 2 causes it to underflow to a
huge value.

Would the min() check then just use remain, causing the loop to write past
the end of the allocated heap buffer?

> @@ -262,7 +263,7 @@ static int write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
>  			to_copy, remain - to_copy);
>  
>  		for (i = 0; i < to_copy; i++)
> -			txbuf16[i] = cpu_to_be16(vmem16[i]);
> +			put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);
>  
>  		vmem16 = vmem16 + to_copy;
>  		ret = par->fbtftops.write(par, par->txbuf.buf,
> diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
> index 30e436ff19e4c..9e25c19ec9ada 100644
> --- a/drivers/staging/fbtft/fbtft-bus.c
> +++ b/drivers/staging/fbtft/fbtft-bus.c

[Severity: High]
This is a pre-existing issue, but does the define_fbtft_write_reg macro
also suffer from unaligned stores?

fbtft-bus.c:define_fbtft_write_reg() {
    ...
    if (par->startbyte)
        *buf++ = par->startbyte | (((size & 0xF) + 1) << 4);
    ...
    *buf = modifier((data_type)va_arg(args, unsigned int));
    ...
}

If par->startbyte is used, buf is offset by 1 byte. If data_type is a
16-bit type, would this dereference create a misaligned store and fault
on strict-alignment architectures?


[Severity: High]
This is a pre-existing issue, but fbtft_write_vmem16_bus8() shares the same
integer underflow behavior as fb_ra8875.c:

fbtft-bus.c:fbtft_write_vmem16_bus8() {
    ...
    tx_array_size = par->txbuf.len / 2;
    if (par->startbyte) {
        ...
        tx_array_size -= 2;
    ...
    while (remain) {
        to_copy = min(tx_array_size, remain);
    ...
}

Could this similarly overwrite memory beyond the heap buffer if a small
txbuflen is provided?

> @@ -154,7 +154,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
>  			to_copy, remain - to_copy);
>  
>  		for (i = 0; i < to_copy; i++)
> -			txbuf16[i] = cpu_to_be16(vmem16[i]);
> +			put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);

[Severity: Low]
Does this file need to include linux/unaligned.h?

Since the header was included in fb_ra8875.c but not added here, could
this cause build failures on architectures where put_unaligned() isn't
transitively included by other headers?

>  
>  		vmem16 = vmem16 + to_copy;
>  		ret = par->fbtftops.write(par, par->txbuf.buf,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625110237.304435-1-suryasaimadhu369@gmail.com?part=1

[PATCH] staging: fbtft: fix unaligned access and txbuf safety issues

[suryasaimadhu]

This patch addresses several pre-existing issues in the fbtft driver:

1. define_fbtft_write_reg(): when par->startbyte is set, buf is
   advanced by one byte creating a misaligned pointer for 16-bit types.
   Use put_unaligned() for register writes and fix the SPI transfer
   size from len * (sizeof(data_type) + offset) to
   len * sizeof(data_type) + offset.

2. fbtft_write_vmem16_bus8() and fb_ra8875 write_vmem16_bus8(): same
   unaligned 16-bit stores when txbuf is byte-offset for a start
   prefix. Use put_unaligned() for pixel data copies.

3. tx_array_size underflow: both vmem helpers subtract 2 from
   tx_array_size when a startbyte prefix is used. A small txbuflen
   device property causes unsigned underflow and out-of-bounds heap
   writes. Fall back to the non-buffered write path when the buffer
   is too small.

4. fb_ra8875 write_vmem16_bus8(): missing NULL check for
   par->txbuf.buf, which remains NULL on big-endian when txbuflen is
   0 because the PAGE_SIZE fallback is little-endian only. Fall back
   to direct write when the buffer is missing.

Also replace empty modifier arguments in define_fbtft_write_reg() with
a no-op macro to fix checkpatch warnings.

Signed-off-by: suryasaimadhu <suryasaimadhu369@gmail.com>
---
 drivers/staging/fbtft/fb_ra8875.c | 10 +++++++++-
 drivers/staging/fbtft/fbtft-bus.c | 25 +++++++++++++++++--------
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/fbtft/fb_ra8875.c b/drivers/staging/fbtft/fb_ra8875.c
index 0ab1de664..06f650aac 100644
--- a/drivers/staging/fbtft/fb_ra8875.c
+++ b/drivers/staging/fbtft/fb_ra8875.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 
 #include <linux/gpio/consumer.h>
+#include <linux/unaligned.h>
 #include "fbtft.h"
 
 #define DRVNAME "fb_ra8875"
@@ -250,7 +251,14 @@ static int write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
 
 	remain = len / 2;
 	vmem16 = (u16 *)(par->info->screen_buffer + offset);
+
+	if (!par->txbuf.buf)
+		return par->fbtftops.write(par, vmem16, len);
+
 	tx_array_size = par->txbuf.len / 2;
+	if (tx_array_size <= 2)
+		return par->fbtftops.write(par, vmem16, len);
+
 	txbuf16 = par->txbuf.buf + 1;
 	tx_array_size -= 2;
 	*(u8 *)(par->txbuf.buf) = 0x00;
@@ -262,7 +270,7 @@ static int write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
 			to_copy, remain - to_copy);
 
 		for (i = 0; i < to_copy; i++)
-			txbuf16[i] = cpu_to_be16(vmem16[i]);
+			put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);
 
 		vmem16 = vmem16 + to_copy;
 		ret = par->fbtftops.write(par, par->txbuf.buf,
diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index 30e436ff1..52a0c5c0c 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -4,6 +4,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/spi/spi.h>
 #include "fbtft.h"
+#include <linux/unaligned.h>
 
 /*****************************************************************************
  *
@@ -11,6 +12,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, ...)                                \
 {                                                                             \
@@ -39,7 +41,7 @@ void func(struct fbtft_par *par, int len, ...)                                \
 		offset = 1;                                                   \
 	}                                                                     \
 									      \
-	*buf = modifier((data_type)va_arg(args, unsigned int));               \
+	put_unaligned(modifier((data_type)va_arg(args, unsigned int)), buf);  \
 	ret = fbtft_write_buf_dc(par, par->buf, sizeof(data_type) + offset,   \
 				 0);                                          \
 	if (ret < 0)							      \
@@ -51,20 +53,22 @@ void func(struct fbtft_par *par, int len, ...)                                \
 									      \
 	if (len) {                                                            \
 		i = len;                                                      \
-		while (i--)						      \
-			*buf++ = modifier((data_type)va_arg(args,             \
-							    unsigned int));   \
+		while (i--) {                                                 \
+			put_unaligned(modifier((data_type)va_arg(args,        \
+					       unsigned int)), buf);          \
+			buf++;                                                \
+		}                                                             \
 		fbtft_write_buf_dc(par, par->buf,			      \
-				   len * (sizeof(data_type) + offset), 1);    \
+				   len * sizeof(data_type) + offset, 1);      \
 	}                                                                     \
 out:									      \
 	va_end(args);                                                         \
 }                                                                             \
 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)
 
 void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...)
 {
@@ -142,19 +146,24 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
 	tx_array_size = par->txbuf.len / 2;
 
 	if (par->startbyte) {
+		if (tx_array_size <= 2)
+			return par->fbtftops.write(par, vmem16, len);
 		txbuf16 = par->txbuf.buf + 1;
 		tx_array_size -= 2;
 		*(u8 *)(par->txbuf.buf) = par->startbyte | 0x2;
 		startbyte_size = 1;
 	}
 
+	if (!tx_array_size)
+		return par->fbtftops.write(par, vmem16, len);
+
 	while (remain) {
 		to_copy = min(tx_array_size, remain);
 		dev_dbg(par->info->device, "to_copy=%zu, remain=%zu\n",
 			to_copy, remain - to_copy);
 
 		for (i = 0; i < to_copy; i++)
-			txbuf16[i] = cpu_to_be16(vmem16[i]);
+			put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);
 
 		vmem16 = vmem16 + to_copy;
 		ret = par->fbtftops.write(par, par->txbuf.buf,
-- 
2.47.3

Re: [PATCH] staging: fbtft: fix unaligned access and txbuf safety issues

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware protocol violation in unbuffered SPI write fallback paths.
--

commit 3d08e05dccbf9cbe108bf3c042c710ace7899fe1
Author: suryasaimadhu <suryasaimadhu369@gmail.com>

staging: fbtft: fix unaligned access and txbuf safety issues

This commit fixes unaligned access issues during register writes and pixel
data copies in the fbtft driver. It also adds bounds checking and fallbacks
for when the transmit buffer size underflows or is missing.

> diff --git a/drivers/staging/fbtft/fb_ra8875.c b/drivers/staging/fbtft/fb_ra8875.c
> index 0ab1de6647d07..06f650aacc204 100644
> --- a/drivers/staging/fbtft/fb_ra8875.c
> +++ b/drivers/staging/fbtft/fb_ra8875.c
> @@ -250,7 +251,14 @@ static int write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
>  
>  	remain = len / 2;
>  	vmem16 = (u16 *)(par->info->screen_buffer + offset);
> +
> +	if (!par->txbuf.buf)
> +		return par->fbtftops.write(par, vmem16, len);
> +
>  	tx_array_size = par->txbuf.len / 2;
> +	if (tx_array_size <= 2)
> +		return par->fbtftops.write(par, vmem16, len);
> +

[Severity: High]
Does this fallback bypass the 0x00 command prefix required by the RA8875
hardware?

Since vmem16 points directly to the framebuffer memory, this direct write
will omit the prefix, which might cause the display controller to
misinterpret the first pixel as a command.

Additionally, could this skip the cpu_to_be16() conversion on
little-endian CPUs, sending pixels in the wrong byte order?

Would it be safer to return an error if the required transmission buffer
is unavailable or too small to prepend the hardware protocol prefix?

>  	txbuf16 = par->txbuf.buf + 1;
>  	tx_array_size -= 2;
>  	*(u8 *)(par->txbuf.buf) = 0x00;

[ ... ]

> diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
> index 30e436ff19e4c..52a0c5c0cbaed 100644
> --- a/drivers/staging/fbtft/fbtft-bus.c
> +++ b/drivers/staging/fbtft/fbtft-bus.c

[ ... ]

> @@ -142,19 +146,24 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
>  	tx_array_size = par->txbuf.len / 2;
>  
>  	if (par->startbyte) {
> +		if (tx_array_size <= 2)
> +			return par->fbtftops.write(par, vmem16, len);

[Severity: High]
Does this skip prepending the par->startbyte prefix when the buffer is
too small, despite the prefix being explicitly required?

Like the RA8875 case above, this fallback bypasses both the command
prefix and endianness conversions.

>  		txbuf16 = par->txbuf.buf + 1;
>  		tx_array_size -= 2;
>  		*(u8 *)(par->txbuf.buf) = par->startbyte | 0x2;
>  		startbyte_size = 1;
>  	}
>  
> +	if (!tx_array_size)
> +		return par->fbtftops.write(par, vmem16, len);
> +

[Severity: High]
Could this result in the same issue of bypassing necessary prefixes and
endianness conversions if tx_array_size evaluates to zero?

Should we return an error instead of silently sending incorrectly
formatted data?

>  	while (remain) {
>  		to_copy = min(tx_array_size, remain);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625114215.325973-1-suryasaimadhu369@gmail.com?part=1

Re: [PATCH] staging: fbtft: fix unaligned access and txbuf safety issues

[Dan Carpenter]

On Thu, Jun 25, 2026 at 07:42:15PM +0800, suryasaimadhu wrote:
> This patch addresses several pre-existing issues in the fbtft driver:
> 
> 1. define_fbtft_write_reg(): when par->startbyte is set, buf is
>    advanced by one byte creating a misaligned pointer for 16-bit types.
>    Use put_unaligned() for register writes and fix the SPI transfer
>    size from len * (sizeof(data_type) + offset) to
>    len * sizeof(data_type) + offset.
> 
> 2. fbtft_write_vmem16_bus8() and fb_ra8875 write_vmem16_bus8(): same
>    unaligned 16-bit stores when txbuf is byte-offset for a start
>    prefix. Use put_unaligned() for pixel data copies.
> 
> 3. tx_array_size underflow: both vmem helpers subtract 2 from
>    tx_array_size when a startbyte prefix is used. A small txbuflen
>    device property causes unsigned underflow and out-of-bounds heap
>    writes. Fall back to the non-buffered write path when the buffer
>    is too small.
> 
> 4. fb_ra8875 write_vmem16_bus8(): missing NULL check for
>    par->txbuf.buf, which remains NULL on big-endian when txbuflen is
>    0 because the PAGE_SIZE fallback is little-endian only. Fall back
>    to direct write when the buffer is missing.
> 
> Also replace empty modifier arguments in define_fbtft_write_reg() with
> a no-op macro to fix checkpatch warnings.
> 
> Signed-off-by: suryasaimadhu <suryasaimadhu369@gmail.com>

Is this how you would sign a legal document?

This patch does too many things at once.  Split it up.  Also please
a delay between sending us patches.  Otherwise it's overwhelming to
deal with.  Bunch them together in a patchset instead of sending them
one by one.

regards,
dan carpenter

Re: [PATCH] staging: fbtft: fix unaligned access and txbuf safety issues

[Andy Shevchenko]

On Thu, Jun 25, 2026 at 07:42:15PM +0800, suryasaimadhu wrote:
> This patch addresses several pre-existing issues in the fbtft driver:
> 
> 1. define_fbtft_write_reg(): when par->startbyte is set, buf is
>    advanced by one byte creating a misaligned pointer for 16-bit types.
>    Use put_unaligned() for register writes and fix the SPI transfer
>    size from len * (sizeof(data_type) + offset) to
>    len * sizeof(data_type) + offset.
> 
> 2. fbtft_write_vmem16_bus8() and fb_ra8875 write_vmem16_bus8(): same
>    unaligned 16-bit stores when txbuf is byte-offset for a start
>    prefix. Use put_unaligned() for pixel data copies.
> 
> 3. tx_array_size underflow: both vmem helpers subtract 2 from
>    tx_array_size when a startbyte prefix is used. A small txbuflen
>    device property causes unsigned underflow and out-of-bounds heap
>    writes. Fall back to the non-buffered write path when the buffer
>    is too small.
> 
> 4. fb_ra8875 write_vmem16_bus8(): missing NULL check for
>    par->txbuf.buf, which remains NULL on big-endian when txbuflen is
>    0 because the PAGE_SIZE fallback is little-endian only. Fall back
>    to direct write when the buffer is missing.
> 
> Also replace empty modifier arguments in define_fbtft_write_reg() with
> a no-op macro to fix checkpatch warnings.

This looks like v2 of the thing without changelog and addressing the comments
that have been given against v1. I'm not even going to review that.
Please, consolidate feedback, take your time to study process documentation
(Documentation/process/* in the Linux kernel source tree) and try again a bit
later.

(The fix and report are valuable in general, thanks for doing that.)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: fbtft: fix unaligned vmem writes when txbuf is byte-offset

[kernel test robot]

Hi suryasaimadhu,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/suryasaimadhu/staging-fbtft-fix-unaligned-vmem-writes-when-txbuf-is-byte-offset/20260625-190423
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20260625110237.304435-1-suryasaimadhu369%40gmail.com
patch subject: [PATCH] staging: fbtft: fix unaligned vmem writes when txbuf is byte-offset
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20260625/202606252119.rClVa8On-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 16.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260625/202606252119.rClVa8On-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606252119.rClVa8On-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/staging/fbtft/fbtft-bus.c: In function 'fbtft_write_vmem16_bus8':
>> drivers/staging/fbtft/fbtft-bus.c:157:25: error: implicit declaration of function 'put_unaligned' [-Wimplicit-function-declaration]
     157 |                         put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);
         |                         ^~~~~~~~~~~~~


vim +/put_unaligned +157 drivers/staging/fbtft/fbtft-bus.c

   113	
   114	/*****************************************************************************
   115	 *
   116	 *   int (*write_vmem)(struct fbtft_par *par);
   117	 *
   118	 *****************************************************************************/
   119	
   120	/* 16 bit pixel over 8-bit databus */
   121	int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
   122	{
   123		u16 *vmem16;
   124		__be16 *txbuf16 = par->txbuf.buf;
   125		size_t remain;
   126		size_t to_copy;
   127		size_t tx_array_size;
   128		int i;
   129		int ret = 0;
   130		size_t startbyte_size = 0;
   131	
   132		remain = len / 2;
   133		vmem16 = (u16 *)(par->info->screen_buffer + offset);
   134	
   135		gpiod_set_value(par->gpio.dc, 1);
   136	
   137		/* non buffered write */
   138		if (!par->txbuf.buf)
   139			return par->fbtftops.write(par, vmem16, len);
   140	
   141		/* buffered write */
   142		tx_array_size = par->txbuf.len / 2;
   143	
   144		if (par->startbyte) {
   145			txbuf16 = par->txbuf.buf + 1;
   146			tx_array_size -= 2;
   147			*(u8 *)(par->txbuf.buf) = par->startbyte | 0x2;
   148			startbyte_size = 1;
   149		}
   150	
   151		while (remain) {
   152			to_copy = min(tx_array_size, remain);
   153			dev_dbg(par->info->device, "to_copy=%zu, remain=%zu\n",
   154				to_copy, remain - to_copy);
   155	
   156			for (i = 0; i < to_copy; i++)
 > 157				put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);
   158	
   159			vmem16 = vmem16 + to_copy;
   160			ret = par->fbtftops.write(par, par->txbuf.buf,
   161							startbyte_size + to_copy * 2);
   162			if (ret < 0)
   163				return ret;
   164			remain -= to_copy;
   165		}
   166	
   167		return ret;
   168	}
   169	EXPORT_SYMBOL(fbtft_write_vmem16_bus8);
   170	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] staging: fbtft: fix unaligned vmem writes when txbuf is byte-offset

[kernel test robot]

Hi suryasaimadhu,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/suryasaimadhu/staging-fbtft-fix-unaligned-vmem-writes-when-txbuf-is-byte-offset/20260625-190423
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20260625110237.304435-1-suryasaimadhu369%40gmail.com
patch subject: [PATCH] staging: fbtft: fix unaligned vmem writes when txbuf is byte-offset
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20260625/202606252144.F56jkpaD-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 6cc609bb250b21b47fc7d394b4019101e9983597)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260625/202606252144.F56jkpaD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606252144.F56jkpaD-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/staging/fbtft/fbtft-bus.c:157:4: error: call to undeclared function 'put_unaligned'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     157 |                         put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);
         |                         ^
   1 error generated.


vim +/put_unaligned +157 drivers/staging/fbtft/fbtft-bus.c

   113	
   114	/*****************************************************************************
   115	 *
   116	 *   int (*write_vmem)(struct fbtft_par *par);
   117	 *
   118	 *****************************************************************************/
   119	
   120	/* 16 bit pixel over 8-bit databus */
   121	int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
   122	{
   123		u16 *vmem16;
   124		__be16 *txbuf16 = par->txbuf.buf;
   125		size_t remain;
   126		size_t to_copy;
   127		size_t tx_array_size;
   128		int i;
   129		int ret = 0;
   130		size_t startbyte_size = 0;
   131	
   132		remain = len / 2;
   133		vmem16 = (u16 *)(par->info->screen_buffer + offset);
   134	
   135		gpiod_set_value(par->gpio.dc, 1);
   136	
   137		/* non buffered write */
   138		if (!par->txbuf.buf)
   139			return par->fbtftops.write(par, vmem16, len);
   140	
   141		/* buffered write */
   142		tx_array_size = par->txbuf.len / 2;
   143	
   144		if (par->startbyte) {
   145			txbuf16 = par->txbuf.buf + 1;
   146			tx_array_size -= 2;
   147			*(u8 *)(par->txbuf.buf) = par->startbyte | 0x2;
   148			startbyte_size = 1;
   149		}
   150	
   151		while (remain) {
   152			to_copy = min(tx_array_size, remain);
   153			dev_dbg(par->info->device, "to_copy=%zu, remain=%zu\n",
   154				to_copy, remain - to_copy);
   155	
   156			for (i = 0; i < to_copy; i++)
 > 157				put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);
   158	
   159			vmem16 = vmem16 + to_copy;
   160			ret = par->fbtftops.write(par, par->txbuf.buf,
   161							startbyte_size + to_copy * 2);
   162			if (ret < 0)
   163				return ret;
   164			remain -= to_copy;
   165		}
   166	
   167		return ret;
   168	}
   169	EXPORT_SYMBOL(fbtft_write_vmem16_bus8);
   170	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] staging: fbtft: fix unaligned vmem writes when txbuf is byte-offset

[Andy Shevchenko]

On Thu, Jun 25, 2026 at 07:02:37PM +0800, suryasaimadhu wrote:
> fbtft_write_vmem16_bus8() and fb_ra8875's write_vmem16_bus8() offset
> txbuf16 by one byte for a command/start prefix, then store 16-bit pixel
> data via txbuf16[i]. On strict-alignment architectures this can fault
> the same way as the write_reg path fixed in the previous patch.

.write_reg()

Can you provide a real world example of the failure?

> Use put_unaligned() for these stores.

Sounds like a fix without Fixes tag.

> Signed-off-by: suryasaimadhu <suryasaimadhu369@gmail.com>

Use your name in accordance with your official documents.

...

> +++ b/drivers/staging/fbtft/fb_ra8875.c
> @@ -10,6 +10,7 @@
>  #include <linux/delay.h>

>  

You should move this blank line to be...

>  #include <linux/gpio/consumer.h>
> +#include <linux/unaligned.h>

...here.

>  #include "fbtft.h"

...

>  #define DRVNAME "fb_ra8875"

Are other drivers also being affected?

...

> +++ b/drivers/staging/fbtft/fbtft-bus.c

Also needs linux/unaligned.h.

> +			put_unaligned(cpu_to_be16(vmem16[i]), &txbuf16[i]);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: fbtft: fix unaligned access and buffer size when startbyte is used

[Andy Shevchenko]

On Thu, Jun 25, 2026 at 06:30:41PM +0800, suryasaimadhu wrote:
> When par->startbyte is non-zero, buf is advanced by one byte creating
> an unaligned pointer for 16-bit types (u16, __be16). Dereferencing this
> unaligned pointer can cause a kernel panic on strict-alignment
> architectures.
> 
> Fix by using put_unaligned() instead of direct pointer dereference.
> 
> Also fix incorrect buffer size calculation in fbtft_write_buf_dc() call:
>   len * (sizeof(data_type) + offset)  /* wrong: multiplies offset by len */
>   len * sizeof(data_type) + offset    /* correct: one startbyte +
>                                          len items */

Same comments as per your other patch contributions. Make it cleaner next time.
So it seem a v2 should be a mini-series with fixes for different
issues/drivers/et cetera.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: fbtft: fix unaligned access and buffer size when startbyte is used

[David Laight]

On Thu, 25 Jun 2026 18:30:41 +0800
suryasaimadhu <suryasaimadhu369@gmail.com> wrote:

> When par->startbyte is non-zero, buf is advanced by one byte creating
> an unaligned pointer for 16-bit types (u16, __be16). Dereferencing this
> unaligned pointer can cause a kernel panic on strict-alignment
> architectures.
> 
> Fix by using put_unaligned() instead of direct pointer dereference.
> 
> Also fix incorrect buffer size calculation in fbtft_write_buf_dc() call:
>   len * (sizeof(data_type) + offset)  /* wrong: multiplies offset by len */
>   len * sizeof(data_type) + offset    /* correct: one startbyte +
>                                          len items */

That should probably be a separate patch.

> 
> Signed-off-by: suryasaimadhu <suryasaimadhu369@gmail.com>
> ---
>  drivers/staging/fbtft/fbtft-bus.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
> index 2169f8d1d..cfcf4d7e7 100644
> --- a/drivers/staging/fbtft/fbtft-bus.c
> +++ b/drivers/staging/fbtft/fbtft-bus.c
> @@ -4,6 +4,7 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/spi/spi.h>
>  #include "fbtft.h"
> +#include <linux/unaligned.h>
>  
>  /*****************************************************************************
>   *
> @@ -40,7 +41,7 @@ void func(struct fbtft_par *par, int len, ...)                                \

I'd consider changing that to:
	func(struct fbtft_par *par, int len, u8 cmd, ...)
and probably reducing len by one.

It makes it more obvious that the first parameter is mandatory and the ... is
associated data.

	David

>  		offset = 1;                                                   \
>  	}                                                                     \
>  									      \
> -	*buf = modifier((data_type)va_arg(args, unsigned int));               \
> +	put_unaligned(modifier((data_type)va_arg(args, unsigned int)), buf);  \
>  	ret = fbtft_write_buf_dc(par, par->buf, sizeof(data_type) + offset,   \
>  				 0);                                          \
>  	if (ret < 0)							      \
> @@ -52,11 +53,13 @@ void func(struct fbtft_par *par, int len, ...)                                \
>  									      \
>  	if (len) {                                                            \
>  		i = len;                                                      \
> -		while (i--)						      \
> -			*buf++ = modifier((data_type)va_arg(args,             \
> -							    unsigned int));   \
> +		while (i--) {                                                 \
> +			put_unaligned(modifier((data_type)va_arg(args,        \
> +					       unsigned int)), buf);          \
> +			buf++;                                                \
> +		}                                                             \
>  		fbtft_write_buf_dc(par, par->buf,			      \
> -				   len * (sizeof(data_type) + offset), 1);    \
> +				   len * sizeof(data_type) + offset, 1);      \
>  	}                                                                     \
>  out:									      \
>  	va_end(args);                                                         \