Re: [PATCH] x86/setup: replace strlcat() with snprintf() in setup_arch()

[David Laight <david.laight.linux@gmail.com>]

On Fri, 5 Jun 2026 21:28:50 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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