[PATCH] xen/efi: Fix crash with initial empty EFI options

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

EFI code path split options from EFI LoadOptions fields in 2
pieces, first EFI options, second Xen options.
"get_argv" function is called first to get the number of arguments
in the LoadOptions, second, after allocating enough space, to
fill some "argc"/"argv" variable. However the first parsing could
be different from second as second is able to detect "--" argument
separator. So it was possible that "argc" was bigger that the "argv"
array leading to potential buffer overflows, in particular
a string like "-- a b c" would lead to buffer overflow in "argv"
resulting in crashes.
Using EFI shell is possible to pass any kind of string in
LoadOptions.

Fixes: 201f261e859e ("EFI: move x86 boot/runtime code to common/efi")
Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
---
 xen/common/efi/boot.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 9306dc8953..597252cfc4 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -345,6 +345,7 @@ static unsigned int __init get_argv(unsigned int argc, 
CHAR16 **argv,
                                     VOID *data, UINTN size, UINTN *offset,
                                     CHAR16 **options)
 {
+    CHAR16 **const orig_argv = argv;
     CHAR16 *ptr = (CHAR16 *)(argv + argc + 1), *prev = NULL, *cmdline = NULL;
     bool prev_sep = true;
 
@@ -384,7 +385,7 @@ static unsigned int __init get_argv(unsigned int argc, 
CHAR16 **argv,
                 {
                     cmdline = data + *offset;
                     /* Cater for the image name as first component. */
-                    ++argc;
+                    ++argv;
                 }
             }
         }
@@ -402,7 +403,7 @@ static unsigned int __init get_argv(unsigned int argc, 
CHAR16 **argv,
         {
             if ( cur_sep )
                 ++ptr;
-            else if ( argv )
+            else if ( orig_argv )
             {
                 *ptr = *cmdline;
                 *++ptr = 0;
@@ -410,8 +411,8 @@ static unsigned int __init get_argv(unsigned int argc, 
CHAR16 **argv,
         }
         else if ( !cur_sep )
         {
-            if ( !argv )
-                ++argc;
+            if ( !orig_argv )
+                ++argv;
             else if ( prev && wstrcmp(prev, L"--") == 0 )
             {
                 --argv;
@@ -428,9 +429,9 @@ static unsigned int __init get_argv(unsigned int argc, 
CHAR16 **argv,
         }
         prev_sep = cur_sep;
     }
-    if ( argv )
+    if ( orig_argv )
         *argv = NULL;
-    return argc;
+    return argv - orig_argv;
 }
 
 static EFI_FILE_HANDLE __init get_parent_handle(const EFI_LOADED_IMAGE 
*loaded_image,
@@ -1348,8 +1349,8 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE 
ImageHandle,
                                   (argc + 1) * sizeof(*argv) +
                                       loaded_image->LoadOptionsSize,
                                   (void **)&argv) == EFI_SUCCESS )
-            get_argv(argc, argv, loaded_image->LoadOptions,
-                     loaded_image->LoadOptionsSize, &offset, &options);
+            argc = get_argv(argc, argv, loaded_image->LoadOptions,
+                            loaded_image->LoadOptionsSize, &offset, &options);
         else
             argc = 0;
         for ( i = 1; i < argc; ++i )
-- 
2.43.0