OpenLinuxBoot: Fix booting with TuneD in Fedora 41

In GRUB2+blscfg mode: - Allow grub vars in 'initrd' as well as in 'options' - Allow multiple initrds on one line In GRUB2+blscfg mode (seem to be allowed now, not required for fix): - Allow multiple 'initrd' lines - Allow multiple 'options' lines Add variant of OcParseVars which can parse as value-only tokens. Signed-off-by: Mike Beaton <[email protected]>
acidanthera · Nov 16, 2024 · 16b300e · 16b300e
1 parent 6fb63d4
commit 16b300e
Show file tree

Hide file tree

Showing 10 changed files with 254 additions and 82 deletions.
diff --git a/Docs/Configuration.tex b/Docs/Configuration.tex
@@ -6920,7 +6920,8 @@ \subsubsection{Configuration}
   \begin{itemize}
   \tightlist
     \item \texttt{LINUX\_BOOT\_ADD\_RW},
-    \item \texttt{LINUX\_BOOT\_LOG\_VERBOSE} and
+    \item \texttt{LINUX\_BOOT\_LOG\_VERBOSE},
+    \item \texttt{LINUX\_BOOT\_LOG\_GRUB\_VARS} and
     \item \texttt{LINUX\_BOOT\_ADD\_DEBUG\_INFO}.
   \end{itemize}
   \medskip
@@ -7001,6 +7002,17 @@ \subsubsection{Configuration}
     partition's unique partition uuid, to each generated entry name. Can help with debugging
     the origin of entries generated by the driver when there are multiple Linux installs on
     one system.
+    \item \texttt{0x00010000} (bit \texttt{16}) --- \texttt{LINUX\_BOOT\_LOG\_GRUB\_VARS},
+    When a \texttt{BootLoaderSpecByDefault} setup is detected, log available GRUB variables
+    found in \texttt{grub2/grubenv} and \texttt{grub2/grub.cfg}.
+    \item \texttt{0x00020000} (bit \texttt{17}) --- \texttt{LINUX\_BOOT\_FIX\_TUNED},
+    In some circumstances, such as after upgrades which add TuneD to existing systems, the TuneD
+    system tuning plugin may add its GRUB variables to \texttt{loader/entries/*.conf} files but not
+    initialise them in \texttt{grub2/grub.cfg}. In order to avoid incorrect boots, OpenLinuxBoot
+    treats used non-initialised GRUB variables as an error. When this flag is set, empty values
+    are added for the TuneD variables \texttt{tuned\_params} and \texttt{tuned\_initrd} if they
+    are not present. This is required for OpenLinuxBoot on TuneD systems with this problem, and
+    harmless otherwise.
   \end{itemize} \medskip
 
   Flag values can be specified in hexadecimal beginning with \texttt{0x} or in decimal,
@@ -7048,7 +7060,7 @@ \subsubsection{Additional information}
 
 OpenLinuxBoot can detect the \texttt{loader/entries/*.conf} files created according to the
 \href{https://systemd.io/BOOT_LOADER_SPECIFICATION/}{Boot Loader Specification} or the closely related
-\href{https://fedoraproject.org/wiki/Changes/BootLoaderSpecByDefault}{systemd BootLoaderSpecByDefault}. The
+\href{https://fedoraproject.org/wiki/Changes/BootLoaderSpecByDefault}{Fedora BootLoaderSpecByDefault}. The
 former is specific to systemd-boot and is used by Arch Linux, the latter applies to most Fedora-related distros
 including Fedora itself, RHEL and variants.
 
@@ -7064,13 +7076,13 @@ \subsubsection{Additional information}
 \texttt{autoopts:\{partuuid\}=...} (\texttt{+=} variants of these options will not work, as these only add additional
 arguments).
 
-BootLoaderSpecByDefault (but not pure Boot Loader Specification) can expand GRUB variables
+Fedora \texttt{BootLoaderSpecByDefault} (but not pure Boot Loader Specification) can expand GRUB variables
 in the \texttt{*.conf} files -- and this is used in practice in certain distros such as CentOS.
 In order to handle this correctly, when this situation is detected OpenLinuxBoot extracts all variables from
 \texttt{\{boot\}/grub2/grubenv} and also any unconditionally set variables from
 \texttt{\{boot\}/grub2/grub.cfg}, and then expands these where required in \texttt{*.conf} file entries.
 
-The only currently supported method of starting Linux kernels relies on their being compiled with EFISTUB.
+The only currently supported method of starting Linux kernels from OpenLinuxBoot relies on their being compiled with EFISTUB.
 This applies to almost all modern distros, particularly those which use systemd. Note that most modern
 distros use systemd as their system manager, even though most do not use systemd-boot as
 their bootloader.

diff --git a/Include/Acidanthera/Library/OcBootManagementLib.h b/Include/Acidanthera/Library/OcBootManagementLib.h
@@ -1926,7 +1926,7 @@ OcParseLoadOptions (
 
 /**
   Parse Unix-style var file or string. Parses a couple of useful ASCII
-  GRUB config files (multi-line, name=var, with optinal comments) and
+  GRUB config files (multi-line, name=var, with optional comments) and
   defines a standard format for Unicode UEFI LoadOptions.
 
   Assumes CHAR_NULL terminated Unicode string of space separated options,
@@ -1940,9 +1940,10 @@ OcParseLoadOptions (
 
   @param[in]   StrVars            Raw var string.
   @param[out]  ParsedVars         Parsed variables if successful, NULL otherwise.
-                                  Caller may free after use with OcFlexArrayFree
-                                  if required.
+                                  Caller may free after use with OcFlexArrayFree.
   @param[in]   StringFormat       Are option names and values Unicode or ASCII?
+  @param[in]   TokensOnly         If TRUE parse as a sequence of token values only,
+                                  rather than as a sequence of name[=[value]] pairs.
 
   @retval EFI_SUCCESS             Success.
   @retval EFI_NOT_FOUND           Missing or empty load options.
@@ -1953,7 +1954,8 @@ EFI_STATUS
 OcParseVars (
   IN           VOID              *StrVars,
   OUT       OC_FLEX_ARRAY        **ParsedVars,
-  IN     CONST OC_STRING_FORMAT  StringFormat
+  IN     CONST OC_STRING_FORMAT  StringFormat,
+  IN     CONST BOOLEAN           TokensOnly
   );
 
 /**

diff --git a/Include/Acidanthera/Library/OcStringLib.h b/Include/Acidanthera/Library/OcStringLib.h
@@ -742,4 +742,16 @@ OcIsSpace (
   CHAR16  Ch
   );
 
+/**
+  Determine if a particular character is whitespace or CHAR_NULL.
+
+  @param[in] Ch               The character to check.
+
+  @return  Returns TRUE if Ch is a whitespace character or CHAR_NULL.
+**/
+BOOLEAN
+OcIsSpaceOrNull (
+  CHAR16  Ch
+  );
+
 #endif // OC_STRING_LIB_H
diff --git a/Library/OcBootManagementLib/BootArguments.c b/Library/OcBootManagementLib/BootArguments.c
@@ -1,5 +1,5 @@
 /** @file
-  Copyright (C) 2019-2021, vit9696, mikebeaton. All rights reserved.<BR>
+  Copyright (C) 2019-2024, vit9696, mikebeaton. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-3-Clause
 **/
 
@@ -24,19 +24,19 @@ typedef enum PARSE_VARS_STATE_ {
   PARSE_VARS_WHITE_SPACE,
   PARSE_VARS_COMMENT,
   PARSE_VARS_NAME,
-  PARSE_VARS_VALUE_FIRST,
   PARSE_VARS_VALUE,
   PARSE_VARS_QUOTED_VALUE,
   PARSE_VARS_SHELL_EXPANSION
 } PARSE_VARS_STATE;
 
 //
-// Shift from token start to current position forwards by offset characters.
+// Shift memory from token start to current position forwards by offset bytes
+// and update token to point to shifted start (thereby discarding offset bytes
+// fromec the end of the token).
 //
 #define SHIFT_TOKEN(pos, token, offset)  do {\
   CopyMem ((UINT8 *)(token) + (offset), (token), (UINT8 *)(pos) - (UINT8 *)(token)); \
   (token) = (UINT8 *)(token) + (offset); \
-  (pos)   = (UINT8 *)(pos) + (offset); \
 } while (0)
 
 VOID
@@ -434,7 +434,7 @@ OcParseLoadOptions (
     return EFI_NOT_FOUND;
   }
 
-  Status = OcParseVars (LoadedImage->LoadOptions, ParsedVars, OcStringFormatUnicode);
+  Status = OcParseVars (LoadedImage->LoadOptions, ParsedVars, OcStringFormatUnicode, FALSE);
 
   if (Status == EFI_INVALID_PARAMETER) {
     DEBUG ((DEBUG_ERROR, "OCB: Failed to parse LoadOptions (%p:%u)\n", LoadedImage->LoadOptions, LoadedImage->LoadOptionsSize));
@@ -449,16 +449,20 @@ EFI_STATUS
 OcParseVars (
   IN           VOID              *StrVars,
   OUT       OC_FLEX_ARRAY        **ParsedVars,
-  IN     CONST OC_STRING_FORMAT  StringFormat
+  IN     CONST OC_STRING_FORMAT  StringFormat,
+  IN     CONST BOOLEAN           TokensOnly
   )
 {
   VOID              *Pos;
+  VOID              *NewPos;
   PARSE_VARS_STATE  State;
   PARSE_VARS_STATE  PushState;
-  BOOLEAN           Retake;
   CHAR16            Ch;
+  CHAR16            NewCh;
+  CHAR16            QuoteChar;
   VOID              *Name;
   VOID              *Value;
+  VOID              *OriginalValue;
   OC_PARSED_VAR     *Option;
 
   if ((StrVars == NULL) || ((StringFormat == OcStringFormatUnicode) ? (((CHAR16 *)StrVars)[0] == CHAR_NULL) : (((CHAR8 *)StrVars)[0] == '\0'))) {
@@ -474,17 +478,31 @@ OcParseVars (
   Pos       = StrVars;
   State     = PARSE_VARS_WHITE_SPACE;
   PushState = PARSE_VARS_WHITE_SPACE;
-  Retake    = FALSE;
+  QuoteChar = CHAR_NULL;
 
   do {
     Ch = (StringFormat == OcStringFormatUnicode) ? *((CHAR16 *)Pos) : *((CHAR8 *)Pos);
     switch (State) {
       case PARSE_VARS_WHITE_SPACE:
         if (Ch == '#') {
           State = PARSE_VARS_COMMENT;
-        } else if (!(OcIsSpace (Ch) || (Ch == CHAR_NULL))) {
-          Name  = Pos;
-          State = PARSE_VARS_NAME;
+        } else if (!OcIsSpaceOrNull (Ch)) {
+          if (TokensOnly) {
+            Option = OcFlexArrayAddItem (*ParsedVars);
+            if (Option == NULL) {
+              OcFlexArrayFree (ParsedVars);
+              return EFI_OUT_OF_RESOURCES;
+            }
+
+            DEBUG ((OC_TRACE_PARSE_VARS, "OCB: Value-only token\n"));
+
+            Value         = Pos;
+            OriginalValue = Value;
+            State         = PARSE_VARS_VALUE;
+          } else {
+            Name  = Pos;
+            State = PARSE_VARS_NAME;
+          }
         }
 
         break;
@@ -497,15 +515,17 @@ OcParseVars (
         break;
 
       case PARSE_VARS_NAME:
-        if ((Ch == L'=') || OcIsSpace (Ch) || (Ch == CHAR_NULL)) {
+        if ((Ch == L'=') || OcIsSpaceOrNull (Ch)) {
           if (StringFormat == OcStringFormatUnicode) {
             *((CHAR16 *)Pos) = CHAR_NULL;
           } else {
             *((CHAR8 *)Pos) = '\0';
           }
 
           if (Ch == L'=') {
-            State = PARSE_VARS_VALUE_FIRST;
+            State         = PARSE_VARS_VALUE;
+            Value         = (UINT8 *)Pos + ((StringFormat == OcStringFormatUnicode) ? sizeof (CHAR16) : sizeof (CHAR8));
+            OriginalValue = Value;
           } else {
             State = PARSE_VARS_WHITE_SPACE;
           }
@@ -533,18 +553,6 @@ OcParseVars (
 
         break;
 
-      case PARSE_VARS_VALUE_FIRST:
-        if (Ch == L'"') {
-          State = PARSE_VARS_QUOTED_VALUE;
-          Value = (UINT8 *)Pos + ((StringFormat == OcStringFormatUnicode) ? sizeof (CHAR16) : sizeof (CHAR8));
-        } else {
-          State  = PARSE_VARS_VALUE;
-          Value  = Pos;
-          Retake = TRUE;
-        }
-
-        break;
-
       case PARSE_VARS_SHELL_EXPANSION:
         if (Ch == '`') {
           ASSERT (PushState != PARSE_VARS_WHITE_SPACE);
@@ -553,23 +561,39 @@ OcParseVars (
 
         break;
 
+      //
+      // In token value (but not name) we handle sh and grub quoting and string concatenation, e.g. 'abc\'"'\""def becomes abc\'"def.
+      //
       case PARSE_VARS_VALUE:
       case PARSE_VARS_QUOTED_VALUE:
-        if (Ch == L'`') {
+        if ((State != PARSE_VARS_QUOTED_VALUE) && ((Ch == L'\'') || (Ch == L'"'))) {
+          QuoteChar = Ch;
+          SHIFT_TOKEN (Pos, Value, (StringFormat == OcStringFormatUnicode) ? sizeof (CHAR16) : sizeof (CHAR8));
+          State = PARSE_VARS_QUOTED_VALUE;
+        } else if ((State == PARSE_VARS_QUOTED_VALUE) && (Ch == QuoteChar)) {
+          SHIFT_TOKEN (Pos, Value, (StringFormat == OcStringFormatUnicode) ? sizeof (CHAR16) : sizeof (CHAR8));
+          QuoteChar = CHAR_NULL;
+          State = PARSE_VARS_VALUE;
+        } else if (((State != PARSE_VARS_QUOTED_VALUE) || (QuoteChar == L'"')) && (Ch == L'\\')) {
+          NewPos = (UINT8 *)Pos + ((StringFormat == OcStringFormatUnicode) ? sizeof (CHAR16) : sizeof (CHAR8));
+          NewCh = (StringFormat == OcStringFormatUnicode) ? *((CHAR16 *)Pos) : *((CHAR8 *)Pos);
+          // 
+          // https://www.gnu.org/software/bash/manual/html_node/Double-Quotes.html
+          //
+          if ((State != PARSE_VARS_QUOTED_VALUE) || (NewCh == '"') || (NewCh == '\\') || (NewCh == '$') || (NewCh == '`')) {
+            SHIFT_TOKEN (Pos, Value, (StringFormat == OcStringFormatUnicode) ? sizeof (CHAR16) : sizeof (CHAR8));
+            Pos = NewPos;
+            Ch = NewCh;
+          }
+        } else if (Ch == L'`') {
           PushState = State;
           State     = PARSE_VARS_SHELL_EXPANSION;
-        } else if (Ch == L'\\') {
-          SHIFT_TOKEN (Pos, Value, (StringFormat == OcStringFormatUnicode) ? sizeof (CHAR16) : sizeof (CHAR8));
-          Ch = (StringFormat == OcStringFormatUnicode) ? *((CHAR16 *)Pos) : *((CHAR8 *)Pos);
-        } else if (
-                   ((State == PARSE_VARS_VALUE) && (OcIsSpace (Ch) || (Ch == CHAR_NULL))) ||
-                   ((State == PARSE_VARS_QUOTED_VALUE) && (Ch == '"')))
-        {
+        } else if ((State == PARSE_VARS_VALUE) && OcIsSpaceOrNull (Ch)) {
           //
-          // Explicitly quoted empty string needs to be stored detectably
-          // differently from missing value.
+          // Explicitly quoted empty string (e.g. `var=""`) is stored detectably differently from missing value (i.e. `var=`, or just `var`).
           //
-          if ((State != PARSE_VARS_QUOTED_VALUE) && (Pos == Value)) {
+          if (Pos == OriginalValue) {
+            ASSERT (!TokensOnly);
             DEBUG ((OC_TRACE_PARSE_VARS, "OCB: No value %u\n", 2));
           } else {
             if (PushState != PARSE_VARS_WHITE_SPACE) {
@@ -588,9 +612,10 @@ OcParseVars (
             }
           }
 
-          Value  = NULL;
-          Option = NULL;
-          State  = PARSE_VARS_WHITE_SPACE;
+          Value         = NULL;
+          OriginalValue = NULL;
+          Option        = NULL;
+          State         = PARSE_VARS_WHITE_SPACE;
         }
 
         break;
@@ -600,11 +625,7 @@ OcParseVars (
         break;
     }
 
-    if (Retake) {
-      Retake = FALSE;
-    } else {
-      Pos = (UINT8 *)Pos + ((StringFormat == OcStringFormatUnicode) ? sizeof (CHAR16) : sizeof (CHAR8));
-    }
+    Pos = (UINT8 *)Pos + ((StringFormat == OcStringFormatUnicode) ? sizeof (CHAR16) : sizeof (CHAR8));
   } while (Ch != CHAR_NULL);
 
   if ((State != PARSE_VARS_WHITE_SPACE) || (PushState != PARSE_VARS_WHITE_SPACE)) {

diff --git a/Library/OcStringLib/OcUnicodeLib.c b/Library/OcStringLib/OcUnicodeLib.c
@@ -530,3 +530,11 @@ OcIsSpace (
 {
   return (Ch == L' ') || (Ch == L'\t') || (Ch == L'\r') || (Ch == L'\n') || (Ch == L'\v')  || (Ch == L'\f');
 }
+
+BOOLEAN
+OcIsSpaceOrNull (
+  CHAR16  Ch
+  )
+{
+  return (Ch == CHAR_NULL) || OcIsSpace (Ch);
+}
diff --git a/Platform/OpenLinuxBoot/Autodetect.c b/Platform/OpenLinuxBoot/Autodetect.c
@@ -269,7 +269,7 @@ LoadOsRelease (
         "LNX: Reading %s\n",
         OS_RELEASE_FILE
         ));
-      Status = OcParseVars (mEtcOsReleaseFileContents, &mEtcOsReleaseOptions, OcStringFormatAscii);
+      Status = OcParseVars (mEtcOsReleaseFileContents, &mEtcOsReleaseOptions, OcStringFormatAscii, FALSE);
       if (EFI_ERROR (Status)) {
         FreePool (mEtcOsReleaseFileContents);
         mEtcOsReleaseFileContents = NULL;
@@ -320,7 +320,7 @@ LoadDefaultGrub (
       "LNX: Reading %s\n",
       GRUB_DEFAULT_FILE
       ));
-    Status = OcParseVars (mEtcDefaultGrubFileContents, &mEtcDefaultGrubOptions, OcStringFormatAscii);
+    Status = OcParseVars (mEtcDefaultGrubFileContents, &mEtcDefaultGrubOptions, OcStringFormatAscii, FALSE);
     if (EFI_ERROR (Status)) {
       FreePool (mEtcDefaultGrubFileContents);
       mEtcDefaultGrubFileContents = NULL;

diff --git a/Platform/OpenLinuxBoot/GrubVars.c b/Platform/OpenLinuxBoot/GrubVars.c
@@ -82,7 +82,7 @@ InternalSetGrubVar (
     Var->Errors |= Errors;
 
     DEBUG ((
-      OC_TRACE_GRUB_VARS,
+      (gLinuxBootFlags & LINUX_BOOT_LOG_GRUB_VARS) == 0 ? DEBUG_VERBOSE : DEBUG_INFO,
       "LNX: Repeated %a=%a (0x%x->0x%x)\n",
       Key,
       Value,
@@ -100,7 +100,7 @@ InternalSetGrubVar (
     Var->Errors = Errors;
 
     DEBUG ((
-      OC_TRACE_GRUB_VARS,
+      (gLinuxBootFlags & LINUX_BOOT_LOG_GRUB_VARS) == 0 ? DEBUG_VERBOSE : DEBUG_INFO,
       "LNX: Added %a=%a (0x%x)\n",
       Key,
       Value,
@@ -273,7 +273,13 @@ InternalExpandGrubVars (
     }
   }
 
-  DEBUG ((OC_TRACE_GRUB_VARS, "LNX: Expanding '%a' => '%a' - %r\n", Value, *Result, Status));
+  DEBUG ((
+    (gLinuxBootFlags & LINUX_BOOT_LOG_GRUB_VARS) == 0 ? DEBUG_VERBOSE : DEBUG_INFO,
+    "LNX: Expanding '%a' => '%a' - %r\n",
+    Value,
+    *Result,
+    Status
+    ));
 
   return Status;
 }