[PATCH] staging: fbtft: fix len<=0 buffer overflow in define_fbtft_write_reg()

[PATCH] staging: fbtft: fix len<=0 buffer overflow in define_fbtft_write_reg()

[Anshika Jain]

If len is 0 or negative when a define_fbtft_write_reg()-generated
function is called, len-- underflows to -1, causing the subsequent
while (i--) loop to run approximately 2^31 times and write far past
the end of buf.

This is a latent bug: nothing in the current code prevents two
adjacent negative values in an init_sequence from producing len=0,
and there is no guarantee future or out-of-tree panel definitions
won't do so.

Add an early return for len <= 0, matching the existing guard already
present in the sibling function fbtft_write_reg8_bus9().

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260830161529.14500-1-anshikajain196872@gmail.com?part=1

Signed-off-by: Anshika Jain <anshikajain196872@gmail.com>
---
 drivers/staging/fbtft/fbtft-bus.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index e03aa251c..7d7135571 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -20,6 +20,9 @@ void func(struct fbtft_par *par, int len, ...)                                \
 	int offset = 0;                                                       \
 	buffer_type *buf = (buffer_type *)par->buf;                           \
 									      \
+	if (len <= 0)                                                         \
+		return;                                                       \
+									      \
 	if (unlikely(par->debug & DEBUG_WRITE_REGISTER)) {                    \
 		va_start(args, len);                                          \
 		for (i = 0; i < len; i++) {                                   \
-- 
2.34.1

Re: [PATCH] staging: fbtft: fix len<=0 buffer overflow in define_fbtft_write_reg()

[Greg Kroah-Hartman]

On Wed, Sep 02, 2026 at 10:20:26AM +0530, Anshika Jain wrote:
> If len is 0 or negative when a define_fbtft_write_reg()-generated
> function is called, len-- underflows to -1, causing the subsequent
> while (i--) loop to run approximately 2^31 times and write far past
> the end of buf.
> 
> This is a latent bug: nothing in the current code prevents two
> adjacent negative values in an init_sequence from producing len=0,
> and there is no guarantee future or out-of-tree panel definitions
> won't do so.
> 
> Add an early return for len <= 0, matching the existing guard already
> present in the sibling function fbtft_write_reg8_bus9().
> 
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260830161529.14500-1-anshikajain196872@gmail.com?part=1
> 
> Signed-off-by: Anshika Jain <anshikajain196872@gmail.com>
> ---
>  drivers/staging/fbtft/fbtft-bus.c | 3 +++
>  1 file changed, 3 insertions(+)

Did you forget an assisted-by: tag that helped you write this patch and
changelog?

thanks,

greg k-h

Re: [PATCH] staging: fbtft: fix len<=0 buffer overflow in define_fbtft_write_reg()

[Nam Cao]

Anshika Jain <anshikajain196872@gmail.com> writes:
> This is a latent bug: nothing in the current code prevents
> two adjacent negative values in an init_sequence from producing len=0,

Can you elaborate what you mean by "two adjacent negative values in an
init_sequence from producing len=0"? How is it possible that negative
values in the init sequence causing len to be zero?

> and there is no guarantee future or out-of-tree panel definitions
> won't do so.

Basic testing of future drivers would prevent such bug. It is not on any
rarely-executed code path.

Nam

Re: [PATCH] staging: fbtft: fix len<=0 buffer overflow in define_fbtft_write_reg()

[Anshika Jain]

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

You're right, I take that back.

I said two negative values next to each other could make len equal to zero.
I checked again, and that can't actually happen in either place that calls
this code.

In the first place: there's a check just before the loop starts. If the
next value is negative, the function stops right away and never reaches the
loop. So the loop always has at least one real value to read.In the second
place: the code clears part of the number right before checking it. That
clearing always makes the check pass on the first try. So this loop also
always runs at least once.

So you're right — len can't actually reach zero through any existing code
today. This isn't a bug that's currently happening.

What's still true: the macro itself has no check for len <= 0. It only
stays safe because both callers happen to prevent it from ever reaching
zero. If either caller changes in the future, or a new caller is added
without the same care, it would break.

I'd like to keep this as a small safety check for the future, not as a fix
for a bug happening today. If you think that's not worth adding, I'm fine
dropping the patch.

Thanks for pushing on this — I should have checked both callers fully
before saying it was already happening.

On Wed, 2 Sept 2026 at 17:55, Nam Cao <namcao@linutronix.de> wrote:

> Anshika Jain <anshikajain196872@gmail.com> writes:
> > This is a latent bug: nothing in the current code prevents
> > two adjacent negative values in an init_sequence from producing len=0,
>
> Can you elaborate what you mean by "two adjacent negative values in an
> init_sequence from producing len=0"? How is it possible that negative
> values in the init sequence causing len to be zero?
>
> > and there is no guarantee future or out-of-tree panel definitions
> > won't do so.
>
> Basic testing of future drivers would prevent such bug. It is not on any
> rarely-executed code path.
>
> Nam
>

[-- Attachment #2: Type: text/html, Size: 2674 bytes --]

Re: [PATCH] staging: fbtft: fix len<=0 buffer overflow in define_fbtft_write_reg()

[Nam Cao]

No html email please, the mailing lists will not get them.

Anshika Jain <anshikajain196872@gmail.com> writes:
> I'd like to keep this as a small safety check for the future, not as a fix
> for a bug happening today. If you think that's not worth adding, I'm fine
> dropping the patch.

As mentioned, such bug would show up immediately in testing. So I vote
dropping it.

Nam

Re: [PATCH] staging: fbtft: fix len<=0 buffer overflow in define_fbtft_write_reg()

[Greg Kroah-Hartman]

On Thu, Sep 03, 2026 at 10:24:36AM +0200, Nam Cao wrote:
> No html email please, the mailing lists will not get them.
> 
> Anshika Jain <anshikajain196872@gmail.com> writes:
> > I'd like to keep this as a small safety check for the future, not as a fix
> > for a bug happening today. If you think that's not worth adding, I'm fine
> > dropping the patch.
> 
> As mentioned, such bug would show up immediately in testing. So I vote
> dropping it.

Agreed, I've now dropped it from my review queue.

thanks,

greg k-h

Re: [PATCH] staging: fbtft: fix len<=0 buffer overflow in define_fbtft_write_reg()

[sashiko-bot]

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

Pre-existing issues:
- [High] Incorrect transfer size calculation in `define_fbtft_write_reg` causes an out-of-bounds read of `par->buf` and transmits uninitialized memory.
- [High] Unaligned pointer dereference in `define_fbtft_write_reg` when `startbyte` is used with 16-bit data types.
--

commit fc3573259bc5556dbb41e0e35027f130ca8f6b63
Author: Anshika Jain <anshikajain196872@gmail.com>

staging: fbtft: fix len<=0 buffer overflow in define_fbtft_write_reg()

If len is 0 or negative when a define_fbtft_write_reg()-generated
function is called, len-- underflows to -1, causing the subsequent
while (i--) loop to run approximately 2^31 times and write far past
the end of buf.

Link: https://sashiko.dev/#/patchset/20260830161529.14500-1-anshikajain196872@gmail.com?part=1

> diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
> index 30e436ff19e4c..0927877249e74 100644
> --- a/drivers/staging/fbtft/fbtft-bus.c
> +++ b/drivers/staging/fbtft/fbtft-bus.c
> @@ -19,6 +19,9 @@ void func(struct fbtft_par *par, int len, ...)                                \
>  	int offset = 0;                                                       \
>  	buffer_type *buf = (buffer_type *)par->buf;                           \
>  									      \
> +	if (len <= 0)                                                         \
> +		return;                                                       \
> +									      \

[Severity: High]
This is a pre-existing issue, but I noticed an unaligned pointer dereference
when startbyte is used with 16-bit data types in define_fbtft_write_reg().

When par->startbyte is true, buf is assigned an offset address:

    if (par->startbyte) {
        *(u8 *)par->buf = par->startbyte;
        buf = (buffer_type *)(par->buf + 1);
        offset = 1;
    }

Since par->buf is an aligned allocation, adding 1 byte produces an odd
(unaligned) memory address. When buffer_type is a 16-bit type (e.g., __be16),
operations later in the function perform unaligned 16-bit stores:

    *buf = modifier((data_type)va_arg(args, unsigned int));
    ...
    *buf++ = modifier((data_type)va_arg(args, unsigned int));

On strict-alignment architectures, this could trigger an alignment fault and
lead to a kernel panic. Should this use the put_unaligned() family of helpers
instead?

[Severity: High]
This is also a pre-existing issue, but is the transfer size calculation in
define_fbtft_write_reg() causing an out-of-bounds read of par->buf?

The macro computes the buffer size to transmit via fbtft_write_buf_dc() as:

    fbtft_write_buf_dc(par, par->buf,
                       len * (sizeof(data_type) + offset), 1);

Because of the parentheses, this incorrectly multiplies the offset by the
remaining length instead of calculating len * sizeof(data_type) + offset.

For example, if the device tree enables the startbyte property for a display
using a 16-bit write_reg macro, len can reach 64 during fbtft_init_display().
After decrementing, len = 63, offset = 1, and sizeof(u16) = 2. This results
in calculating an outbound transmission of 189 bytes.

Since par->buf is allocated as exactly 128 bytes, this reads past the end
of the buffer. Could this leak adjacent SLUB objects to the SPI hardware and
corrupt the display initialization sequence with garbage data?

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