Re: [PATCH] media: siano: Replace strlcat() with snprintf() in sms_ir_init()

[Mariia Nikitash]

On Fri, Nov 6, 2026 at 8:06 AM Kees Cook <kees@xxxxxxxxxx> wrote:
>
>
>
> On November 6, 2026 2:27:23 AM PST, David Laight <david.laight.linux@xxxxxxxxx> wrote:
> >On Thu, 5 Nov 2026 17:44:00 -0800
> >Mariia Nikitash <nikitash.mariiaw@xxxxxxxxx> wrote:
> >
> >> On Thu, Nov 5, 2026 at 3:32 AM David Laight <david.laight.linux@xxxxxxxxx>
> >> wrote:
> >> >
> >> > On Wed, 4 Nov 2026 12:25:17 +0100
> >> > Alejandro Colomar <alx+linux-hardening@xxxxxxxxxx> wrote:
> >> >
> >> > > Hi Mariia,
> >> > >
> >> > > > Date: 2026-11-04 01:02:39-0800
> >> > > > From: Mariia Nikitash <nikitash.mariiaw@xxxxxxxxx>
> >> > > >
> >> > > > In preparation for removing the deprecated strlcat() API[1],
> >> > > > replace the strscpy() and strlcat() calls with a single snprintf()
> >> > > > call to construct the IR device path.
> >> > > >
> >> > > > Both the source and destination buffers have the same capacity,
> >> > >
> >> > > The line above seems irrelevant, and potentially confusing.
> >> > > The size of the input is not important, because since we're appending
> >> > > stuff after it, truncation may actually happen.
> >> >
> >> > And gcc's stupid format overflow check is going to complain.
> >>
> >> Thanks for pointing this out! I initially tested with Clang, but reproduced
> >> the warning with GCC. I've switched to seq_buf for v3.
> >
> >That is entirely OTT.
> >Currently scnprintf() doesn't complain.
>
>

Hi Kees,


> What does GCC say about it? Is it mad about src >= dest length but the format adds chars?

GCC 13 reported -Wformat-truncation:

error: ‘/ir0’ directive output may be truncated writing 4 bytes
into a region of size between 1 and 32


note: ‘snprintf’ output between 5 and 36 bytes
into a destination of size 32

Both devpath and ir.phys are 32-byte buffers, so the suffix may not fit.


> I agree that seq_buf isn't needed here; that's really only for complicated cases where the simpler solutions don't fit. (In this case, snprintf-like is best.)

That makes sense.


> Both seq_buf and scnprintf avoid the warning because GCC just happens to not recognize the pattern when using those helpers, so this is just silencing the warning, much like strlcat() did too.
>
> Truncation was the norm already, so scnprintf is probably the best fit, but I think it may be worth looking at the surrounding code and conventions to decide if truncation should be communicated upward to the caller (i.e. fail), or if the structures involved should be adjusted to avoid truncation chance, etc etc. If possible we should avoid the weakness, but if not, just use the minimal solution here, which is scnprintf, though that kicks the warning down the road...

I traced the USB/SDIO initialization paths and checked the caller's
error handling. The existing code permits truncation, so I kept v4
focused on preserving that behavior with scnprintf(). Any broader
changes to truncation handling could be addressed separately.

Thank you!

>
> -Kees
>
>

Mariia
> --
> Kees Cook