[FREELDR] Parse NT kernel load options in a consistent manner.

- Introduce a set of NtLdrGet(Next)Option(Ex) helpers that allow
  retrieving respectively, the "next" option in an options string,
  and retrieving a given named option in such an options string,
  by correctly parsing that string.

  Valid syntaxes:
      /OPTION1 /OPTION2/OPTION3 OPTION4 /OPTION5(=...) ...

  Options separators are slashes, or whitespace (space, tab), mandatory
  if no slash is used, and otherwise optional.

- Use these functions wherever NT load options are being parsed.

- Simplify the parsing of /DEBUGPORT=... using these functions.

- When parsing the /HAL=... or /KERNEL=... options, only the first
  encountered one is taken into account, any other ones are discarded.

- When parsing the other load options, only their first occurrences are
  taken into account, any other repetitions are discarded.
  * The NOPAE option overrides any previous PAE option.
  * Any NOEXECUTE(=) option should override any previous
    NOEXECUTE=ALWAYSOFF (or equivalently, EXECUTE) option.

boot/freeldr/freeldr/CMakeLists.txt

@@ -251,6 +251,7 @@ list(APPEND FREELDR_BASE_SOURCE
                      ## otherwise we get linking errors with Rtl**Bitmap** APIs.
                      ## Do not happen on MSVC builds however...
     ntldr/inffile.c
+    ntldr/ntldropts.c
     lib/rtl/libsupp.c)
 
 if(ARCH STREQUAL "i386")

boot/freeldr/freeldr/disk/ramdisk.c

@@ -10,6 +10,7 @@
 /* INCLUDES *******************************************************************/
 
 #include <freeldr.h>
+#include "../ntldr/ntldropts.h"
 
 /* GLOBALS ********************************************************************/
 
@@ -241,30 +242,31 @@ RamDiskInitialize(
         /* If we don't have any load options, initialize an empty Ramdisk */
         if (LoadOptions)
         {
-            PCHAR Option;
+            PCSTR Option;
+            ULONG FileNameLength;
 
             /* Ramdisk image file name */
-            Option = strstr(LoadOptions, "/RDPATH=");
+            Option = NtLdrGetOptionEx(LoadOptions, "RDPATH=", &FileNameLength);
-            if (Option)
+            if (Option && (FileNameLength > 7))
             {
-                /* Copy the file name - everything until the next separator */
+                /* Copy the file name */
-                Option += 8;
+                Option += 7; FileNameLength -= 7;
                 RtlStringCbCopyNA(FileName, sizeof(FileName),
-                                  Option, strcspn(Option, " \t") * sizeof(CHAR));
+                                  Option, FileNameLength * sizeof(CHAR));
             }
 
             /* Ramdisk image length */
-            Option = strstr(LoadOptions, "/RDIMAGELENGTH=");
+            Option = NtLdrGetOption(LoadOptions, "RDIMAGELENGTH=");
             if (Option)
             {
-                RamDiskImageLength = _atoi64(Option + 15);
+                RamDiskImageLength = _atoi64(Option + 14);
             }
 
             /* Ramdisk image offset */
-            Option = strstr(LoadOptions, "/RDIMAGEOFFSET=");
+            Option = NtLdrGetOption(LoadOptions, "RDIMAGEOFFSET=");
             if (Option)
             {
-                RamDiskImageOffset = atol(Option + 15);
+                RamDiskImageOffset = atol(Option + 14);
             }
         }
 

boot/freeldr/freeldr/ntldr/ntldropts.c

@@ -0,0 +1,131 @@
+/*
+ * PROJECT:     FreeLoader
+ * LICENSE:     GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+)
+ * PURPOSE:     NT Kernel Load Options Support Functions
+ * COPYRIGHT:   Copyright 2020 Hermes Belusca-Maito
+ */
+
+/* INCLUDES ******************************************************************/
+
+#include <freeldr.h>
+#include "ntldropts.h"
+
+/* FUNCTIONS *****************************************************************/
+
+PCSTR
+NtLdrGetNextOption(
+    IN OUT PCSTR* Options,
+    OUT PULONG OptionLength OPTIONAL)
+{
+    PCSTR NextOption;
+    PCSTR Option = NULL;
+    ULONG Length = 0;
+
+    if (OptionLength)
+        *OptionLength = 0;
+
+    if (!Options || !*Options)
+        return NULL;
+
+    /* Loop over each option */
+    NextOption = *Options;
+    while (*NextOption)
+    {
+        /* Skip possible initial whitespace */
+        NextOption += strspn(NextOption, " \t");
+
+        /* Stop now if we have already found an option.
+         * NextOption points to the next following option. */
+        if (Option)
+            break;
+
+        /*
+         * Check whether a new option starts. Options are delimited
+         * with an option separator '/' or with whitespace.
+         */
+        if (*NextOption == '/')
+            ++NextOption;
+
+        /* Get the actual length of the option until
+         * the next whitespace or option separator. */
+        Length = (ULONG)strcspn(NextOption, " \t/");
+
+        /* Retrieve the option if present and go to the beginning of the next one */
+        if (Length != 0)
+            Option = NextOption;
+
+        /* Restart after the end of the option */
+        NextOption += Length;
+    }
+
+    *Options = NextOption;
+    if (Option && OptionLength)
+        *OptionLength = Length;
+    return Option;
+}
+
+/*
+ * OptionName specifies the option name, without any leading
+ * option separator '/', to search for within the Options.
+ * The search is made case-insensitive.
+ */
+PCSTR
+NtLdrGetOptionExN(
+    IN PCSTR Options,
+    IN PCCH OptionName,
+    IN ULONG OptNameLength,
+    OUT PULONG OptionLength OPTIONAL)
+{
+    PCSTR NextOptions;
+    PCSTR Option = NULL;
+    ULONG OptLength = 0;
+
+    if (OptionLength)
+        *OptionLength = 0;
+
+    if (!Options || !*Options)
+        return NULL;
+    if (!OptionName || (OptNameLength == 0) || !*OptionName)
+        return NULL;
+
+    NextOptions = Options;
+    while ((Option = NtLdrGetNextOption(&NextOptions, &OptLength)))
+    {
+        /*
+         * Check whether the option to find exactly matches the current
+         * load option, or is a prefix thereof if this is an option with
+         * appended data.
+         */
+        if ((OptLength >= OptNameLength) &&
+            (_strnicmp(Option, OptionName, OptNameLength) == 0))
+        {
+            if ((OptLength == OptNameLength) ||
+                (OptionName[OptNameLength-1] == '=') ||
+                (OptionName[OptNameLength-1] == ':'))
+            {
+                break;
+            }
+        }
+    }
+
+    if (Option && OptionLength)
+        *OptionLength = OptLength;
+    return Option;
+}
+
+PCSTR
+NtLdrGetOptionEx(
+    IN PCSTR Options,
+    IN PCSTR OptionName,
+    OUT PULONG OptionLength OPTIONAL)
+{
+    return NtLdrGetOptionExN(Options, OptionName, strlen(OptionName), OptionLength);
+}
+
+PCSTR
+NtLdrGetOption(
+    IN PCSTR Options,
+    IN PCSTR OptionName)
+{
+    return NtLdrGetOptionEx(Options, OptionName, NULL);
+}

boot/freeldr/freeldr/ntldr/ntldropts.h

@@ -0,0 +1,31 @@
+/*
+ * PROJECT:     FreeLoader
+ * LICENSE:     GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+)
+ * PURPOSE:     NT Kernel Load Options Support Functions
+ * COPYRIGHT:   Copyright 2020 Hermes Belusca-Maito
+ */
+
+#pragma once
+
+PCSTR
+NtLdrGetNextOption(
+    IN OUT PCSTR* Options,
+    OUT PULONG OptionLength OPTIONAL);
+
+PCSTR
+NtLdrGetOptionExN(
+    IN PCSTR Options,
+    IN PCCH OptionName,
+    IN ULONG OptNameLength,
+    OUT PULONG OptionLength OPTIONAL);
+
+PCSTR
+NtLdrGetOptionEx(
+    IN PCSTR Options,
+    IN PCSTR OptionName,
+    OUT PULONG OptionLength OPTIONAL);
+
+PCSTR
+NtLdrGetOption(
+    IN PCSTR Options,
+    IN PCSTR OptionName);

boot/freeldr/freeldr/ntldr/setupldr.c

@@ -10,6 +10,7 @@
 #include <arc/setupblk.h>
 #include "winldr.h"
 #include "inffile.h"
+#include "ntldropts.h"
 
 #include <debug.h>
 DBG_DEFAULT_CHANNEL(WINDOWS);
@@ -185,6 +186,7 @@ LoadReactOSSetup(
     PCSTR SystemPartition;
     PCSTR SystemPath;
     PSTR FileName;
+    ULONG FileNameLength;
     BOOLEAN BootFromFloppy;
     BOOLEAN Success;
     HINF InfHandle;
@@ -294,16 +296,16 @@ LoadReactOSSetup(
     TRACE("BootOptions: '%s'\n", BootOptions2);
 
     /* Check if a RAM disk file was given */
-    FileName = strstr(BootOptions2, "/RDPATH=");
+    FileName = (PSTR)NtLdrGetOptionEx(BootOptions2, "RDPATH=", &FileNameLength);
-    if (FileName)
+    if (FileName && (FileNameLength > 7))
     {
         /* Load the RAM disk */
         Status = RamDiskInitialize(FALSE, BootOptions2, SystemPartition);
         if (Status != ESUCCESS)
         {
-            FileName += 8;
+            FileName += 7; FileNameLength -= 7;
             UiMessageBox("Failed to load RAM disk file '%.*s'",
-                         strcspn(FileName, " \t"), FileName);
+                         FileNameLength, FileName);
             return Status;
         }
     }
@@ -321,7 +323,8 @@ LoadReactOSSetup(
             UiMessageBox("Failed to open txtsetup.sif");
             return ENOENT;
         }
-        RtlStringCbCopyA(FileName, sizeof(BootPath) - (FileName - BootPath)*sizeof(CHAR), SystemPath);
+        FileNameLength = (ULONG)(sizeof(BootPath) - (FileName - BootPath)*sizeof(CHAR));
+        RtlStringCbCopyA(FileName, FileNameLength, SystemPath);
         RtlStringCbCopyA(FilePath, sizeof(FilePath), BootPath);
         RtlStringCbCatA(FilePath, sizeof(FilePath), "txtsetup.sif");
         if (InfOpenFile(&InfHandle, FilePath, &ErrorLine))

boot/freeldr/freeldr/ntldr/winldr.c

@@ -8,6 +8,7 @@
 #include <freeldr.h>
 #include <ndk/ldrtypes.h>
 #include "winldr.h"
+#include "ntldropts.h"
 #include "registry.h"
 
 #include <debug.h>
@@ -497,12 +498,13 @@ LoadWindowsCore(IN USHORT OperatingSystemVersion,
                 IN OUT PLDR_DATA_TABLE_ENTRY* KernelDTE)
 {
     BOOLEAN Success;
-    PCSTR Options;
+    PCSTR Option;
+    ULONG OptionLength;
+    PLDR_DATA_TABLE_ENTRY HalDTE, KdComDTE = NULL;
     CHAR DirPath[MAX_PATH];
     CHAR HalFileName[MAX_PATH];
     CHAR KernelFileName[MAX_PATH];
     CHAR KdTransportDllName[MAX_PATH];
-    PLDR_DATA_TABLE_ENTRY HalDTE, KdComDTE = NULL;
 
     if (!KernelDTE) return FALSE;
 
@@ -520,46 +522,25 @@ LoadWindowsCore(IN USHORT OperatingSystemVersion,
     RtlStringCbCopyA(HalFileName   , sizeof(HalFileName)   , "hal.dll");
     RtlStringCbCopyA(KernelFileName, sizeof(KernelFileName), "ntoskrnl.exe");
 
-    /* Find any "/HAL=" or "/KERNEL=" switch in the boot options */
+    /* Check for any "/HAL=" or "/KERNEL=" override option */
-    Options = BootOptions;
-    while (Options)
-    {
-        /* Skip possible initial whitespace */
-        Options += strspn(Options, " \t");
 
-        /* Check whether a new option starts and it is either HAL or KERNEL */
+    Option = NtLdrGetOptionEx(BootOptions, "HAL=", &OptionLength);
-        if (*Options != '/' || (++Options,
+    if (Option && (OptionLength > 4))
-            !(_strnicmp(Options, "HAL=",    4) == 0 ||
-              _strnicmp(Options, "KERNEL=", 7) == 0)) )
     {
-            /* Search for another whitespace */
+        /* Retrieve the HAL file name */
-            Options = strpbrk(Options, " \t");
+        Option += 4; OptionLength -= 4;
-            continue;
+        RtlStringCbCopyNA(HalFileName, sizeof(HalFileName), Option, OptionLength);
-        }
-        else
-        {
-            size_t i = strcspn(Options, " \t"); /* Skip whitespace */
-            if (i == 0)
-            {
-                /* Use the default values */
-                break;
-            }
-
-            /* We have found either HAL or KERNEL options */
-            if (_strnicmp(Options, "HAL=", 4) == 0)
-            {
-                Options += 4; i -= 4;
-                RtlStringCbCopyNA(HalFileName, sizeof(HalFileName), Options, i);
         _strupr(HalFileName);
     }
-            else if (_strnicmp(Options, "KERNEL=", 7) == 0)
+
+    Option = NtLdrGetOptionEx(BootOptions, "KERNEL=", &OptionLength);
+    if (Option && (OptionLength > 7))
     {
-                Options += 7; i -= 7;
+        /* Retrieve the KERNEL file name */
-                RtlStringCbCopyNA(KernelFileName, sizeof(KernelFileName), Options, i);
+        Option += 7; OptionLength -= 7;
+        RtlStringCbCopyNA(KernelFileName, sizeof(KernelFileName), Option, OptionLength);
         _strupr(KernelFileName);
     }
-        }
-    }
 
     TRACE("HAL file = '%s' ; Kernel file = '%s'\n", HalFileName, KernelFileName);
 
@@ -583,65 +564,36 @@ LoadWindowsCore(IN USHORT OperatingSystemVersion,
          * the name "kdcom.dll". [...]"
          */
 
-        /*
+        /* Check whether there is a DEBUGPORT option */
-         * This loop replaces a dumb call to strstr(..., "DEBUGPORT=").
+        Option = NtLdrGetOptionEx(BootOptions, "DEBUGPORT=", &OptionLength);
-         * Indeed I want it to be case-insensitive to allow "debugport="
+        if (Option && (OptionLength > 10))
-         * or "DeBuGpOrT=" or... , and I don't want it to match malformed
-         * command-line options, such as:
-         *
-         * "...foo DEBUGPORT=xxx bar..."
-         * "...foo/DEBUGPORT=xxx bar..."
-         * "...foo/DEBUGPORT=bar..."
-         *
-         * i.e. the "DEBUGPORT=" switch must start with a slash and be separated
-         * from the rest by whitespace, unless it begins the command-line, e.g.:
-         *
-         * "/DEBUGPORT=COM1 foo...bar..."
-         * "...foo /DEBUGPORT=USB bar..."
-         * or:
-         * "...foo /DEBUGPORT= bar..."
-         * (in that case, we default the port to COM).
-         */
-        Options = BootOptions;
-        while (Options)
         {
-            /* Skip possible initial whitespace */
+            /* Move to the debug port name */
-            Options += strspn(Options, " \t");
+            Option += 10; OptionLength -= 10;
+            ASSERT(OptionLength > 0);
 
-            /* Check whether a new option starts and it is the DEBUGPORT one */
-            if (*Options != '/' || _strnicmp(++Options, "DEBUGPORT=", 10) != 0)
-            {
-                /* Search for another whitespace */
-                Options = strpbrk(Options, " \t");
-                continue;
-            }
-            else
-            {
-                /* We found the DEBUGPORT option. Move to the port name. */
-                Options += 10;
-                break;
-            }
-        }
-
-        if (Options)
-        {
             /*
-             * We have found the DEBUGPORT option. Parse the port name.
+             * Parse the port name.
-             * Format: /DEBUGPORT=COM1 or /DEBUGPORT=FILE:\Device\HarddiskX\PartitionY\debug.log or /DEBUGPORT=FOO
+             * Format: /DEBUGPORT=COM[1-9]
-             * If we only have /DEBUGPORT= (i.e. without any port name), defaults it to "COM".
+             * or: /DEBUGPORT=FILE:\Device\HarddiskX\PartitionY\debug.log
+             * or: /DEBUGPORT=FOO
+             * If we only have /DEBUGPORT= (i.e. without any port name),
+             * defaults it to "COM".
              */
             RtlStringCbCopyA(KdTransportDllName, sizeof(KdTransportDllName), "KD");
-            if (_strnicmp(Options, "COM", 3) == 0 && '0' <= Options[3] && Options[3] <= '9')
+            if (_strnicmp(Option, "COM", 3) == 0 && '0' <= Option[3] && Option[3] <= '9')
             {
-                RtlStringCbCatNA(KdTransportDllName, sizeof(KdTransportDllName), Options, 3);
+                RtlStringCbCatNA(KdTransportDllName, sizeof(KdTransportDllName), Option, 3);
             }
             else
             {
-                size_t i = strcspn(Options, " \t:"); /* Skip valid separators: whitespace or colon */
+                /* Get the actual length of the debug port
-                if (i == 0)
+                 * until the next whitespace or colon. */
+                OptionLength = (ULONG)strcspn(Option, " \t:");
+                if (OptionLength == 0)
                     RtlStringCbCatA(KdTransportDllName, sizeof(KdTransportDllName), "COM");
                 else
-                    RtlStringCbCatNA(KdTransportDllName, sizeof(KdTransportDllName), Options, i);
+                    RtlStringCbCatNA(KdTransportDllName, sizeof(KdTransportDllName), Option, OptionLength);
             }
             RtlStringCbCatA(KdTransportDllName, sizeof(KdTransportDllName), ".DLL");
             _strupr(KdTransportDllName);
@@ -655,88 +607,75 @@ LoadWindowsCore(IN USHORT OperatingSystemVersion,
     }
 
     /* Parse the boot options */
-    Options = BootOptions;
     TRACE("LoadWindowsCore: BootOptions '%s'\n", BootOptions);
-    while (Options)
-    {
-        /* Skip possible initial whitespace */
-        Options += strspn(Options, " \t");
 
-        /* Check whether a new option starts */
+    if (NtLdrGetOption(BootOptions, "3GB"))
-        if (*Options == '/')
-        {
-            Options++;
-
-            if (_strnicmp(Options, "3GB", 3) == 0)
     {
         /* We found the 3GB option. */
         FIXME("LoadWindowsCore: 3GB - TRUE (not implemented)\n");
         VirtualBias = TRUE;
     }
-            if (_strnicmp(Options, "SOS", 3) == 0)
+    if (NtLdrGetOption(BootOptions, "SOS"))
     {
         /* We found the SOS option. */
         FIXME("LoadWindowsCore: SOS - TRUE (not implemented)\n");
         SosEnabled = TRUE;
     }
+
     if (OperatingSystemVersion > _WIN32_WINNT_NT4)
     {
-                if (_strnicmp(Options, "SAFEBOOT", 8) == 0)
+        if (NtLdrGetOption(BootOptions, "SAFEBOOT"))
         {
             /* We found the SAFEBOOT option. */
             FIXME("LoadWindowsCore: SAFEBOOT - TRUE (not implemented)\n");
             SafeBoot = TRUE;
         }
-                if (_strnicmp(Options, "PAE", 3) == 0)
+        if (NtLdrGetOption(BootOptions, "PAE"))
         {
             /* We found the PAE option. */
             FIXME("LoadWindowsCore: PAE - TRUE (not implemented)\n");
             PaeEnabled = TRUE;
         }
     }
+
     if (OperatingSystemVersion > _WIN32_WINNT_WIN2K)
     {
-                if (_strnicmp(Options, "NOPAE", 5) == 0)
+        if (NtLdrGetOption(BootOptions, "NOPAE"))
         {
             /* We found the NOPAE option. */
             FIXME("LoadWindowsCore: NOPAE - TRUE (not implemented)\n");
             PaeDisabled = TRUE;
         }
-                if (_strnicmp(Options, "BOOTLOGO", 8) == 0)
+        if (NtLdrGetOption(BootOptions, "BOOTLOGO"))
         {
             /* We found the BOOTLOGO option. */
             FIXME("LoadWindowsCore: BOOTLOGO - TRUE (not implemented)\n");
             BootLogo = TRUE;
         }
+
         if (!LoaderBlock->SetupLdrBlock)
         {
-                    if (_strnicmp(Options, "NOEXECUTE=ALWAYSOFF", 19) == 0)
+            if (NtLdrGetOption(BootOptions, "EXECUTE"))
-                    {
-                        /* We found the NOEXECUTE=ALWAYSOFF option. */
-                        FIXME("LoadWindowsCore: NOEXECUTE=ALWAYSOFF - TRUE (not implemented)\n");
-                        NoexecuteDisabled = TRUE;
-                    }
-                    else if (_strnicmp(Options, "NOEXECUTE", 9) == 0)
-                    {
-                        /* We found the NOEXECUTE option. */
-                        FIXME("LoadWindowsCore: NOEXECUTE - TRUE (not implemented)\n");
-                        NoexecuteEnabled = TRUE;
-                    }
-
-                    if (_strnicmp(Options, "EXECUTE", 7) == 0)
             {
                 /* We found the EXECUTE option. */
                 FIXME("LoadWindowsCore: EXECUTE - TRUE (not implemented)\n");
                 NoexecuteDisabled = TRUE;
             }
+            if (NtLdrGetOption(BootOptions, "NOEXECUTE=ALWAYSOFF"))
+            {
+                /* We found the NOEXECUTE=ALWAYSOFF option. */
+                FIXME("LoadWindowsCore: NOEXECUTE=ALWAYSOFF - TRUE (not implemented)\n");
+                NoexecuteDisabled = TRUE;
+            }
+            if (NtLdrGetOption(BootOptions, "NOEXECUTE"))
+            {
+                /* We found the NOEXECUTE option. */
+                FIXME("LoadWindowsCore: NOEXECUTE - TRUE (not implemented)\n");
+                NoexecuteEnabled = TRUE;
             }
         }
     }
 
-        /* Search for another whitespace */
-        Options = strpbrk(Options, " \t");
-    }
-
     if (SafeBoot)
     {
         PaeDisabled = TRUE;
@@ -826,6 +765,7 @@ LoadAndBootWindows(
     PCSTR ArgValue;
     PCSTR SystemPartition;
     PCSTR FileName;
+    ULONG FileNameLength;
     BOOLEAN Success;
     USHORT OperatingSystemVersion;
     PLOADER_PARAMETER_BLOCK LoaderBlock;
@@ -912,12 +852,12 @@ LoadAndBootWindows(
     AppendBootTimeOptions(BootOptions);
 
     /*
-     * Set "/HAL=" and "/KERNEL=" options if needed.
+     * Set the "/HAL=" and "/KERNEL=" options if needed.
      * If already present on the standard "Options=" option line, they take
      * precedence over those passed via the separate "Hal=" and "Kernel="
      * options.
      */
-    if (strstr(BootOptions, "/HAL=") != 0)
+    if (!NtLdrGetOption(BootOptions, "HAL="))
     {
         /*
          * Not found in the options, try to retrieve the
@@ -930,7 +870,7 @@ LoadAndBootWindows(
             RtlStringCbCatA(BootOptions, sizeof(BootOptions), ArgValue);
         }
     }
-    if (strstr(BootOptions, "/KERNEL=") != 0)
+    if (!NtLdrGetOption(BootOptions, "KERNEL="))
     {
         /*
          * Not found in the options, try to retrieve the
@@ -947,16 +887,16 @@ LoadAndBootWindows(
     TRACE("BootOptions: '%s'\n", BootOptions);
 
     /* Check if a RAM disk file was given */
-    FileName = strstr(BootOptions, "/RDPATH=");
+    FileName = NtLdrGetOptionEx(BootOptions, "RDPATH=", &FileNameLength);
-    if (FileName)
+    if (FileName && (FileNameLength > 7))
     {
         /* Load the RAM disk */
         Status = RamDiskInitialize(FALSE, BootOptions, SystemPartition);
         if (Status != ESUCCESS)
         {
-            FileName += 8;
+            FileName += 7; FileNameLength -= 7;
             UiMessageBox("Failed to load RAM disk file '%.*s'",
-                         strcspn(FileName, " \t"), FileName);
+                         FileNameLength, FileName);
             return Status;
         }
     }