Re: [PATCH v2 0/5] Fix coverity bugs and add checks for elf values in grub-core

[Darren Kenny]

Hi Alec,

These changes look good to me,

Reviewed-by: Darren Kenny 

Thanks,

Darren.

On Friday, 2022-08-12 at 18:25:44 -04, Alec Brown wrote:
> Updates from v1:
>   - A few patches didn't work on 32-bit platforms. Fixed this by renaming
>   function definitions in multiboot_elfxx.c. The patches that did work 
> were
>   merged with the master branch.
>   - Added a patch to make error conditionals consistent in comparing to
>   GRUB_ERR_NONE.
>
> Coverity identified several untrusted loop bounds and untrusted allocation 
> size
> bugs in grub-core/loader/i386/bsdXX.c and grub-core/loader/multiboot_elfXX.c.
> Upon review of these bugs, I found that specific checks weren't being made to
> various elf header values based on the elf manual page. The first version of
> the patch series contained the patches fixing the Coverity bugs, but those 
> have
> been merged with the master branch. The remaining patches add functions to
> check for the correct elf header values, update previous work done in
> util/grub-module-verifierXX.c that checks elf header values, and update error
> conditionals of the affected files.
>
> Also, I was able to verify that these patches work by using Multiboot and
> Multiboot2 to boot a Xen image.
>
> Alec Brown (5):
>   elf: Validate number of elf section header table entries
>   elf: Validate elf section header table index for section name string 
> table
>   elf: Validate number of elf program header table entries
>   util/grub-module-verifierXX.c: Changed get_shnum() return type
>   loader: Update error conditionals to use enums
>
>  grub-core/kern/elf.c   |  18 ++
>  grub-core/kern/elfXX.c | 101 
> +
>  grub-core/loader/i386/bsdXX.c  | 131 
> +++
>  grub-core/loader/multiboot_elfxx.c |  85 
> +
>  include/grub/elf.h |  23 +++
>  util/grub-module-verifierXX.c  |  10 ++
>  6 files changed, 292 insertions(+), 76 deletions(-)
>
> Interdiff against v1:
> diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
> index 2cc1daa49..09d909f1b 100644
> --- a/grub-core/loader/i386/bsdXX.c
> +++ b/grub-core/loader/i386/bsdXX.c
> @@ -52,7 +52,7 @@ read_headers (grub_file_t file, const char *filename, 
> Elf_Ehdr *e, Elf_Shdr **sh
>  return grub_error (GRUB_ERR_BAD_OS, N_("invalid arch-dependent ELF 
> magic"));
>
>err = grub_elf_get_shnum (e, &shnum);
> -  if (err)
> +  if (err != GRUB_ERR_NONE)
>  return err;
>
>*shdr = grub_calloc (shnum, e->e_shentsize);
> @@ -94,7 +94,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct 
> grub_relocator *relocator,
>curload = module = ALIGN_PAGE (*kern_end);
>
>err = read_headers (file, argv[0], &e, &shdr);
> -  if (err)
> +  if (err != GRUB_ERR_NONE)
>  goto out;
>
>err = grub_elf_get_shnum (&e, &shnum);
> @@ -118,7 +118,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct 
> grub_relocator *relocator,
>  grub_relocator_chunk_t ch;
>  err = grub_relocator_alloc_chunk_addr (relocator, &ch,
>module, chunk_size);
> -if (err)
> +if (err != GRUB_ERR_NONE)
>goto out;
>  chunk_src = get_virtual_current_address (ch);
>}
> @@ -143,7 +143,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct 
> grub_relocator *relocator,
> case SHT_PROGBITS:
>   err = load (file, argv[0], (grub_uint8_t *) chunk_src + curload - 
> *kern_end,
>   s->sh_offset, s->sh_size);
> - if (err)
> + if (err != GRUB_ERR_NONE)
> goto out;
>   break;
> case SHT_NOBITS:
> @@ -159,11 +159,11 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct 
> grub_relocator *relocator,
>err = grub_freebsd_add_meta_module (argv[0], 
> FREEBSD_MODTYPE_ELF_MODULE_OBJ,
>   argc - 1, argv + 1, module,
>   curload - module);
> -  if (! err)
> +  if (err == GRUB_ERR_NONE)
>  err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA
>  | FREEBSD_MODINFOMD_ELFHDR,
>  &e, sizeof (e));
> -  if (! err)
> +  if (err == GRUB_ERR_NONE)
>  err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA
>  | FREEBSD_MODINFOMD_SHDR,
>  shdr, shnum * e.e_shentsize);
> @@ -192,7 +192,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct 
> grub_relocator *relocator,
>curload = module = ALIGN_PAGE (*kern_end);
>
>err = read_headers (file, argv[0], &e, &shdr);
> -  if (err)
> +  if (err != GRUB_ERR_NONE)
>  goto o

Re: [PATCH v2 1/1] Add support for grub-emu to kexec Linux menu entries

[Raymund Will]

Hi,

please let me try to add a bit of context and explain three IMHO
crucial points of the proposed patch.

First, it is meant to work without any changes to config-files
on 'linux'-systems, simply by calling `grub-emu --kexec`.
And, called this way, it actually uses `systemctl kexec` exclusively
to instruct `systemd` to perform the "kexec"-reboot in a sane and
safe manner.

Second, it only supports a very limited set of commands in `grub.cfg`,
as `grub-emu` can not implement the full functionality of a firmware-
loaded `grub` binary (like raw-device access) and it hinges massively
on proper `kexec`-support from the machine/firmware, so unfortunately
it won't be universally useful,

Third, for use in a "pre-boot environment" (i.e. initrd/chroot), which
has full control over it's state, but no (fully) working `systemd`,
a call to `grub-emu --kexec --kexec` changes the behavior to allow a
fall-forward to `kexec -e`.  As a safe-guard for the adventurously
minded `systemctl kexec` is still tried first!

This third point describes the use-case the original patch-set was
developed for and the second doesn't hurt (much) on the systems it
is deployed/used in the field.  But the first was found to be really
useful, especially on machines, which can reliably `kexec`, but are
dead slow through cold-boot (think "huge memory", "tons of devices")
and/or have "exotic" console setups ("3215" anybody?).  Note that,
as the boot configuration (namely `grub.cfg`) wasn't changed, regular
boot can't be affected by this short-cut (particularly, when `kexec`
might have failed).

Granted, the duplication of `--kexec` to signify "force it", might
as well be spelled out as `--force-kexec` (or something similar).
(But that change will provoke inconsistencies during an indefinite
migration phase, where pre-boot images don't match binaries in the
root filesystem, notably, when rollback snapshots come into play.)

Config-overrides in `grub.cfg` in turn would be a nice addition, but
are relatively expensive to implement, as they'd probably need to be
parsed and split into an array for `grub_util_exec()`, right?

But, please, still leave sane defaults in the binary, for out-of-
the-box, no-config-changes-necessary usage, pretty please!

Would it be possible to re-evaluate the proposed patch with this
in mind?

PS: in light of my statements above, the "description" of this
patch definitely needs re-wording...

Thanks,
-- 
Raymund WILL  r...@suse.de
SUSE Software Solutions Germany GmbH, Frankenstr. 146, D-90461 Nuernberg
Geschaeftsfuehrer: Ivo Totev, et al(HRB 36809, AG Nuernberg)

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] luks2: Continue trying all keyslots even if there are some failures

[Patrick Steinhardt]

On Fri, Jul 22, 2022 at 03:04:50AM -0500, Glenn Washburn wrote:
> luks2_get_keyslot can fail for a variety of reasons that do not neccesarily
> mean the next keyslot should not be tried (eg. a new kdf type). So always
> try the next slot. This will make GRUB more resilient to non-spec json data
> that 3rd party systems may add. We do not care if some of the keyslots are
> unusable, only if there is at least one that is.
> 
> Signed-off-by: Glenn Washburn 
> ---
>  grub-core/disk/luks2.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index bf741d70f..d8d3180ed 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -610,7 +610,15 @@ luks2_recover_key (grub_disk_t source,
>grub_errno = GRUB_ERR_NONE;
>ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, json_idx);
>if (ret)
> - goto err;
> + {
> +   /*
> +* luks2_get_keyslot can fail for a variety of reasons that do not
> +* neccesarily mean the next keyslot should not be tried (eg. a new
> +* kdf type). So always try the next slot.
> +*/
> +   grub_dprintf ("luks2", "Failed to get keyslot %" PRIuGRUB_UINT64_T 
> "\n", keyslot.idx);
> +   continue;
> + }
>if (grub_errno != GRUB_ERR_NONE)
> grub_dprintf ("luks2", "Ignoring unhandled error %d from 
> luks2_get_keyslot\n", grub_errno);
>  
> -- 
> 2.34.1
> 

Reviewed-by: Patrick Steinhardt 


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Patrick Steinhardt]

On Thu, Aug 11, 2022 at 12:48:43PM -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.
> 
> 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 
> ---
>  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 | 28 
>  5 files changed, 39 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 7f837d52c..9e1e6a5d9 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];
> @@ -92,19 +89,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;
>  }
>  
> @@ -123,7 +110,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.  */
> @@ -143,7 +130,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 bf741d70f..ac3ab095c 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -350,8 +350,6 @@ luks2_scan (grub_disk_t disk, g

Re: [PATCH v5 1/2] json: Add function to unescape JSON-encoded strings

[Patrick Steinhardt]

On Tue, Jul 12, 2022 at 03:39:13PM +0200, Daniel Kiper wrote:
> On Mon, Jul 11, 2022 at 09:08:09AM -0400, Nicholas Vinson wrote:
> > On 7/11/22 06:44, Patrick Steinhardt wrote:
> > > JSON strings require certain characters to be encoded, either by using a
> > > single reverse solidus character "\" for a set of popular characters, or
> > > by using a Unicode representation of "\uX". The jsmn library doesn't
> > > handle unescaping for us, so we must implement this functionality for
> > > ourselves.
> > >
> > > Add a new function `grub_json_unescape ()` that takes a potentially
> > > escaped JSON string as input and returns a new unescaped string.
> > >
> > > Signed-off-by: Patrick Steinhardt 
> > > ---
> > >   grub-core/lib/json/json.c | 118 ++
> > >   grub-core/lib/json/json.h |  12 
> > >   2 files changed, 130 insertions(+)
> > >
> > > diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
> > > index 1c20c75ea..1eadd1ce9 100644
> > > --- a/grub-core/lib/json/json.c
> > > +++ b/grub-core/lib/json/json.c
> > > @@ -262,3 +262,121 @@ grub_json_getint64 (grub_int64_t *out, const 
> > > grub_json_t *parent, const char *ke
> > > return GRUB_ERR_NONE;
> > >   }
> > > +
> > > +grub_err_t
> > > +grub_json_unescape (char **out, grub_size_t *outlen, const char *in, 
> > > grub_size_t inlen)
> > > +{
> > > +  grub_err_t ret = GRUB_ERR_NONE;
> > > +  grub_size_t inpos, resultpos;
> > > +  char *result;
> > > +
> > > +  if (out == NULL || outlen == NULL)
> > > +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("output parameters are 
> > > not set"));
> > > +
> > > +  result = grub_calloc (1, inlen + 1);
> > > +  if (result == NULL)
> > > +return GRUB_ERR_OUT_OF_MEMORY;
> > > +
> > > +  for (inpos = resultpos = 0; inpos < inlen; inpos++)
> > > +{
> > > +  if (in[inpos] == '\\')
> > > + {
> > > +   inpos++;
> > > +   if (inpos >= inlen)
> > > + {
> > > +   ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("expected escaped 
> > > character"));
> > > +   goto err;
> > > + }
> > > +
> > > +   switch (in[inpos])
> > > + {
> > > +   case '"':
> > > + result[resultpos++] = '"';
> > > + break;
> > > +
> > > +   case '/':
> > > + result[resultpos++] = '/';
> > > + break;
> > > +
> > > +   case '\\':
> > > + result[resultpos++] = '\\';
> > > + break;
> > > +
> > > +   case 'b':
> > > + result[resultpos++] = '\b';
> > > + break;
> > > +
> > > +   case 'f':
> > > + result[resultpos++] = '\f';
> > > + break;
> > > +
> > > +   case 'r':
> > > + result[resultpos++] = '\r';
> > > + break;
> > > +
> > > +   case 'n':
> > > + result[resultpos++] = '\n';
> > > + break;
> > > +
> > > +   case 't':
> > > + result[resultpos++] = '\t';
> > > + break;
> > > +
> > > +   case 'u':
> > > + {
> > > +   char values[4] = {0};
> > > +   unsigned i;
> > > +
> > > +   inpos++;
> > > +   if (inpos + ARRAY_SIZE(values) > inlen)
> > > + {
> > > +   ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unicode 
> > > sequence too short"));
> > > +   goto err;
> > > + }
> >
> > I recommend relaxing these errors a little bit. While technically
> > correct, I'm thinking it might serve GRUB better to be a bit more
> > forgiving.
> >
> > Perhaps something like:
> >
> > if the input is '\u' where ? is not a hex digit, set the value to
> > 'u', and process  separately.
> >
> > if  is less than ARRAY_SIZE chars, left-pad  with 0s* until it's
> > the correct length then convert the value.
> >
> > (* I don't mean to literally pad. I mean to treat the short number as if
> > it was left-padded with 0s)
> >
> > > +
> > > +   for (i = 0; i < ARRAY_SIZE(values); i++)
> > > + {
> > > +   char c = in[inpos++];
> > > +
> > > +   if (c >= '0' && c <= '9')
> > > + values[i] = c - '0';
> > > +   else if (c >= 'A' && c <= 'F')
> > > + values[i] = c - 'A' + 10;
> > > +   else if (c >= 'a' && c <= 'f')
> > > + values[i] = c - 'a' + 10;
> > > +   else
> > > + {
> >
> > With a similar argument here. Treat the short  as suggested above
> > and resume normal processing at the non-hex digit position.
> >
> > > +   ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
> > > + N_("unicode sequence with invalid 
> > > character '%c'"), c);
> > > +   goto err;
> > > + }
> > > + }
> > > +
> > > +   if (values[0] != 0 || values[1] != 0)
> > > + result[resultpos++] = values[0] << 4 | values[1];
> > > +   result[resultpos++] = values[2] << 4 | values[3];
> > > +
> > > +   /* Offset the incr

Re: [PATCH v9 03/10] LoongArch: Add setjmp implementation

[孙海勇]

Hi

 I've tested this patch set on a physical machine, the OS 
environment is a CLFS system I made based on LoongArch and they work 
well. It would be great if this patch set could be merged into the 
master branch. That way, I can happily boot my system directly with the 
official code.



在 2022/8/11 20:28, Xiaotian Wu 写道:

Signed-off-by: Xiaotian Wu 
Signed-off-by: Zhou Yang 
Signed-off-by: Sun Haiyong 
---
  grub-core/lib/loongarch64/setjmp.S | 69 ++
  grub-core/lib/setjmp.S |  2 +
  include/grub/loongarch64/setjmp.h  | 27 
  3 files changed, 98 insertions(+)
  create mode 100644 grub-core/lib/loongarch64/setjmp.S
  create mode 100644 include/grub/loongarch64/setjmp.h

diff --git a/grub-core/lib/loongarch64/setjmp.S 
b/grub-core/lib/loongarch64/setjmp.S
new file mode 100644
index 0..41d58f569
--- /dev/null
+++ b/grub-core/lib/loongarch64/setjmp.S
@@ -0,0 +1,69 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2022 Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see .
+ */
+
+#include 
+
+   .file   "setjmp.S"
+
+GRUB_MOD_LICENSE "GPLv3+"
+
+   .text
+
+/*
+ * int grub_setjmp (jmp_buf env)
+ */
+FUNCTION(grub_setjmp)
+   st.d $s0, $a0, 0x0
+   st.d $s1, $a0, 0x8
+   st.d $s2, $a0, 0x10
+   st.d $s3, $a0, 0x18
+   st.d $s4, $a0, 0x20
+   st.d $s5, $a0, 0x28
+   st.d $s6, $a0, 0x30
+   st.d $s7, $a0, 0x38
+   st.d $s8, $a0, 0x40
+   st.d $fp, $a0, 0x48
+   st.d $sp, $a0, 0x50
+   st.d $ra, $a0, 0x58
+
+   move $a0, $zero
+   jr   $ra
+
+/*
+ * void grub_longjmp (jmp_buf env, int val)
+ */
+FUNCTION(grub_longjmp)
+   ld.d $s0, $a0, 0x0
+   ld.d $s1, $a0, 0x8
+   ld.d $s2, $a0, 0x10
+   ld.d $s3, $a0, 0x18
+   ld.d $s4, $a0, 0x20
+   ld.d $s5, $a0, 0x28
+   ld.d $s6, $a0, 0x30
+   ld.d $s7, $a0, 0x38
+   ld.d $s8, $a0, 0x40
+   ld.d $fp, $a0, 0x48
+   ld.d $sp, $a0, 0x50
+   ld.d $ra, $a0, 0x58
+
+   /* Return 1 if passed 0, otherwise returns the value in place. */
+   li.w $a0, 1
+   beqz $a1, 1f
+   move $a0, $a1
+1:
+   jr   $ra
diff --git a/grub-core/lib/setjmp.S b/grub-core/lib/setjmp.S
index 9c8721088..cba1d546d 100644
--- a/grub-core/lib/setjmp.S
+++ b/grub-core/lib/setjmp.S
@@ -19,6 +19,8 @@
  #include "./arm/setjmp.S"
  #elif defined(__aarch64__)
  #include "./arm64/setjmp.S"
+#elif defined(__loongarch64)
+#include "./loongarch64/setjmp.S"
  #elif defined(__riscv)
  #include "./riscv/setjmp.S"
  #else
diff --git a/include/grub/loongarch64/setjmp.h 
b/include/grub/loongarch64/setjmp.h
new file mode 100644
index 0..cb3e17763
--- /dev/null
+++ b/include/grub/loongarch64/setjmp.h
@@ -0,0 +1,27 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2022 Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see .
+ */
+
+#ifndef GRUB_SETJMP_CPU_HEADER
+#define GRUB_SETJMP_CPU_HEADER 1
+
+typedef grub_uint64_t grub_jmp_buf[12];
+
+int grub_setjmp (grub_jmp_buf env) RETURNS_TWICE;
+void grub_longjmp (grub_jmp_buf env, int val) __attribute__ ((noreturn));
+
+#endif /* ! GRUB_SETJMP_CPU_HEADER */



___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v9 03/10] LoongArch: Add setjmp implementation

[孙海勇]

Hi

 I've tested this patch set on a physical machine, the OS environment is a 
CLFS system I made based on LoongArch and they work well. It would be great if 
this patch set could be merged into the master branch. That way, I can happily 
boot my system directly with the official code.


> -原始邮件-
> 发件人: "Xiaotian Wu" 
> 发送时间:2022-08-11 20:28:18 (星期四)
> 收件人: grub-devel@gnu.org
> 抄送: g...@xen0n.name, developm...@efficientek.com, daniel.ki...@oracle.com, 
> "Xiaotian Wu" , "Zhou Yang" , 
> "Sun Haiyong" 
> 主题: [PATCH v9 03/10] LoongArch: Add setjmp implementation
> 
> Signed-off-by: Xiaotian Wu 
> Signed-off-by: Zhou Yang 
> Signed-off-by: Sun Haiyong 
> ---
>  grub-core/lib/loongarch64/setjmp.S | 69 ++
>  grub-core/lib/setjmp.S |  2 +
>  include/grub/loongarch64/setjmp.h  | 27 
>  3 files changed, 98 insertions(+)
>  create mode 100644 grub-core/lib/loongarch64/setjmp.S
>  create mode 100644 include/grub/loongarch64/setjmp.h
> 
> diff --git a/grub-core/lib/loongarch64/setjmp.S 
> b/grub-core/lib/loongarch64/setjmp.S
> new file mode 100644
> index 0..41d58f569
> --- /dev/null
> +++ b/grub-core/lib/loongarch64/setjmp.S
> @@ -0,0 +1,69 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2022 Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +#include 
> +
> + .file   "setjmp.S"
> +
> +GRUB_MOD_LICENSE "GPLv3+"
> +
> + .text
> +
> +/*
> + * int grub_setjmp (jmp_buf env)
> + */
> +FUNCTION(grub_setjmp)
> + st.d $s0, $a0, 0x0
> + st.d $s1, $a0, 0x8
> + st.d $s2, $a0, 0x10
> + st.d $s3, $a0, 0x18
> + st.d $s4, $a0, 0x20
> + st.d $s5, $a0, 0x28
> + st.d $s6, $a0, 0x30
> + st.d $s7, $a0, 0x38
> + st.d $s8, $a0, 0x40
> + st.d $fp, $a0, 0x48
> + st.d $sp, $a0, 0x50
> + st.d $ra, $a0, 0x58
> +
> + move $a0, $zero
> + jr   $ra
> +
> +/*
> + * void grub_longjmp (jmp_buf env, int val)
> + */
> +FUNCTION(grub_longjmp)
> + ld.d $s0, $a0, 0x0
> + ld.d $s1, $a0, 0x8
> + ld.d $s2, $a0, 0x10
> + ld.d $s3, $a0, 0x18
> + ld.d $s4, $a0, 0x20
> + ld.d $s5, $a0, 0x28
> + ld.d $s6, $a0, 0x30
> + ld.d $s7, $a0, 0x38
> + ld.d $s8, $a0, 0x40
> + ld.d $fp, $a0, 0x48
> + ld.d $sp, $a0, 0x50
> + ld.d $ra, $a0, 0x58
> +
> + /* Return 1 if passed 0, otherwise returns the value in place. */
> + li.w $a0, 1
> + beqz $a1, 1f
> + move $a0, $a1
> +1:
> + jr   $ra
> diff --git a/grub-core/lib/setjmp.S b/grub-core/lib/setjmp.S
> index 9c8721088..cba1d546d 100644
> --- a/grub-core/lib/setjmp.S
> +++ b/grub-core/lib/setjmp.S
> @@ -19,6 +19,8 @@
>  #include "./arm/setjmp.S"
>  #elif defined(__aarch64__)
>  #include "./arm64/setjmp.S"
> +#elif defined(__loongarch64)
> +#include "./loongarch64/setjmp.S"
>  #elif defined(__riscv)
>  #include "./riscv/setjmp.S"
>  #else
> diff --git a/include/grub/loongarch64/setjmp.h 
> b/include/grub/loongarch64/setjmp.h
> new file mode 100644
> index 0..cb3e17763
> --- /dev/null
> +++ b/include/grub/loongarch64/setjmp.h
> @@ -0,0 +1,27 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2022 Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +#ifndef GRUB_SETJMP_CPU_HEADER
> +#define GRUB_SETJMP_CPU_HEADER   1
> +
> +typedef grub_uint64_t grub_jmp_buf[12];
> +
> +int grub_setjmp (grub_jmp_buf env) RETURNS_TWICE;
> +void grub_longjmp (grub_jmp_buf env, int val) __attribute__ ((noreturn));
> +
> +#endif /* ! GRUB_SETJMP_CPU_HEADER */
> -- 
> 2.35.1


本邮件及其附件含有龙芯中科的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人

[PATCH v6 0/2] luks2: Fix decoding of digests and salts with escaped chars

[Patrick Steinhardt]

Hi,

this is the sixth version of my patch series which fixes decoding of
digests and salts in LUKS2 headers in case they happen to contain
escaped characters. While modern cryptsetup versions in fact don't
escape any characters part of the Base64 alphabet, old versions of
cryptsetup did this until v2.0.2.

There's only a single change compared to v5, which is a removed type
cast that was not in fact needed. I didn't include the feedback from
Nicholas to make the JSON string parsing more lenient. While sensible,
it's rather a theoretical concern right now as theer was only a single
version of cryptsetup that ever wrote escaped characters, and even then
of the Base64 alphabet only the backslash may have been escaped. So I
think we should rather defer any improvements until there we discover
real-world problems or until there are more usecases for this function.

Patrick

Patrick Steinhardt (2):
  json: Add function to unescape JSON-encoded strings
  luks2: Fix decoding of digests and salts with escaped chars

 grub-core/disk/luks2.c|  28 +++--
 grub-core/lib/json/json.c | 118 ++
 grub-core/lib/json/json.h |  12 
 3 files changed, 154 insertions(+), 4 deletions(-)

Range-diff against v5:
1:  ebab6b092 ! 1:  c44675566 json: Add function to unescape JSON-encoded 
strings
@@ Commit message
 Add a new function `grub_json_unescape ()` that takes a potentially
 escaped JSON string as input and returns a new unescaped string.
 
+Reviewed-by: Daniel Kiper 
 Signed-off-by: Patrick Steinhardt 
 
  ## grub-core/lib/json/json.c ##
2:  60ccd669d ! 2:  16ae4ef05 luks2: Fix decoding of digests and salts with 
escaped chars
@@ Commit message
 that handles unescaping for us.
 
 Reported-by: Afdal
+Reviewed-by: Daniel Kiper 
 Signed-off-by: Patrick Steinhardt 
 
  ## grub-core/disk/luks2.c ##
@@ grub-core/disk/luks2.c: luks2_scan (grub_disk_t disk, 
grub_cryptomount_args_t ca
 +  if (grub_json_unescape (&unescaped, &unescaped_len, in, inlen) != 
GRUB_ERR_NONE)
 +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not unescape 
Base64 string"));
 +
-+  successful = base64_decode (unescaped, (grub_size_t) unescaped_len, 
(char *) decoded, decodedlen);
++  successful = base64_decode (unescaped, unescaped_len, (char *) decoded, 
decodedlen);
 +  grub_free (unescaped);
 +  if (!successful)
 +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not decode Base64 
string"));
-- 
2.37.1



signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v6 1/2] json: Add function to unescape JSON-encoded strings

[Patrick Steinhardt]

JSON strings require certain characters to be encoded, either by using a
single reverse solidus character "\" for a set of popular characters, or
by using a Unicode representation of "\uX". The jsmn library doesn't
handle unescaping for us, so we must implement this functionality for
ourselves.

Add a new function `grub_json_unescape ()` that takes a potentially
escaped JSON string as input and returns a new unescaped string.

Reviewed-by: Daniel Kiper 
Signed-off-by: Patrick Steinhardt 
---
 grub-core/lib/json/json.c | 118 ++
 grub-core/lib/json/json.h |  12 
 2 files changed, 130 insertions(+)

diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
index 1c20c75ea..1eadd1ce9 100644
--- a/grub-core/lib/json/json.c
+++ b/grub-core/lib/json/json.c
@@ -262,3 +262,121 @@ grub_json_getint64 (grub_int64_t *out, const grub_json_t 
*parent, const char *ke
 
   return GRUB_ERR_NONE;
 }
+
+grub_err_t
+grub_json_unescape (char **out, grub_size_t *outlen, const char *in, 
grub_size_t inlen)
+{
+  grub_err_t ret = GRUB_ERR_NONE;
+  grub_size_t inpos, resultpos;
+  char *result;
+
+  if (out == NULL || outlen == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("output parameters are not 
set"));
+
+  result = grub_calloc (1, inlen + 1);
+  if (result == NULL)
+return GRUB_ERR_OUT_OF_MEMORY;
+
+  for (inpos = resultpos = 0; inpos < inlen; inpos++)
+{
+  if (in[inpos] == '\\')
+   {
+ inpos++;
+ if (inpos >= inlen)
+   {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("expected escaped 
character"));
+ goto err;
+   }
+
+ switch (in[inpos])
+   {
+ case '"':
+   result[resultpos++] = '"';
+   break;
+
+ case '/':
+   result[resultpos++] = '/';
+   break;
+
+ case '\\':
+   result[resultpos++] = '\\';
+   break;
+
+ case 'b':
+   result[resultpos++] = '\b';
+   break;
+
+ case 'f':
+   result[resultpos++] = '\f';
+   break;
+
+ case 'r':
+   result[resultpos++] = '\r';
+   break;
+
+ case 'n':
+   result[resultpos++] = '\n';
+   break;
+
+ case 't':
+   result[resultpos++] = '\t';
+   break;
+
+ case 'u':
+   {
+ char values[4] = {0};
+ unsigned i;
+
+ inpos++;
+ if (inpos + ARRAY_SIZE(values) > inlen)
+   {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unicode 
sequence too short"));
+ goto err;
+   }
+
+ for (i = 0; i < ARRAY_SIZE(values); i++)
+   {
+ char c = in[inpos++];
+
+ if (c >= '0' && c <= '9')
+   values[i] = c - '0';
+ else if (c >= 'A' && c <= 'F')
+   values[i] = c - 'A' + 10;
+ else if (c >= 'a' && c <= 'f')
+   values[i] = c - 'a' + 10;
+ else
+   {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+   N_("unicode sequence with invalid 
character '%c'"), c);
+ goto err;
+   }
+   }
+
+ if (values[0] != 0 || values[1] != 0)
+   result[resultpos++] = values[0] << 4 | values[1];
+ result[resultpos++] = values[2] << 4 | values[3];
+
+ /* Offset the increment that's coming in via the loop 
increment. */
+ inpos--;
+
+ break;
+   }
+
+ default:
+   ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unrecognized 
escaped character '%c'"), in[inpos]);
+   goto err;
+   }
+   }
+  else
+ result[resultpos++] = in[inpos];
+}
+
+  *out = result;
+  *outlen = resultpos;
+
+ err:
+  if (ret != GRUB_ERR_NONE)
+grub_free (result);
+
+  return ret;
+}
diff --git a/grub-core/lib/json/json.h b/grub-core/lib/json/json.h
index 4ea2a22d8..626074c35 100644
--- a/grub-core/lib/json/json.h
+++ b/grub-core/lib/json/json.h
@@ -125,4 +125,16 @@ extern grub_err_t EXPORT_FUNC(grub_json_getint64) 
(grub_int64_t *out,
   const grub_json_t *parent,
   const char *key);
 
+/*
+ * Unescape escaped characters and Unicode sequences in the
+ * given JSON-encoded string. Returns a newly allocated string
+ * passed back via the `out` parameter that has a length of
+ * `*outlen`.
+ *
+ * See https://datatracker.ietf.org/doc/html/rfc8259#section-7 for more
+ * information on e

[PATCH v6 2/2] luks2: Fix decoding of digests and salts with escaped chars

[Patrick Steinhardt]

It was reported in the #grub IRC channel on Libera that decryption of
LUKS2 partitions fails with errors about invalid digests and/or salts.
In all of these cases, what failed was decoding the Base64
representation of these, where the encoded data contained invalid
characters.

As it turns out, the root cause is that json-c, which is used by
cryptsetup to read and write the JSON header, will escape some
characters by prepending a backslash when writing JSON strings by
default. Most importantly, json-c also escapes the forward slash, which
is part of the Base64 alphabet. Because GRUB doesn't know to unescape
such characters, decoding this string will rightfully fail.

Interestingly, this issue has until now only been reported by users of
Ubuntu 18.04. And a bit of digging in fact reveals that cryptsetup has
changed the logic in a054206d (Suppress useless slash escaping in json
lib, 2018-04-20), which has been released with cryptsetup v2.0.3. Ubuntu
18.04 is still shipping with cryptsetup v2.0.2 though, which explains
why this is not a more frequent issue.

Fix the issue by using our new `grub_json_unescape ()` helper function
that handles unescaping for us.

Reported-by: Afdal
Reviewed-by: Daniel Kiper 
Signed-off-by: Patrick Steinhardt 
---
 grub-core/disk/luks2.c | 28 
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index bf741d70f..32cc8c6cb 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -384,6 +384,24 @@ luks2_scan (grub_disk_t disk, grub_cryptomount_args_t 
cargs)
   return cryptodisk;
 }
 
+static grub_err_t
+luks2_base64_decode (const char *in, grub_size_t inlen, grub_uint8_t *decoded, 
idx_t *decodedlen)
+{
+  grub_size_t unescaped_len = 0;
+  char *unescaped = NULL;
+  bool successful;
+
+  if (grub_json_unescape (&unescaped, &unescaped_len, in, inlen) != 
GRUB_ERR_NONE)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not unescape Base64 
string"));
+
+  successful = base64_decode (unescaped, unescaped_len, (char *) decoded, 
decodedlen);
+  grub_free (unescaped);
+  if (!successful)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not decode Base64 
string"));
+
+  return GRUB_ERR_NONE;
+}
+
 static grub_err_t
 luks2_verify_key (grub_luks2_digest_t *d, grub_uint8_t *candidate_key,
  grub_size_t candidate_key_len)
@@ -395,9 +413,11 @@ luks2_verify_key (grub_luks2_digest_t *d, grub_uint8_t 
*candidate_key,
   gcry_err_code_t gcry_ret;
 
   /* Decode both digest and salt */
-  if (!base64_decode (d->digest, grub_strlen (d->digest), (char *)digest, 
&digestlen))
+  if (luks2_base64_decode (d->digest, grub_strlen (d->digest),
+  digest, &digestlen) != GRUB_ERR_NONE)
 return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid digest");
-  if (!base64_decode (d->salt, grub_strlen (d->salt), (char *)salt, &saltlen))
+  if (luks2_base64_decode (d->salt, grub_strlen (d->salt),
+  salt, &saltlen) != GRUB_ERR_NONE)
 return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid digest salt");
 
   /* Configure the hash used for the digest. */
@@ -435,8 +455,8 @@ luks2_decrypt_key (grub_uint8_t *out_key,
   gcry_err_code_t gcry_ret;
   grub_err_t ret;
 
-  if (!base64_decode (k->kdf.salt, grub_strlen (k->kdf.salt),
-(char *)salt, &saltlen))
+  if (luks2_base64_decode (k->kdf.salt, grub_strlen (k->kdf.salt),
+  salt, &saltlen) != GRUB_ERR_NONE)
 {
   ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid keyslot salt");
   goto err;
-- 
2.37.1



signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v6 0/2] luks2: Fix decoding of digests and salts with escaped chars

[Glenn Washburn]

On Mon, 15 Aug 2022 17:52:45 +0200
Patrick Steinhardt  wrote:

> Hi,
> 
> this is the sixth version of my patch series which fixes decoding of
> digests and salts in LUKS2 headers in case they happen to contain
> escaped characters. While modern cryptsetup versions in fact don't
> escape any characters part of the Base64 alphabet, old versions of
> cryptsetup did this until v2.0.2.
> 
> There's only a single change compared to v5, which is a removed type
> cast that was not in fact needed. I didn't include the feedback from
> Nicholas to make the JSON string parsing more lenient. While sensible,
> it's rather a theoretical concern right now as theer was only a single
> version of cryptsetup that ever wrote escaped characters, and even then
> of the Base64 alphabet only the backslash may have been escaped. So I
> think we should rather defer any improvements until there we discover
> real-world problems or until there are more usecases for this function.

I support and concur with this rationale. We can change it when it
becomes an issue.

Glenn

> 
> Patrick
> 
> Patrick Steinhardt (2):
>   json: Add function to unescape JSON-encoded strings
>   luks2: Fix decoding of digests and salts with escaped chars
> 
>  grub-core/disk/luks2.c|  28 +++--
>  grub-core/lib/json/json.c | 118 ++
>  grub-core/lib/json/json.h |  12 
>  3 files changed, 154 insertions(+), 4 deletions(-)
> 
> Range-diff against v5:
> 1:  ebab6b092 ! 1:  c44675566 json: Add function to unescape JSON-encoded 
> strings
> @@ Commit message
>  Add a new function `grub_json_unescape ()` that takes a potentially
>  escaped JSON string as input and returns a new unescaped string.
>  
> +Reviewed-by: Daniel Kiper 
>  Signed-off-by: Patrick Steinhardt 
>  
>   ## grub-core/lib/json/json.c ##
> 2:  60ccd669d ! 2:  16ae4ef05 luks2: Fix decoding of digests and salts with 
> escaped chars
> @@ Commit message
>  that handles unescaping for us.
>  
>  Reported-by: Afdal
> +Reviewed-by: Daniel Kiper 
>  Signed-off-by: Patrick Steinhardt 
>  
>   ## grub-core/disk/luks2.c ##
> @@ grub-core/disk/luks2.c: luks2_scan (grub_disk_t disk, 
> grub_cryptomount_args_t ca
>  +  if (grub_json_unescape (&unescaped, &unescaped_len, in, inlen) != 
> GRUB_ERR_NONE)
>  +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not unescape 
> Base64 string"));
>  +
> -+  successful = base64_decode (unescaped, (grub_size_t) unescaped_len, 
> (char *) decoded, decodedlen);
> ++  successful = base64_decode (unescaped, unescaped_len, (char *) 
> decoded, decodedlen);
>  +  grub_free (unescaped);
>  +  if (!successful)
>  +return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not decode 
> Base64 string"));

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Glenn Washburn]

On Mon, 15 Aug 2022 17:28:24 +0200
Patrick Steinhardt  wrote:

> On Thu, Aug 11, 2022 at 12:48:43PM -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.
> > 
> > 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 
> > ---
> >  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 | 28 
> >  5 files changed, 39 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 7f837d52c..9e1e6a5d9 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];
> > @@ -92,19 +89,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;
> >  }
> >  
> > @@ -123,7 +110,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.  */
> > @@ -143,7 +130,7 @@ luks_scan (grub_disk_t disk, grub_cryptomount_args_t 
> > cargs)
> >return NULL;
> >  }
> >  
> > -  COMPILE_TIME_ASSERT (sizeof (newdev->uuid) >= sizeof (uuid));
> > +  COMPILE_TIME_