[PATCH v4 0/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner

[PATCH v4 0/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner

[Glenn Washburn]

Updates since v3:
 * Rebase onto latest master
 * Whitespace fixes

Glenn

Glenn Washburn (2):
  luks, luks2: Force header.uuid to be NULL terminated
  cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner

 grub-core/disk/cryptodisk.c |  4 ++--
 grub-core/disk/geli.c       |  2 +-
 grub-core/disk/luks.c       | 24 +++++++-----------------
 grub-core/disk/luks2.c      | 18 +++++++-----------
 include/grub/misc.h         | 27 +++++++++++++++++++++++++++
 5 files changed, 44 insertions(+), 31 deletions(-)

-- 
2.34.1

[PATCH v4 1/2] luks, luks2: Force header.uuid to be NULL terminated

[Glenn Washburn]

According to the LUKS specification the uuid header field is of data type
"char[]", which is defined as "a string stored as null terminated sequence
of 8-bit characters". So enforce this by adding a null byte as the last byte
of the uuid. The LUKS2 specification defers to the LUKS1 specification in
this regard.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/disk/luks.c  | 3 +++
 grub-core/disk/luks2.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
index 7f837d52c..ad72e5d1f 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -87,6 +87,9 @@ luks_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
       return NULL;
     }
 
+  /* According to the spec the uuid field is NULL terminated, so enforce it. */
+  header.uuid[sizeof (header.uuid) - 1] = '\0';
+
   /* Look for LUKS magic sequence.  */
   if (grub_memcmp (header.magic, LUKS_MAGIC, sizeof (header.magic))
       || grub_be_to_cpu16 (header.version) != 1)
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index bf741d70f..1346c3821 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -342,6 +342,9 @@ luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
       header = &secondary;
   grub_memcpy (outhdr, header, sizeof (*header));
 
+  /* According to the spec the uuid field is NULL terminated, so enforce it. */
+  header->uuid[sizeof (header->uuid) - 1] = '\0';
+
   return GRUB_ERR_NONE;
 }
 
-- 
2.34.1

Re: [PATCH v4 1/2] luks, luks2: Force header.uuid to be NULL terminated

[Daniel Kiper]

On Fri, Jul 22, 2022 at 02:43:13AM -0500, Glenn Washburn wrote:
> According to the LUKS specification the uuid header field is of data type
> "char[]", which is defined as "a string stored as null terminated sequence
> of 8-bit characters". So enforce this by adding a null byte as the last byte
> of the uuid. The LUKS2 specification defers to the LUKS1 specification in
> this regard.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

In general Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...

Though I want to ask you to add a sentence to the commit message saying
why current code works now. And a second sentence should say the part of
existing code making it work will be removed by subsequent patch.
Otherwise it is not really clear why this patch is needed...

Or maybe we should merge patch #1 and #2... However, I do not have strong
opinion here.

Daniel

Re: [PATCH v4 1/2] luks, luks2: Force header.uuid to be NULL terminated

[Glenn Washburn]

On Wed, 3 Aug 2022 15:30:58 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Fri, Jul 22, 2022 at 02:43:13AM -0500, Glenn Washburn wrote:
> > According to the LUKS specification the uuid header field is of data type
> > "char[]", which is defined as "a string stored as null terminated sequence
> > of 8-bit characters". So enforce this by adding a null byte as the last byte
> > of the uuid. The LUKS2 specification defers to the LUKS1 specification in
> > this regard.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> 
> In general Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...
> 
> Though I want to ask you to add a sentence to the commit message saying
> why current code works now. And a second sentence should say the part of
> existing code making it work will be removed by subsequent patch.
> Otherwise it is not really clear why this patch is needed...

Looking at this again, this patch really isn't needed in a functional
sense, with or without the following patch. I think I might have seen
this as desirable at one time because this was written before Patricks
commit 1066336d (luks: Fix out-of-bounds copy of UUID).

Its not needed for the subsequent patch because UUID comparison is
always bounded. I'll drop this change.

Glenn

> 
> Or maybe we should merge patch #1 and #2... However, I do not have strong
> opinion here.
> 
> Daniel

[PATCH v4 2/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner

[Glenn Washburn]

A user can now specify UUID strings with dashes, instead of having to remove
dashes. This is backwards-compatability preserving and also fixes a source
of user confusion over the inconsistency with how UUIDs are specified
between file system UUIDs and cryptomount UUIDs. Since cryptsetup, the
reference implementation for LUKS, displays and generates UUIDs with dashes
there has been additional confusion when using the UUID strings from
cryptsetup as exact input into GRUB does not find the expected cryptodisk.

A new function grub_uuidcasecmp is added that is general enough to be used
other places where UUIDs are being compared.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/disk/cryptodisk.c |  4 ++--
 grub-core/disk/geli.c       |  2 +-
 grub-core/disk/luks.c       | 21 ++++-----------------
 grub-core/disk/luks2.c      | 15 ++++-----------
 include/grub/misc.h         | 27 +++++++++++++++++++++++++++
 5 files changed, 38 insertions(+), 31 deletions(-)

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index f1fe0d390..00d66ab02 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -701,7 +701,7 @@ grub_cryptodisk_open (const char *name, grub_disk_t disk)
   if (grub_memcmp (name, "cryptouuid/", sizeof ("cryptouuid/") - 1) == 0)
     {
       for (dev = cryptodisk_list; dev != NULL; dev = dev->next)
-	if (grub_strcasecmp (name + sizeof ("cryptouuid/") - 1, dev->uuid) == 0)
+	if (grub_uuidcasecmp (name + sizeof ("cryptouuid/") - 1, dev->uuid, sizeof (dev->uuid)) == 0)
 	  break;
     }
   else
@@ -928,7 +928,7 @@ grub_cryptodisk_get_by_uuid (const char *uuid)
 {
   grub_cryptodisk_t dev;
   for (dev = cryptodisk_list; dev != NULL; dev = dev->next)
-    if (grub_strcasecmp (dev->uuid, uuid) == 0)
+    if (grub_uuidcasecmp (dev->uuid, uuid, sizeof (dev->uuid)) == 0)
       return dev;
   return NULL;
 }
diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
index e190066f9..722910d64 100644
--- a/grub-core/disk/geli.c
+++ b/grub-core/disk/geli.c
@@ -305,7 +305,7 @@ geli_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
       return NULL;
     }
 
-  if (cargs->search_uuid != NULL && grub_strcasecmp (cargs->search_uuid, uuid) != 0)
+  if (cargs->search_uuid != NULL && grub_uuidcasecmp (cargs->search_uuid, uuid, sizeof (uuid)) != 0)
     {
       grub_dprintf ("geli", "%s != %s\n", uuid, cargs->search_uuid);
       return NULL;
diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
index ad72e5d1f..3ca08b805 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -66,10 +66,7 @@ static grub_cryptodisk_t
 luks_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
 {
   grub_cryptodisk_t newdev;
-  const char *iptr;
   struct grub_luks_phdr header;
-  char *optr;
-  char uuid[sizeof (header.uuid) + 1];
   char ciphername[sizeof (header.cipherName) + 1];
   char ciphermode[sizeof (header.cipherMode) + 1];
   char hashspec[sizeof (header.hashSpec) + 1];
@@ -95,19 +92,9 @@ luks_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
       || grub_be_to_cpu16 (header.version) != 1)
     return NULL;
 
-  grub_memset (uuid, 0, sizeof (uuid));
-  optr = uuid;
-  for (iptr = header.uuid; iptr < &header.uuid[ARRAY_SIZE (header.uuid)];
-       iptr++)
+  if (cargs->search_uuid != NULL && grub_uuidcasecmp (cargs->search_uuid, header.uuid, sizeof (header.uuid)) != 0)
     {
-      if (*iptr != '-')
-	*optr++ = *iptr;
-    }
-  *optr = 0;
-
-  if (cargs->search_uuid != NULL && grub_strcasecmp (cargs->search_uuid, uuid) != 0)
-    {
-      grub_dprintf ("luks", "%s != %s\n", uuid, cargs->search_uuid);
+      grub_dprintf ("luks", "%s != %s\n", header.uuid, cargs->search_uuid);
       return NULL;
     }
 
@@ -126,7 +113,7 @@ luks_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
   newdev->source_disk = NULL;
   newdev->log_sector_size = GRUB_LUKS1_LOG_SECTOR_SIZE;
   newdev->total_sectors = grub_disk_native_sectors (disk) - newdev->offset_sectors;
-  grub_memcpy (newdev->uuid, uuid, sizeof (uuid));
+  grub_memcpy (newdev->uuid, header.uuid, sizeof (header.uuid));
   newdev->modname = "luks";
 
   /* Configure the hash used for the AF splitter and HMAC.  */
@@ -146,7 +133,7 @@ luks_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
       return NULL;
     }
 
-  COMPILE_TIME_ASSERT (sizeof (newdev->uuid) >= sizeof (uuid));
+  COMPILE_TIME_ASSERT (sizeof (newdev->uuid) >= sizeof (header.uuid));
   return newdev;
 }
 
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index 1346c3821..2fb500c37 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -353,8 +353,6 @@ luks2_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
 {
   grub_cryptodisk_t cryptodisk;
   grub_luks2_header_t header;
-  char uuid[sizeof (header.uuid) + 1];
-  grub_size_t i, j;
 
   if (cargs->check_boot)
     return NULL;
@@ -365,14 +363,9 @@ luks2_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
       return NULL;
     }
 
-  for (i = 0, j = 0; i < sizeof (header.uuid); i++)
-    if (header.uuid[i] != '-')
-      uuid[j++] = header.uuid[i];
-  uuid[j] = '\0';
-
-  if (cargs->search_uuid != NULL && grub_strcasecmp (cargs->search_uuid, uuid) != 0)
+  if (cargs->search_uuid != NULL && grub_uuidcasecmp (cargs->search_uuid, header.uuid, sizeof (header.uuid)) != 0)
     {
-      grub_dprintf ("luks2", "%s != %s\n", uuid, cargs->search_uuid);
+      grub_dprintf ("luks2", "%s != %s\n", header.uuid, cargs->search_uuid);
       return NULL;
     }
 
@@ -380,8 +373,8 @@ luks2_scan (grub_disk_t disk, grub_cryptomount_args_t cargs)
   if (!cryptodisk)
     return NULL;
 
-  COMPILE_TIME_ASSERT (sizeof (cryptodisk->uuid) >= sizeof (uuid));
-  grub_memcpy (cryptodisk->uuid, uuid, sizeof (uuid));
+  COMPILE_TIME_ASSERT (sizeof (cryptodisk->uuid) >= sizeof (header.uuid));
+  grub_memcpy (cryptodisk->uuid, header.uuid, sizeof (header.uuid));
 
   cryptodisk->modname = "luks2";
   return cryptodisk;
diff --git a/include/grub/misc.h b/include/grub/misc.h
index 776dbf8af..39957ecf9 100644
--- a/include/grub/misc.h
+++ b/include/grub/misc.h
@@ -243,6 +243,33 @@ grub_strncasecmp (const char *s1, const char *s2, grub_size_t n)
     - (int) grub_tolower ((grub_uint8_t) *s2);
 }
 
+/* Do a case insensitive compare of two UUID strings by ignoring all dashes */
+static inline int
+grub_uuidcasecmp (const char *uuid1, const char *uuid2, grub_size_t n)
+{
+  if (n == 0)
+    return 0;
+
+  while (*s1 && *s2 && --n)
+    {
+      /* Skip forward to non-dash on both UUIDs. */
+      while ('-' == *s1)
+        ++s1;
+
+      while ('-' == *s2)
+        ++s2;
+
+      if (grub_tolower (*s1) != grub_tolower (*s2))
+	break;
+
+      s1++;
+      s2++;
+    }
+
+  return (int) grub_tolower ((grub_uint8_t) *s1)
+    - (int) grub_tolower ((grub_uint8_t) *s2);
+}
+
 /*
  * Note that these differ from the C standard's definitions of strtol,
  * strtoul(), and strtoull() by the addition of two const qualifiers on the end
-- 
2.34.1

Re: [PATCH v4 2/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner

[Daniel Kiper]

On Fri, Jul 22, 2022 at 02:43:14AM -0500, Glenn Washburn wrote:
> A user can now specify UUID strings with dashes, instead of having to remove
> dashes. This is backwards-compatability preserving and also fixes a source
> of user confusion over the inconsistency with how UUIDs are specified
> between file system UUIDs and cryptomount UUIDs. Since cryptsetup, the
> reference implementation for LUKS, displays and generates UUIDs with dashes
> there has been additional confusion when using the UUID strings from
> cryptsetup as exact input into GRUB does not find the expected cryptodisk.

I expect the confusion comes from differences between older and newer
cryptsetup implementations. AIUI older versions used UUIDs without
dashes, and this is what GRUB expects today, but newer ones use UUIDs
with dashes. Right? If it is true it took me some time to get it from
the text above. So, I think it should be improved a bit to make it more
clear...

> A new function grub_uuidcasecmp is added that is general enough to be used
> other places where UUIDs are being compared.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/disk/cryptodisk.c |  4 ++--
>  grub-core/disk/geli.c       |  2 +-
>  grub-core/disk/luks.c       | 21 ++++-----------------
>  grub-core/disk/luks2.c      | 15 ++++-----------
>  include/grub/misc.h         | 27 +++++++++++++++++++++++++++
>  5 files changed, 38 insertions(+), 31 deletions(-)

[...]

> diff --git a/include/grub/misc.h b/include/grub/misc.h
> index 776dbf8af..39957ecf9 100644
> --- a/include/grub/misc.h
> +++ b/include/grub/misc.h
> @@ -243,6 +243,33 @@ grub_strncasecmp (const char *s1, const char *s2, grub_size_t n)
>      - (int) grub_tolower ((grub_uint8_t) *s2);
>  }
>
> +/* Do a case insensitive compare of two UUID strings by ignoring all dashes */
> +static inline int
> +grub_uuidcasecmp (const char *uuid1, const char *uuid2, grub_size_t n)
> +{
> +  if (n == 0)
> +    return 0;
> +
> +  while (*s1 && *s2 && --n)
> +    {
> +      /* Skip forward to non-dash on both UUIDs. */
> +      while ('-' == *s1)
> +        ++s1;
> +
> +      while ('-' == *s2)
> +        ++s2;
> +
> +      if (grub_tolower (*s1) != grub_tolower (*s2))

I think you took this code from grub_strncasecmp(). It is missing grub_uint8_t
casts here too. However, if you take a look at grub_strcasecmp(), a few lines
above, you can see casts in similar place. It is really confusing. I thing the
casts has been added to drop sign. If I am right then grub_strncasecmp() and
grub_uuidcasecmp() both should be fixed.

> +	break;
> +
> +      s1++;
> +      s2++;
> +    }
> +
> +  return (int) grub_tolower ((grub_uint8_t) *s1)
> +    - (int) grub_tolower ((grub_uint8_t) *s2);

... and these casts suggests again something is wrong above too...

Daniel

Re: [PATCH v4 2/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner

[Glenn Washburn]

On Tue, 2 Aug 2022 18:17:44 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:

> On Fri, Jul 22, 2022 at 02:43:14AM -0500, Glenn Washburn wrote:
> > A user can now specify UUID strings with dashes, instead of having to remove
> > dashes. This is backwards-compatability preserving and also fixes a source
> > of user confusion over the inconsistency with how UUIDs are specified
> > between file system UUIDs and cryptomount UUIDs. Since cryptsetup, the
> > reference implementation for LUKS, displays and generates UUIDs with dashes
> > there has been additional confusion when using the UUID strings from
> > cryptsetup as exact input into GRUB does not find the expected cryptodisk.
> 
> I expect the confusion comes from differences between older and newer
> cryptsetup implementations. AIUI older versions used UUIDs without
> dashes, and this is what GRUB expects today, but newer ones use UUIDs
> with dashes. Right? If it is true it took me some time to get it from
> the text above. So, I think it should be improved a bit to make it more
> clear...

This may be true, I honestly don't know nor recall that being old
behavior of cryptsetup. Do you want me to assume that it is true, and
to modify the commit message accordingly? I could hedge and add a
sentence here like: "It is possible that very old versions of
cryptsetup displayed the UUID without dashes, accounting for the LUKS
handling of UUIDs without dashes".

> 
> > A new function grub_uuidcasecmp is added that is general enough to be used
> > other places where UUIDs are being compared.
> >
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  grub-core/disk/cryptodisk.c |  4 ++--
> >  grub-core/disk/geli.c       |  2 +-
> >  grub-core/disk/luks.c       | 21 ++++-----------------
> >  grub-core/disk/luks2.c      | 15 ++++-----------
> >  include/grub/misc.h         | 27 +++++++++++++++++++++++++++
> >  5 files changed, 38 insertions(+), 31 deletions(-)
> 
> [...]
> 
> > diff --git a/include/grub/misc.h b/include/grub/misc.h
> > index 776dbf8af..39957ecf9 100644
> > --- a/include/grub/misc.h
> > +++ b/include/grub/misc.h
> > @@ -243,6 +243,33 @@ grub_strncasecmp (const char *s1, const char *s2, grub_size_t n)
> >      - (int) grub_tolower ((grub_uint8_t) *s2);
> >  }
> >
> > +/* Do a case insensitive compare of two UUID strings by ignoring all dashes */
> > +static inline int
> > +grub_uuidcasecmp (const char *uuid1, const char *uuid2, grub_size_t n)
> > +{
> > +  if (n == 0)
> > +    return 0;
> > +
> > +  while (*s1 && *s2 && --n)
> > +    {
> > +      /* Skip forward to non-dash on both UUIDs. */
> > +      while ('-' == *s1)
> > +        ++s1;
> > +
> > +      while ('-' == *s2)
> > +        ++s2;
> > +
> > +      if (grub_tolower (*s1) != grub_tolower (*s2))
> 
> I think you took this code from grub_strncasecmp(). It is missing grub_uint8_t
> casts here too. However, if you take a look at grub_strcasecmp(), a few lines
> above, you can see casts in similar place. It is really confusing. I thing the
> casts has been added to drop sign. If I am right then grub_strncasecmp() and
> grub_uuidcasecmp() both should be fixed.

Yes, this part was just copied. So I'll add the casts here and to
grub_strcasecmp(). Should I add the changes to grub_strcasecmp() in
this patch and add a note in the commit message of the change?

> 
> > +	break;
> > +
> > +      s1++;
> > +      s2++;
> > +    }
> > +
> > +  return (int) grub_tolower ((grub_uint8_t) *s1)
> > +    - (int) grub_tolower ((grub_uint8_t) *s2);
> 
> ... and these casts suggests again something is wrong above too...
> 
> Daniel

Glenn

Re: [PATCH v4 2/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner

[Daniel Kiper]

On Tue, Aug 02, 2022 at 01:17:25PM -0500, Glenn Washburn wrote:
> On Tue, 2 Aug 2022 18:17:44 +0200
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > On Fri, Jul 22, 2022 at 02:43:14AM -0500, Glenn Washburn wrote:
> > > A user can now specify UUID strings with dashes, instead of having to remove
> > > dashes. This is backwards-compatability preserving and also fixes a source
> > > of user confusion over the inconsistency with how UUIDs are specified
> > > between file system UUIDs and cryptomount UUIDs. Since cryptsetup, the
> > > reference implementation for LUKS, displays and generates UUIDs with dashes
> > > there has been additional confusion when using the UUID strings from
> > > cryptsetup as exact input into GRUB does not find the expected cryptodisk.
> >
> > I expect the confusion comes from differences between older and newer
> > cryptsetup implementations. AIUI older versions used UUIDs without
> > dashes, and this is what GRUB expects today, but newer ones use UUIDs
> > with dashes. Right? If it is true it took me some time to get it from
> > the text above. So, I think it should be improved a bit to make it more
> > clear...
>
> This may be true, I honestly don't know nor recall that being old
> behavior of cryptsetup. Do you want me to assume that it is true, and
> to modify the commit message accordingly? I could hedge and add a
> sentence here like: "It is possible that very old versions of
> cryptsetup displayed the UUID without dashes, accounting for the LUKS
> handling of UUIDs without dashes".

I thought you know this. If not you can ignore my comment.

> > > A new function grub_uuidcasecmp is added that is general enough to be used
> > > other places where UUIDs are being compared.
> > >
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > ---
> > >  grub-core/disk/cryptodisk.c |  4 ++--
> > >  grub-core/disk/geli.c       |  2 +-
> > >  grub-core/disk/luks.c       | 21 ++++-----------------
> > >  grub-core/disk/luks2.c      | 15 ++++-----------
> > >  include/grub/misc.h         | 27 +++++++++++++++++++++++++++
> > >  5 files changed, 38 insertions(+), 31 deletions(-)
> >
> > [...]
> >
> > > diff --git a/include/grub/misc.h b/include/grub/misc.h
> > > index 776dbf8af..39957ecf9 100644
> > > --- a/include/grub/misc.h
> > > +++ b/include/grub/misc.h
> > > @@ -243,6 +243,33 @@ grub_strncasecmp (const char *s1, const char *s2, grub_size_t n)
> > >      - (int) grub_tolower ((grub_uint8_t) *s2);
> > >  }
> > >
> > > +/* Do a case insensitive compare of two UUID strings by ignoring all dashes */
> > > +static inline int
> > > +grub_uuidcasecmp (const char *uuid1, const char *uuid2, grub_size_t n)
> > > +{
> > > +  if (n == 0)
> > > +    return 0;
> > > +
> > > +  while (*s1 && *s2 && --n)
> > > +    {
> > > +      /* Skip forward to non-dash on both UUIDs. */
> > > +      while ('-' == *s1)
> > > +        ++s1;
> > > +
> > > +      while ('-' == *s2)
> > > +        ++s2;
> > > +
> > > +      if (grub_tolower (*s1) != grub_tolower (*s2))
> >
> > I think you took this code from grub_strncasecmp(). It is missing grub_uint8_t
> > casts here too. However, if you take a look at grub_strcasecmp(), a few lines
> > above, you can see casts in similar place. It is really confusing. I thing the
> > casts has been added to drop sign. If I am right then grub_strncasecmp() and
> > grub_uuidcasecmp() both should be fixed.
>
> Yes, this part was just copied. So I'll add the casts here and to
> grub_strcasecmp(). Should I add the changes to grub_strcasecmp() in
> this patch and add a note in the commit message of the change?

Please fix grub_strcasecmp() in separate patch. However, it can be a
part of this patch series.

Daniel