On Fri, 5 Jun 2026 21:28:50 +0300
Andy Shevchenko <[email protected]> wrote:
> On Fri, Jun 05, 2026 at 08:05:21PM +0200, Thorsten Blum wrote:
> > On Fri, Jun 05, 2026 at 06:55:31PM +0300, Andy Shevchenko wrote:
> > > On Fri, Jun 05, 2026 at 05:42:48PM +0200, Thorsten Blum wrote:
> > > > On Fri, Jun 05, 2026 at 07:41:11AM +0300, Andy Shevchenko wrote:
> > > > > On Thu, Jun 04, 2026 at 03:17:53PM +0200, Thorsten Blum wrote:
>
> ...
>
> > > > > > strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
> > > > > >
> > > > >
> > > > > This also has third argument fixed. Don't you want to change that?
> > > >
> > > > That doesn't work because boot_command_line, at least the declaration in
> > > > linux/init.h, doesn't have a fixed size.
> > >
> > > Ah, okay.
> > >
> > > > > > #else
> > > > > > if (builtin_cmdline[0]) {
> > > > > > + size_t len = strnlen(builtin_cmdline,
> > > > > > COMMAND_LINE_SIZE);
> > > > > > +
> > > > > > /* append boot loader cmdline to builtin */
> > > > > > - strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
> > > > > > - strlcat(builtin_cmdline, boot_command_line,
> > > > > > COMMAND_LINE_SIZE);
> > > > > > + snprintf(builtin_cmdline + len, COMMAND_LINE_SIZE -
> > > > > > len, " %s",
> > > > > > + boot_command_line);
> > > > >
> > > > > Hmm... Wouldn't GCC complain on this? (Build with `make W=1`.)
> > > >
> > > > No warnings with W=1. Why would GCC warn here?
> > >
> > > Sometimes it complains if it can't prove the size of the string to fit the
> > > destination. You said that there is no size for boot_command_line, I'm not
> > > sure I understand how GCC proves that the above snprintf() won't ever
> > > truncate
> > > the input.
> >
> > The compiler doesn't prove that this cannot truncate. It only knows the
> > buffer sizes, but not the runtime string lengths.
> >
> > snprintf() can truncate, and its return value could be used to detect
> > that. However, the previous version also ignored possible truncation by
> > strlcat(), so I didn't add new truncation handling.
>
> I understand that, but AFAIK strlcat() doesn't induce a warning in such a
> case,
> while GCC does (or at least should).
>
gcc only complains about snprintf() when it knows the the sizes
(including taking strings from arrays).
So I suspect the warnings are mostly false-positives.
But I'm not really sure using snprintf() to avoid strlcat() is a gain.
This could be:
len = strnlen(builtin_cmdline, COMMAND_LINE_SIZE);
if (strscpy(builtin_cmdline + len + 1, boot_command_line,
COMMAND_LINE_SIZE - len - 1) >= 0)
builtin_cmdline[len] = ' ';
but I suspect that doesn't return a useful string on overflow.
I've been trying to remove strcpy(), a lot of code has already
done strlen() for a bound check - so memcpy() can be used instead.
-- David