Re: [PATCH] staging: fbtft: prefer scnprintf over sprintf in fbtft-core.c

On Wed, Aug 19, 2026 at 7:45 PM Tomasz Unger <[email protected]> wrote:
>
> Using sprintf has potential for buffer overflows if the formatted

sprintf()

> string exceeds the destination buffer size. Replace it with
> scnprintf, passing sizeof() of the fixed-size stack buffers

scnprintf()

> (text1[50] and text2[50]) so the write is always bounded.

> In practice an overflow is very unlikely here: text1 only needs
> room for a size_t value from an SPI TX buffer length, and text2
> formats three small integers (bus number, chip select, and
> frequency in MHz) that always come from real hardware ranges far
> below the theoretical worst case for their types. This is
> therefore a defense-in-depth hardening rather than a fix for an
> observed or easily triggered issue.

This paragraph is not for the commit message, rather for the comment.

...

>         if (par->txbuf.buf && par->txbuf.len >= 1024)
> -               sprintf(text1, ", %zu KiB buffer memory", par->txbuf.len >> 10);

The 64-bit number takes up to 20 decimal digits, the rest here is 20
and one for NUL terminator. 50 is more than enough.

> +               scnprintf(text1, sizeof(text1), ", %zu KiB buffer memory", par->txbuf.len >> 10);
>         if (spi)
> -               sprintf(text2, ", spi%d.%d at %d MHz", spi->controller->bus_num,

For this it might be worse, 3 32-bit integers that may take up to 10
decimal digits + sign, so 33 altogether and the rest is 13 and one for
NUL terminator, so still below 50 in the longest case.

> -                       spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);
> +               scnprintf(text2, sizeof(text2), ", spi%d.%d at %d MHz", spi->controller->bus_num,
> +                         spi_get_chipselect(spi, 0), spi->max_speed_hz / 1000000);

Taking the above (and if there are no compiler warnings currently
present) this change is an unneeded churn.

-- 
With Best Regards,
Andy Shevchenko