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

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

[Thorsten Blum]

In preparation to remove strlcat() from the kernel [1], replace two
strlcat() calls with one snprintf() call in setup_arch().

Also drop the explicit size argument of strscpy() to further simplify
the code since strscpy() can determine the size automatically when the
destination buffer has a fixed length.

[1] https://github.com/KSPP/linux/issues/370

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 arch/x86/kernel/setup.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 46882ce79c3a..b11b0ce31a27 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -21,6 +21,7 @@
 #include <linux/pci.h>
 #include <linux/random.h>
 #include <linux/root_dev.h>
+#include <linux/sprintf.h>
 #include <linux/static_call.h>
 #include <linux/sysfb.h>
 #include <linux/swiotlb.h>
@@ -915,16 +916,18 @@ void __init setup_arch(char **cmdline_p)
 	strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
 #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);
 		strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
 	}
 #endif
 	builtin_cmdline_added = true;
 #endif
 
-	strscpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
+	strscpy(command_line, boot_command_line);
 	*cmdline_p = command_line;
 
 	/*

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

[Andy Shevchenko]

On Thu, Jun 04, 2026 at 03:17:53PM +0200, Thorsten Blum wrote:
> In preparation to remove strlcat() from the kernel [1], replace two
> strlcat() calls with one snprintf() call in setup_arch().
> 
> Also drop the explicit size argument of strscpy() to further simplify
> the code since strscpy() can determine the size automatically when the
> destination buffer has a fixed length.

> [1] https://github.com/KSPP/linux/issues/370

Make it Link tag?

...

>  	strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);

This also has third argument fixed. Don't you want to change that?

>  #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`.)

>  		strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);

And this has also third argument.

>  	}
>  #endif
>  	builtin_cmdline_added = true;
>  #endif
>  
> -	strscpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
> +	strscpy(command_line, boot_command_line);

-- 
With Best Regards,
Andy Shevchenko

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

[Thorsten Blum]

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:
> > In preparation to remove strlcat() from the kernel [1], replace two
> > strlcat() calls with one snprintf() call in setup_arch().
> > 
> > Also drop the explicit size argument of strscpy() to further simplify
> > the code since strscpy() can determine the size automatically when the
> > destination buffer has a fixed length.
> 
> > [1] https://github.com/KSPP/linux/issues/370
> 
> Make it Link tag?
> 
> ...
> 
> >  	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.

> >  #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?

> >  		strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
> 
> And this has also third argument.

Same reason as above.

Thanks,
Thorsten

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

[Andy Shevchenko]

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.

> > >  		strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
> > 
> > And this has also third argument.
> 
> Same reason as above.

-- 
With Best Regards,
Andy Shevchenko

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

[Thorsten Blum]

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.

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

[Andy Shevchenko]

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).

-- 
With Best Regards,
Andy Shevchenko

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

[David Laight]

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