[v2,2/5] x86/setup: Rework cmdline_cook() to be compatible with -Wwrite-strings

Commit Message

Andrew Cooper Nov. 21, 2023, 8:15 p.m. UTC

Rework the logic in __start_xen() to not potentially pass NULL into
cmdline_cook().  This makes the logic easier to follow too, and the rest of
__start_xen() is safe when initialising cmdline to the empty string.

Update cmdline_cook() to take and return const pointers, and write a
description of what it does.  It now requires a non-NULL input and guarentees
to return a pointer somewhere in the 'p' string.

Note this only compiles because strstr() launders the const off the pointer
when assigning to the mutable kextra, but that logic only mutates the
mbi->cmdline buffer.

Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: Jan Beulich <[e-mail]>
CC: Roger Pau Monné <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Stefano Stabellini <[e-mail]>
CC: Julien Grall <[e-mail]>
CC: Volodymyr Babchuk <[e-mail]>
CC: Bertrand Marquis <[e-mail]>
CC: Roberto Bagnara <[e-mail]>
CC: Nicola Vetrini <[e-mail]>

v2:
 * create_dom0() changes split out into a previous patch
 * Remove the logic for a NULL input, now that all callers have been adjusted
---
 xen/arch/x86/setup.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

Comments

Jan Beulich Nov. 22, 2023, 9:06 a.m. UTC | #1

On 21.11.2023 21:15, Andrew Cooper wrote:
> Rework the logic in __start_xen() to not potentially pass NULL into
> cmdline_cook().  This makes the logic easier to follow too, and the rest of
> __start_xen() is safe when initialising cmdline to the empty string.
> 
> Update cmdline_cook() to take and return const pointers, and write a
> description of what it does.  It now requires a non-NULL input and guarentees
> to return a pointer somewhere in the 'p' string.
> 
> Note this only compiles because strstr() launders the const off the pointer
> when assigning to the mutable kextra, but that logic only mutates the
> mbi->cmdline buffer.
> 
> Signed-off-by: Andrew Cooper <[e-mail]>

Reviewed-by: Jan Beulich <[e-mail]>

Patch

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index c0302c6bdd62..b171a8f4cf84 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -837,10 +837,15 @@  static bool __init loader_is_grub2(const char *loader_name)
     return (p != NULL) && (p[5] != '0');
 }
 
-static char * __init cmdline_cook(char *p, const char *loader_name)
+/*
+ * Clean up a command line string passed to us by a bootloader.  Strip leading
+ * whitespace, and optionally strip the first parameter if our divination of
+ * the bootloader suggests that it prepended the image name.
+ *
+ * Always returns a pointer within @p.
+ */
+static const char *__init cmdline_cook(const char *p, const char *loader_name)
 {
-    p = p ? : "";
-
     /* Strip leading whitespace. */
     while ( *p == ' ' )
         p++;
@@ -969,8 +974,8 @@  static struct domain *__init create_dom0(const module_t *image,
 /* SAF-1-safe */
 void __init noreturn __start_xen(unsigned long mbi_p)
 {
-    const char *memmap_type = NULL, *loader;
-    char *cmdline, *kextra;
+    const char *memmap_type = NULL, *loader, *cmdline = "";
+    char *kextra;
     void *bsp_stack;
     struct cpu_info *info = get_cpu_info(), *bsp_info;
     unsigned int initrdidx, num_parked = 0;
@@ -1025,9 +1030,9 @@  void __init noreturn __start_xen(unsigned long mbi_p)
                                            : "unknown";
 
     /* Parse the command-line options. */
-    cmdline = cmdline_cook((mbi->flags & MBI_CMDLINE) ?
-                           __va(mbi->cmdline) : NULL,
-                           loader);
+    if ( mbi->flags & MBI_CMDLINE )
+        cmdline = cmdline_cook(__va(mbi->cmdline), loader);
+
     if ( (kextra = strstr(cmdline, " -- ")) != NULL )
     {
         /*