[Xen-devel] [PATCHv2] xen: Add EFI_LOAD_OPTION support

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

When booting Xen via UEFI the Xen config file can contain multiple sections
each describing different boot options. It is currently only possible to choose
which section to boot with if the buffer contains a string. UEFI provides a
different standard to pass optional arguments to an application, and in this
patch we make Xen properly parse this buffer, thus making it possible to have
separate EFI boot options present for the different config sections.

Signed-off-by: Tamas K Lengyel <lengyelt@xxxxxxxxxxxx>
---
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: openxt@xxxxxxxxxxxxxxxx

v2: move EFI_LOAD_OPTION definition into file that uses it
    add more sanity checks to validate the buffer
---
 xen/common/efi/boot.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 56 insertions(+), 6 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 469bf980cc..8f4fafa3da 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -88,6 +88,16 @@ typedef struct _EFI_APPLE_PROPERTIES {
     EFI_APPLE_PROPERTIES_GETALL GetAll;
 } EFI_APPLE_PROPERTIES;
 
+typedef struct _EFI_LOAD_OPTION {
+    UINT32 Attributes;
+    UINT16 FilePathListLength;
+    CHAR16 Description[];
+} EFI_LOAD_OPTION;
+
+#define LOAD_OPTION_ACTIVE              0x00000001
+#define LOAD_OPTION_FORCE_RECONNECT     0x00000002
+#define LOAD_OPTION_HIDDEN              0x00000008
+
 union string {
     CHAR16 *w;
     char *s;
@@ -375,12 +385,52 @@ static void __init PrintErrMesg(const CHAR16 *mesg, 
EFI_STATUS ErrCode)
 
 static unsigned int __init get_argv(unsigned int argc, CHAR16 **argv,
                                     CHAR16 *cmdline, UINTN cmdsize,
-                                    CHAR16 **options)
+                                    CHAR16 **options, bool *elo_active)
 {
     CHAR16 *ptr = (CHAR16 *)(argv + argc + 1), *prev = NULL;
     bool prev_sep = true;
 
-    for ( ; cmdsize > sizeof(*cmdline) && *cmdline;
+    if ( cmdsize > sizeof(EFI_LOAD_OPTION) )
+    {
+        /* See include/efi/efiapi.h for more info about the following */
+        const EFI_LOAD_OPTION *elo = (const EFI_LOAD_OPTION *)cmdline;
+
+        /* The absolute minimum the size of the buffer it needs to be */
+        size_t size_check = sizeof(elo->Attributes) +
+                            sizeof(elo->FilePathListLength) +
+                            elo->FilePathListLength +
+                            sizeof(CHAR16);
+
+        if ( (elo->Attributes & LOAD_OPTION_ACTIVE) && size_check < cmdsize )
+        {
+            const CHAR16 *desc = elo->Description;
+            const UINT8 *opts = (const UINT8 *)desc;
+            size_t i = 0;
+
+            /* Find Description string length in its possible space */
+            while ( i < cmdsize - size_check && *desc++ != L'\0')
+                i += sizeof(CHAR16);
+
+            /* The Description has to end with a NULL char */
+            if ( *desc == L'\0' )
+            {
+                UINTN new_cmdsize = cmdsize;
+
+                opts += i + sizeof(CHAR16) + elo->FilePathListLength;
+                new_cmdsize -= opts - (UINT8 *)elo;
+
+                /* Sanity check the new cmdsize to avoid an underflow */
+                if ( new_cmdsize < cmdsize )
+                {
+                    *elo_active = true;
+                    cmdline = (CHAR16 *)opts;
+                    cmdsize = new_cmdsize;
+                }
+            }
+        }
+    }
+
+    for ( ; cmdsize >= sizeof(*cmdline) && *cmdline;
             cmdsize -= sizeof(*cmdline), ++cmdline )
     {
         bool cur_sep = *cmdline == L' ' || *cmdline == L'\t';
@@ -1071,7 +1121,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
*SystemTable)
     EFI_SHIM_LOCK_PROTOCOL *shim_lock;
     EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
     union string section = { NULL }, name;
-    bool base_video = false;
+    bool base_video = false, elo_active = false;
     char *option_str;
     bool use_cfg_file;
 
@@ -1096,17 +1146,17 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
*SystemTable)
     if ( use_cfg_file )
     {
         argc = get_argv(0, NULL, loaded_image->LoadOptions,
-                        loaded_image->LoadOptionsSize, NULL);
+                        loaded_image->LoadOptionsSize, NULL, &elo_active);
         if ( argc > 0 &&
              efi_bs->AllocatePool(EfiLoaderData,
                                   (argc + 1) * sizeof(*argv) +
                                       loaded_image->LoadOptionsSize,
                                   (void **)&argv) == EFI_SUCCESS )
             get_argv(argc, argv, loaded_image->LoadOptions,
-                     loaded_image->LoadOptionsSize, &options);
+                     loaded_image->LoadOptionsSize, &options, &elo_active);
         else
             argc = 0;
-        for ( i = 1; i < argc; ++i )
+        for ( i = !elo_active; i < argc; ++i )
         {
             CHAR16 *ptr = argv[i];
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel