[PATCH] x86: replace char builtin_cmdline[COMMAND_LINE_SIZE] with struct seq_buf

Michal Gorlas posted 1 patch 1 week, 5 days ago
arch/x86/include/asm/cmdline.h |  3 ++-
arch/x86/kernel/setup.c        | 16 ++++++++++------
arch/x86/lib/cmdline.c         | 10 +++++++---
3 files changed, 19 insertions(+), 10 deletions(-)
[PATCH] x86: replace char builtin_cmdline[COMMAND_LINE_SIZE] with struct seq_buf
Posted by Michal Gorlas 1 week, 5 days ago
In preparation for removing the strlcat API[1], replace the char
builtin_cmdline[COMMAND_LINE_SIZE] with a struct seq_buf.

Removes strlcat() usage when combining CONFIG_CMDLINE with the boot
loader command line.
Use seq_buf_init() to initialize the buffer and seq_buf_puts() to
append the strings instead.

cmdline_find_option() and cmdline_find_option_bool() logic remain
unchanged, and continue to operate on the underlying .buffer member.

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

Signed-off-by: Michal Gorlas <michal.gorlas@9elements.com>
---
 arch/x86/include/asm/cmdline.h |  3 ++-
 arch/x86/kernel/setup.c        | 16 ++++++++++------
 arch/x86/lib/cmdline.c         | 10 +++++++---
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/cmdline.h b/arch/x86/include/asm/cmdline.h
index 6cbd9ae58b21..f3ccfc2d4851 100644
--- a/arch/x86/include/asm/cmdline.h
+++ b/arch/x86/include/asm/cmdline.h
@@ -2,9 +2,10 @@
 #ifndef _ASM_X86_CMDLINE_H
 #define _ASM_X86_CMDLINE_H
 
+#include <linux/seq_buf.h>
 #include <asm/setup.h>
 
-extern char builtin_cmdline[COMMAND_LINE_SIZE];
+extern struct seq_buf builtin_cmdline;
 
 int cmdline_find_option_bool(const char *cmdline_ptr, const char *option);
 int cmdline_find_option(const char *cmdline_ptr, const char *option,
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 1b2edd07a3e1..17a1ffc8c3fd 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/seq_buf.h>
 #include <linux/static_call.h>
 #include <linux/swiotlb.h>
 #include <linux/tboot.h>
@@ -228,7 +229,8 @@ unsigned long saved_video_mode;
 
 static char __initdata command_line[COMMAND_LINE_SIZE];
 #ifdef CONFIG_CMDLINE_BOOL
-char builtin_cmdline[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;
+char cmdline_buf[COMMAND_LINE_SIZE];
+struct seq_buf builtin_cmdline;
 bool builtin_cmdline_added __ro_after_init;
 #endif
 
@@ -907,14 +909,16 @@ void __init setup_arch(char **cmdline_p)
 #endif
 
 #ifdef CONFIG_CMDLINE_BOOL
+	seq_buf_init(&builtin_cmdline, cmdline_buf, COMMAND_LINE_SIZE);
+	seq_buf_puts(&builtin_cmdline, CONFIG_CMDLINE);
 #ifdef CONFIG_CMDLINE_OVERRIDE
-	strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+	strscpy(boot_command_line, builtin_cmdline.buffer, COMMAND_LINE_SIZE);
 #else
-	if (builtin_cmdline[0]) {
+	if (builtin_cmdline.buffer[0]) {
 		/* append boot loader cmdline to builtin */
-		strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
-		strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
-		strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+		seq_buf_puts(&builtin_cmdline, " ");
+		seq_buf_puts(&builtin_cmdline, boot_command_line);
+		strscpy(boot_command_line, builtin_cmdline.buffer, COMMAND_LINE_SIZE);
 	}
 #endif
 	builtin_cmdline_added = true;
diff --git a/arch/x86/lib/cmdline.c b/arch/x86/lib/cmdline.c
index c65cd5550454..620c14676152 100644
--- a/arch/x86/lib/cmdline.c
+++ b/arch/x86/lib/cmdline.c
@@ -214,7 +214,8 @@ int cmdline_find_option_bool(const char *cmdline, const char *option)
 		return ret;
 
 	if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && !builtin_cmdline_added)
-		return __cmdline_find_option_bool(builtin_cmdline, COMMAND_LINE_SIZE, option);
+		return __cmdline_find_option_bool(builtin_cmdline.buffer,
+						  COMMAND_LINE_SIZE, option);
 
 	return ret;
 }
@@ -224,12 +225,15 @@ int cmdline_find_option(const char *cmdline, const char *option, char *buffer,
 {
 	int ret;
 
-	ret = __cmdline_find_option(cmdline, COMMAND_LINE_SIZE, option, buffer, bufsize);
+	ret = __cmdline_find_option(cmdline, COMMAND_LINE_SIZE, option, buffer,
+				    bufsize);
 	if (ret > 0)
 		return ret;
 
 	if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && !builtin_cmdline_added)
-		return __cmdline_find_option(builtin_cmdline, COMMAND_LINE_SIZE, option, buffer, bufsize);
+		return __cmdline_find_option(builtin_cmdline.buffer,
+					     COMMAND_LINE_SIZE, option, buffer,
+					     bufsize);
 
 	return ret;
 }

---
base-commit: 0f61b1860cc3f52aef9036d7235ed1f017632193
change-id: 20260322-x86-remove-strlcat-5762cc8581fb

Best regards,
--  
Michal Gorlas <michal.gorlas@9elements.com>
Re: [PATCH] x86: replace char builtin_cmdline[COMMAND_LINE_SIZE] with struct seq_buf
Posted by Michal Gorlas 1 week, 4 days ago
Okay, so the Sashiko bot found out two regressions [1] that I missed out.
First one about builtin_cmdline.buffer not being initialized for any
early callers of __cmdline_find_option() and __cmdline_find_option_bool()
could be fixed by letting builtin_cmdline be initialized like this:
+ char cmdline_buf[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;
+ struct seq_buf builtin_cmdline = {
+   .buffer = cmdline_buf,
+   .size = COMMAND_LINE_SIZE,
+   .len = sizeof(CONFIG_CMDLINE) - 1,
+ };

But as for the second issue (handling case when (builtin_cmdline
 + boot_command_line) > COMMAND_LINE_SIZE), I am not sure whether even
using seq_buf makes any sense at all. snprintf() (also mentioned as
potential replacement for strlcat in [2]) would make more sense (I think)
and the whole patch would suffice to this:
	if (builtin_cmdline[0]) {
    	/* append boot loader cmdline to builtin */
-       strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
-       strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
+       snprintf(builtin_cmdline, COMMAND_LINE_SIZE, "%s %s",
+                CONFIG_CMDLINE, boot_command_line);
        strscpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
    }

Let me know whether this makes sense.

Best,
Michal    

[1] - https://sashiko.dev/#/patchset/20260322-x86-remove-strlcat-v1-1-4ff893595133%409elements.com
[2] - https://github.com/KSPP/linux/issues/370