grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] core: commands: efi: add commands to get/set EFI vars


From: Glenn Washburn
Subject: Re: [PATCH 2/2] core: commands: efi: add commands to get/set EFI vars
Date: Mon, 29 Aug 2022 00:28:30 -0500

On Wed, 29 Apr 2020 16:54:10 +0200
Flavio Suligoi <f.suligoi@asem.it> wrote:

> Sometimes, using a GRUB boot script (i.e. grub.cfg), it is very important
> to get/set an EFI variable, especially in embedded systems, to detect
> some custom BIOS features.
> For example, based on the content of some EFI variables, a script can decide
> to boot a special kernel instead of the usual kernel, or add some custom boot
> parameters to the kernel command line, etc.
> 
> This patch add two useful commands:
> 
> - efivar_get
> - efivar_set
> 
> to get/set an EFI variable.
> 
> The "get" function can display the EFI variable in ASCII or in HEX+ASCII
> mode (like the Linux command "hexdump"), while the "set" function can set the
> EFI variable in ASCII mode only.
> 
> As GUID, the default GUID used is the standard EFI_GLOBAL_VARIABLE_GUID,
> but it is possible to use any other GUID (standard or custom) using an
> environment variable.
> 
> Example: set the EFI variable "Configuration"" to the ASCII value 
> ‘Board_special
> version 1.0.0’, with the following custom GUID:
> 
> ‘Configuration-a87cb185-864c-4683-a93e-3666ad3cf6b6’
> 
> grub> set guid_used=a87cb185-864c-4683-a93e-3666ad3cf6b6
> grub>
> grub> efivar_set -g guid_used Configuration "Board_special version 1.0.0"

I think it makes more sense for -g to take the literal guid as the
argument. So instead the above line would be:

  efivar_set -g ${guid_used} Configuration "Board_special version 1.0.0"

> grub>
> grub> efivar_get -g guid_used -xv Configuration
> Configuration-a87cb185-864c-4683-a93e-3666ad3cf6b6 :
> 
> 0000 - 42 6f 61 72 64 5f 73 70 65 63 69 61 6c 20 76 65 |Board_special ve|
> 0010 - 72 73 69 6f 6e 20 31 2e 30 2e 30 00             |rsion 1.0.0.    |
> 
> -----
> Usage
> -----
> 
> Usage: efivar_get [OPTIONS] VAR
> 
> Read EFI VAR variable (if not specified, use EFI_GLOBAL_VARIABLE_GUID as
> default GUID: {8be4df61-93ca-11d2-aa0d-00e098032b8c}).
> If --set is specified, the version is set to a variable.
> 
> -s, --set=VARNAME       Assign return value to variable VARNAME.
> -g, --guid-var=GUIDVARNAME   Use the content of GUIDVARNAME as GUID

Per comment above, just use --guid=GUID and change the description

> -x, --hexdump           Display EFI variable content in hex mode
> -v, --verbose           Display more info about EFI variable.
> -h, --help              Display this help and exit.
> -u, --usage             Display the usage of this command and exit.
> 
> Usage: efivar_set [OPTIONS] VAR VALUE
> 
> Write VALUE to EFI variable VAR (if not specified, use 
> EFI_GLOBAL_VARIABLE_GUID
> as default GUID:{8be4df61-93ca-11d2-aa0d-00e098032b8c}).
> If --set is specified, the same EFI variable value is set to a variable.
> If --non-volatile is specified, the non-volatile EFI attribute is set.
> 
> Note: an existing EFI variable can be changed only maintainig the same
> volatile/non-volatile attribute.
> 
> -s, --set=VARNAME       Assign the same EFI variable value to variable
> VARNAME.
> -g, --guid-var=GUIDVARNAME   Use the content of GUIDVARNAME as GUID
> -n, --non-volatile      Set non-volatile attribute.
> -v, --verbose           Display more info about EFI variable
> -h, --help              Display this help and exit.
> -u, --usage             Display the usage of this command and exit.
> 
> Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
> ---
>  docs/grub.texi                          | 137 +++++++++++++++
>  grub-core/Makefile.core.def             |   5 +
>  grub-core/commands/efi/efivar_get_set.c | 288 
> ++++++++++++++++++++++++++++++++
>  grub-core/kern/efi/efi.c                |  57 +++++++
>  include/grub/efi/efi.h                  |   4 +
>  5 files changed, 491 insertions(+)
>  create mode 100644 grub-core/commands/efi/efivar_get_set.c
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index d6408d2..ee59079 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3978,6 +3978,8 @@ you forget a command, you can run the command 
> @command{help}
>  * distrust::                    Remove a pubkey from trusted keys
>  * drivemap::                    Map a drive to another
>  * echo::                        Display a line of text
> +* efivar_get::                  Get EFI variable
> +* efivar_set::                  Set EFI variable
>  * eval::                        Evaluate agruments as GRUB commands
>  * export::                      Export an environment variable
>  * false::                       Do nothing, unsuccessfully
> @@ -4397,6 +4399,141 @@ character will print that character.
>  @end deffn
>  
>  
> +@node efivar_get
> +@subsection efivar_get
> +@deffn Command efivar_get [@option{--set} var] [@option{--guid-var} 
> guidvarname] [@option{--hexdump}] [@option{--verbose}] efivar
> +Get and display an EFI variable @var{efivar} in ASCII or hex+ASCII format.
> +
> +The EFI GUID can be loaded from an environment variable, using the option
> +@option{--guid-var @var{guidvarname}}. If this option is not used and
> +consequently no GUID is specified, the GUID:
> +
> +
> +@center EFI_GLOBAL_VARIABLE_GUID = 8be4df61-93ca-11d2-aa0d-00e098032b8c
> +
> +is implicitly assumed.
> +
> +If the option @option{--set @var{var}} is specified, store the EFI variable 
> to
> +the environment variable @var{var} instead of printing it.
> +
> +If the option @option{--hexdump} is specified, the output is in hexdecimal
> +format.
> +
> +The option @option{--verbose} improve the data information output.
> +
> +@emph{Example:} read the EFI variable:
> +
> +@center @samp{PlatformInfo-19ad5244-fd6b-4e5c-826a-414646d6da6a}
> +
> +@smallexample
> +grub> set guid_used=19ad5244-fd6b-4e5c-826a-414646d6da6a
> +grub> efivar_get -g guid_used -xv PlatformInfo
> +PlatformInfo-19ad5244-fd6b-4e5c-826a-414646d6da6a :
> +
> +0000 - 00 00 e2 00 00 00 00 00 00 00 00 00 f0 5a 0d 00   |.............Z..|
> +0010 - 00 00 00 00 00 00 00 00 00 a8 ff ff ff df 00 00   |................|
> +0020 - 00 00 0f 00 00 00 ff ff ff ff 0f 00 00 00 00 00   |................|
> +0030 - 00 e0 00 00 00 00 00 00 00 04 00 01 24 00 00 00   |............$...|
> +0040 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0050 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0060 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0070 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0080 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0090 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 a8 00   |................|
> +00a0 - 00 00 a8 00 00 00 00 00 00 c0 a0 ef 7a 00 80 01   |............z...|
> +00b0 - 00 00 00 f9 ff ff 8b 0d 9c f9 ff ff c1 e9 02 00   |................|
> +00c0 - 00 00 00 0f 00 00 00 00 00 00 00 10 00 00 00 d0   |................|
> +00d0 - e6 80 eb 80 05 01 00 00 00 00 00 00 00 00 00 00   |................|
> +00e0 - 00 00 00 00 00 00 00 00 01 86 80 34 12 00 00 49   |...........4...I|
> +00f0 - 4e 54 45 4c 20 20 20 45 44 4b 32 20 20 20 20 00   |NTEL   EDK2    .|
> +0100 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0110 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0120 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0130 - 00 00 00 00 00 00 00 00 00 00 30 32 b7 7a c8 3a   |..........02.z.:|
> +0140 - b7 7a b0 34 b7 7a 88 3e b7 7a 00 00 00 00 00 00   |.z.4.z.>.z......|
> +0150 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0160 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0170 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0180 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +0190 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +01a0 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +01b0 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +01c0 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +01d0 - 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00   |................|
> +01e0 - 00 00 00 00 00 00 00 00 00 fc 8b 3d 98            |...........=.   |
> +grub>
> +@end smallexample
> +
> +
> +@emph{Note:} in Linux, the dump of any EFI var in
> +@file{/sys/firmware/efi/efivars} shows, before the variable data, also the
> +4 bytes of the variable attributes. In GRUB, instead, a dump of any EFI 
> variable
> +shows the variable data only.
> +
> +@end deffn
> +
> +
> +@node efivar_set
> +@subsection efivar_set
> +@deffn Command efivar_set [@option{--set} var] [@option{--guid-var} 
> guidvarname] [@option{--non-volatile}] [@option{--verbose}] efivar value
> +Set an EFI variable @var{efivar} with the value @var{value} in ASCII format.
> +
> +The EFI GUID can be loaded from an environment variable, using the option
> +@option{--guid-var @var{guidvarname}}. If this option is not used and 
> consequently
> +no GUID is specified, the GUID:
> +
> +
> +@center EFI_GLOBAL_VARIABLE_GUID = 8be4df61-93ca-11d2-aa0d-00e098032b8c
> +
> +is implicitly assumed.
> +
> +About the EFI variable attributes, the following attributes are set:
> +@itemize @bullet
> +@item
> +GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS
> +@item
> +GRUB_EFI_VARIABLE_RUNTIME_ACCESS
> +@end itemize
> +If the option @option{--non-volatile} is give, the variable is also set with
> +the attribute:
> +@itemize @bullet
> +@item
> +GRUB_EFI_VARIABLE_NON_VOLATILE
> +@end itemize
> +In this way the variable will be stored in the board NVRAM.

s/./ and preserved across reboots./

> +
> +If the option @option{--set @var{var}} is specified, in addition to set the 
> EFI
> +variable with the value @var{value}, also store this value to the environment
> +variable @var{var}.
> +
> +The option @option{--verbose} improve the data information output.
> +
> +@emph{Example:} set the EFI variable @var{Configuration} to the ASCII value
> +@samp{Board_special version 1.0.0}, with the following
> +custom GUID:
> +
> +@center @samp{Configuration-a87cb185-864c-4683-a93e-3666ad3cf6b6}
> +
> +@smallexample
> +grub> set guid_used=a87cb185-864c-4683-a93e-3666ad3cf6b6
> +grub> efivar_set -g guid_used Configuration "Board_special version 1.0.0"
> +grub> efivar_get -g guid_used -xv Configuration
> +Configuration-a87cb185-864c-4683-a93e-3666ad3cf6b6 :
> +
> +0000 - 42 6f 61 72 64 5f 73 70 65 63 69 61 6c 20 76 65   |Board_special ve|
> +0010 - 72 73 69 6f 6e 20 31 2e 30 2e 30 00               |rsion 1.0.0.    |
> +grub>
> +@end smallexample
> +
> +
> +@emph{Note:} in Linux, the dump of any EFI var in
> +@file{/sys/firmware/efi/efivars} shows, before the variable data, also the
> +4 bytes of the variable attributes. In GRUB, instead, a dump of any EFI 
> variable
> +shows the variable data only.
> +
> +@end deffn
> +
> +
>  @node eval
>  @subsection eval
>  
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 48b82e7..12535ba 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2534,3 +2534,8 @@ module = {
>    common = commands/i386/wrmsr.c;
>    enable = x86;
>  };
> +
> +module = {
> +  name = efivar_get_set;
> +  common = commands/efi/efivar_get_set.c;

Perhaps just call this efivars.

> +};
> diff --git a/grub-core/commands/efi/efivar_get_set.c 
> b/grub-core/commands/efi/efivar_get_set.c
> new file mode 100644
> index 0000000..f1d1cd9
> --- /dev/null
> +++ b/grub-core/commands/efi/efivar_get_set.c
> @@ -0,0 +1,288 @@
> +/* efivar_get_set.c - command to get/set an EFI variable  */

s/command/commands/

> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2020  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 <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/dl.h>
> +#include <grub/command.h>
> +#include <grub/misc.h>
> +#include <grub/mm.h>
> +#include <grub/env.h>
> +#include <grub/extcmd.h>
> +#include <grub/i18n.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static const struct grub_arg_option options_efivar_rd[] =
> +{
> +  { "set", 's', 0, N_("Assign return value to variable VARNAME."),
> +    N_("VARNAME"), ARG_TYPE_STRING },
> +  { "guid-var", 'g', 0, N_("Use the content of GUIDVARNAME as GUID"),
> +    N_("GUIDVARNAME"), ARG_TYPE_STRING },

It would be very useful to have another options, say --convert-utf8 or
-c for short, to convert the value of the EFI variable from UTF16 to
UTF8. This should probably not be the default because variable need not
contain strings.

> +  {"hexdump", 'x', 0, N_("Display EFI variable content in hex mode"), 0, 0},
> +  {"verbose", 'v', 0, N_("Display more info about EFI variable."), 0, 0},
> +  { 0, 0, 0, 0, 0, 0 }
> +};
> +
> +static const struct grub_arg_option options_efivar_wr[] =
> +{
> +  { "set", 's', 0, N_("Assign the same EFI variable value to variable 
> VARNAME."),
> +    N_("VARNAME"), ARG_TYPE_STRING },
> +  { "guid-var", 'g', 0, N_("Use the content of GUIDVARNAME as GUID"),
> +    N_("GUIDVARNAME"), ARG_TYPE_STRING },

Ditto.

Also, by default variables should be GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS
but if -r or --runtime-access is passed, then use
GRUB_EFI_VARIABLE_RUNTIME_ACCESS as well.

> +  { "non-volatile", 'n', 0, N_("Set non-volatile attribute."), 0, 
> ARG_TYPE_NONE },
> +  { "verbose", 'v', 0, N_("Display more info about EFI variable"), 0, 0},
> +  { 0, 0, 0, 0, 0, 0 }
> +};
> +
> +static void
> +dump_efi_var_hex (void *efi_var, grub_size_t efi_var_data_size,
> +                  grub_uint8_t verbose)
> +{
> +  #define DUMP_EFI_VAR_HEX_ROW_LEN  16

This should probably go near the top of the file after #include.

> +  grub_uint32_t i, j;
> +  grub_uint8_t *val = efi_var;
> +
> +  grub_printf ("\n");
> +  for (i = 0; i <= (efi_var_data_size / DUMP_EFI_VAR_HEX_ROW_LEN) ; i++)
> +    {
> +      if (verbose)
> +        grub_printf ("%04x - ", i * DUMP_EFI_VAR_HEX_ROW_LEN);
> +
> +      /* Print numerical values */
> +      for (j = 0; j < DUMP_EFI_VAR_HEX_ROW_LEN; j++)
> +        {
> +          if ((i * DUMP_EFI_VAR_HEX_ROW_LEN + j) < efi_var_data_size)
> +            {
> +              grub_printf ("%02x", (unsigned int) val[i * 
> DUMP_EFI_VAR_HEX_ROW_LEN + j]);
> +            }
> +          else
> +            grub_printf ("  ");
> +          if (verbose)
> +            grub_printf (" ");
> +        }
> +
> +      /* If verbose is active, print ASCII chars */
> +      if (verbose)
> +        {
> +          grub_printf ("  |");
> +          for (j = 0; j < DUMP_EFI_VAR_HEX_ROW_LEN; j++)
> +            {
> +              if ((i * DUMP_EFI_VAR_HEX_ROW_LEN + j) < efi_var_data_size)
> +                {
> +                  if (grub_isprint (val[i * DUMP_EFI_VAR_HEX_ROW_LEN + j]))
> +                    grub_printf ("%c", (unsigned int) val[i * 
> DUMP_EFI_VAR_HEX_ROW_LEN + j]);
> +                  else
> +                    grub_printf(".");
> +                }
> +              else
> +                grub_printf(" ");
> +            }
> +          grub_printf ("|\n");
> +        }
> +    }
> +}
> +
> +static void
> +dump_efi_guid (char *efi_var_name, grub_efi_guid_t guid)
> +{
> +  grub_printf ("%s-%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x :\n",
> +               efi_var_name,
> +               (unsigned) guid.data1,
> +               (unsigned) guid.data2,
> +               (unsigned) guid.data3,
> +               (unsigned) guid.data4[0],
> +               (unsigned) guid.data4[1],
> +               (unsigned) guid.data4[2],
> +               (unsigned) guid.data4[3],
> +               (unsigned) guid.data4[4],
> +               (unsigned) guid.data4[5],
> +               (unsigned) guid.data4[6],
> +               (unsigned) guid.data4[7]);
> +}
> +
> +static grub_err_t
> +grub_cmd_efivar_get (grub_extcmd_context_t ctxt, int argc, char **args)
> +{
> +  void *efi_var = NULL;
> +  grub_size_t efi_var_data_size = 0;
> +  char *buf = 0;
> +  grub_efi_guid_t guid = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> +  const char *guid_str;
> +  grub_err_t err;
> +
> +  /* One argument required */
> +  if (argc != 1)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       N_("one argument required: EFI variable name"));
> +
> +  /* Get GUID from env variable */
> +  if (ctxt->state[1].set)
> +    {
> +      guid_str = grub_env_get (ctxt->state[1].arg);
> +      if (guid_str)
> +        {
> +          err = grub_efi_str_to_guid (guid_str, &guid);
> +          if (err)
> +            return err;
> +        }
> +      else
> +        return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                             N_("GUID environment variable not found"));
> +    }
> +  else
> +    grub_printf ("No custom GUID provided, use EFI_GLOBAL_VARIABLE_GUID.\n");

This should be a grub_dprintf() if anything.

> +
> +  /* Get EFI variable */
> +  efi_var = grub_efi_get_variable (args[0], &guid, &efi_var_data_size);
> +  if (!efi_var || !efi_var_data_size)
> +    return grub_error (GRUB_ERR_READ_ERROR, N_("cannot read EFI variable"));
> +
> +  /* Get EFI var data string */
> +  buf = grub_malloc (efi_var_data_size * 2 + 1);
> +  if (!buf)
> +    return grub_errno;
> +
> +  /* Copy EFI variable data */
> +  grub_strncpy (buf, efi_var, efi_var_data_size);
> +
> +  /* Set env variable required? */
> +  if (ctxt->state[0].set)
> +    grub_env_set (ctxt->state[0].arg, buf);

If the data in buf is a UTF16 string (what UEFI uses for strings), then
this will probably always set the environment variable to the empty
string because the first byte will be '\0'.

> +  else
> +    {
> +      /* Verbose ? */
> +      if (ctxt->state[3].set)
> +        {
> +          /* Print GUID */
> +          dump_efi_guid (args[0], guid);
> +
> +          /* Hexdump ? */
> +          if (ctxt->state[2].set)
> +            dump_efi_var_hex(efi_var, efi_var_data_size, 1);
> +          else
> +            grub_printf ("%s", buf);
> +        }
> +      else
> +        /* Hexdump ? */
> +        if (ctxt->state[2].set)
> +          dump_efi_var_hex(efi_var, efi_var_data_size, 0);
> +        else
> +          grub_printf ("%s", buf);
> +    }
> +
> +  grub_free (buf);
> +
> +  return 0;
> +}
> +
> +static grub_err_t
> +grub_cmd_efivar_set (grub_extcmd_context_t ctxt, int argc, char **args)
> +{
> +  grub_err_t efi_err;
> +  grub_efi_guid_t guid = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> +  const char *guid_str;
> +  grub_efi_boolean_t non_volatile = 0;
> +  grub_err_t err;
> +
> +  /* Two arguments required */
> +  if (argc != 2)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       N_("two arguments required: EFI variable name and 
> value"));
> +
> +  /* Get GUID from env variable */
> +  if (ctxt->state[1].set)
> +    {
> +      guid_str = grub_env_get (ctxt->state[1].arg);
> +      if (guid_str)
> +        {
> +          err = grub_efi_str_to_guid (guid_str, &guid);
> +          if (err)
> +            return err;
> +        }
> +      else
> +        return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                           N_("GUID environment variable not found"));
> +    }
> +  else
> +    grub_printf ("No custom GUID provided, use EFI_GLOBAL_VARIABLE_GUID.\n");
> +
> +  /* Volatile or non-volatile ? */
> +  if (ctxt->state[2].set)
> +    non_volatile = 1;
> +
> +  /* Set EFI variable */
> +  efi_err = grub_efi_set_variable (args[0], &guid, args[1],
> +                                   (grub_size_t) grub_strlen(args[1]) + 1,
> +                                   non_volatile);
> +  if (efi_err != GRUB_ERR_NONE)
> +    return grub_error (GRUB_ERR_WRITE_ERROR, N_("cannot write EFI 
> variable"));
> +
> +  /* Set env variable required? */
> +  if (ctxt->state[0].set)
> +    grub_env_set (ctxt->state[0].arg, args[1]);
> +
> +  /* Verbose ? */
> +  if (ctxt->state[3].set)
> +    {
> +      /* Print GUID */
> +      dump_efi_guid (args[0], guid);
> +
> +      grub_printf("%s EFI variable: %s set to value: %s\n",
> +                non_volatile ? "Non-volatile" : "volatile",
> +                args[0], args[1]);
> +    }
> +  return 0;
> +}
> +
> +static grub_extcmd_t cmd_rd_efi_var, cmd_wr_efi_var;
> +
> +GRUB_MOD_INIT(version)
> +{
> +  cmd_rd_efi_var = grub_register_extcmd ("efivar_get",
> +                                         grub_cmd_efivar_get, 0,
> +                                         N_("[OPTIONS] VAR"),

Put the actual options supported here.

> +                                         N_("\nRead EFI VAR variable (if not 
> specified, use "
> +                                            "EFI_GLOBAL_VARIABLE_GUID as 
> default GUID: "
> +                                            
> "{8be4df61-93ca-11d2-aa0d-00e098032b8c}).\n"
> +                                            "If --set is specified, the 
> version is set to a variable."
> +                                           ),
> +                                         options_efivar_rd);
> +
> +  cmd_wr_efi_var = grub_register_extcmd ("efivar_set",
> +                                         grub_cmd_efivar_set, 0,
> +                                         N_("[OPTIONS] VAR VALUE"),

Ditto.

> +                                         N_("\nWrite VALUE to EFI variable 
> VAR (if not specified, use "
> +                                            "EFI_GLOBAL_VARIABLE_GUID as 
> default GUID:"
> +                                            
> "{8be4df61-93ca-11d2-aa0d-00e098032b8c}).\n"
> +                                            "If --set is specified, the same 
> EFI variable value "
> +                                            "is set to a variable.\n"
> +                                            "If --non-volatile is specified, 
> the non-volatile "
> +                                            "EFI attribute is set.\n"
> +                                            "\nNote: an existing EFI 
> variable can be changed only "
> +                                            "maintainig the same 
> volatile/non-volatile attribute."
> +                                           ),
> +                                         options_efivar_wr);
> +}
> +
> +GRUB_MOD_FINI(version)
> +{
> +     grub_unregister_extcmd (cmd_rd_efi_var);
> +     grub_unregister_extcmd (cmd_wr_efi_var);
> +}
> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> index dcb09b2..5d7a8ed 100644
> --- a/grub-core/kern/efi/efi.c
> +++ b/grub-core/kern/efi/efi.c
> @@ -960,3 +960,60 @@ grub_efi_compare_device_paths (const 
> grub_efi_device_path_t *dp1,
>  
>    return 0;
>  }
> +
> +/* Convert a GUID from string format to grub_efi_guid_t format.
> + * For example:
> + *
> + * 8be4df61-93ca-11d2-aa0d-00e098032b8c -->
> + *
> + * --> { 0x8BE4DF61, 0x93CA, 0x11d2, \
> + *       { 0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C }}
> +*/

Multiline comment not quite right, should be

/*
 * ....
 */

> +grub_err_t
> +grub_efi_str_to_guid(const char *guid_str, grub_efi_guid_t *guid)
> +{
> +  char guid_str_tmp[32];
> +  unsigned int i;
> +  union
> +    {
> +      grub_uint64_t octet_64;
> +      grub_uint8_t octet_8[8];
> +    } guid_octet;
> +
> +  /* Check string size */
> +  if (grub_strlen(guid_str) != EFI_GUID_STRING_LENGTH)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       N_("GUID string must be %d chars long\n"),
> +                       EFI_GUID_STRING_LENGTH);
> +

Should check that guid is not NULL.

> +  /* Remove dashes for string check */
> +  grub_snprintf (&guid_str_tmp[0],   9, "%s", guid_str);
> +  grub_snprintf (&guid_str_tmp[8],   5, "%s", guid_str + 9);
> +  grub_snprintf (&guid_str_tmp[12],  5, "%s", guid_str + 14);
> +  grub_snprintf (&guid_str_tmp[16],  5, "%s", guid_str + 19);
> +  grub_snprintf (&guid_str_tmp[20], 13, "%s", guid_str + 24);
> +
> +  /* Check for hex digit only */
> +  for (i = 0; i < grub_strlen (guid_str_tmp); i++)
> +    {
> +      guid_str_tmp[i] = grub_tolower(guid_str_tmp[i]);
> +
> +      if (!(grub_isxdigit (guid_str_tmp[i]))) {
> +
> +         return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                            N_("GUID string must be like the following: 
> 8be4df61-93ca-11d2-aa0d-00e098032b8c"));
> +      }
> +    }
> +
> +  /* Convert from string to GUID */
> +  guid->data1 = grub_strtoul (&guid_str[0],  NULL, 16);
> +  guid->data2 = grub_strtoul (&guid_str[9],  NULL, 16);
> +  guid->data3 = grub_strtoul (&guid_str[14], NULL, 16);
> +  guid_octet.octet_64 = grub_strtoull (&guid_str_tmp[16], NULL, 16);
> +  for (i = 0; i <= 7; i++)
> +    {
> +      guid->data4[i] = guid_octet.octet_8[7 - i];
> +    }

This function seems like its doing a lot more work than necessary. I
think it would be better to loop over every byte of guid_str, skipping
dashes. On each non-dash, error if its not a hex digit. Convert the
hexdigit character to a number and put it directly into the guid as
appropriate, keeping track that the appropriate number of hexdigits
have been seen and error otherwise. I don't we need to care if the
dashes are in the right position or there at all.

Glenn

> +
> +  return GRUB_ERR_NONE;
> +}
> \ No newline at end of file
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index 9b04150..dc87cc2 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -24,6 +24,8 @@
>  #include <grub/dl.h>
>  #include <grub/efi/api.h>
>  
> +#define EFI_GUID_STRING_LENGTH 36
> +
>  /* Functions.  */
>  void *EXPORT_FUNC(grub_efi_locate_protocol) (grub_efi_guid_t *protocol,
>                                            void *registration);
> @@ -90,6 +92,8 @@ EXPORT_FUNC (grub_efi_compare_device_paths) (const 
> grub_efi_device_path_t *dp1,
>  extern void (*EXPORT_VAR(grub_efi_net_config)) (grub_efi_handle_t hnd, 
>                                               char **device,
>                                               char **path);
> +grub_err_t
> +EXPORT_FUNC(grub_efi_str_to_guid)(const char *guid_str, grub_efi_guid_t 
> *guid);
>  
>  #if defined(__arm__) || defined(__aarch64__) || defined(__riscv)
>  void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);



reply via email to

[Prev in Thread] Current Thread [Next in Thread]