All of lore.kernel.org
 help / color / mirror / Atom feed
* Grub get and set efi variables
@ 2015-11-03 19:39 Mat Troi
  2015-11-03 20:12 ` SevenBits
  0 siblings, 1 reply; 28+ messages in thread
From: Mat Troi @ 2015-11-03 19:39 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

Hi,

Searching through google, I see there was an email thread to add a patch
for getting and setting efi variable in GRUB2.
https://lists.gnu.org/archive/html/grub-devel/2013-11/msg00328.html

However, looking at the tree, it doesn't look like this patch was added, am
I missing something?  Does anyone know if we have command to get/set efi
variables in GRUB2?
http://git.savannah.gnu.org/gitweb/?p=grub.git;a=tree;f=grub-core/commands/efi;h=005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=HEAD

Thanks,
Mat

[-- Attachment #2: Type: text/html, Size: 834 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-03 19:39 Grub get and set efi variables Mat Troi
@ 2015-11-03 20:12 ` SevenBits
  2015-11-03 23:05   ` Mat Troi
  0 siblings, 1 reply; 28+ messages in thread
From: SevenBits @ 2015-11-03 20:12 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

On Tuesday, November 3, 2015, Mat Troi <mattroisang@gmail.com> wrote:

> Hi,
>
> Searching through google, I see there was an email thread to add a patch
> for getting and setting efi variable in GRUB2.
> https://lists.gnu.org/archive/html/grub-devel/2013-11/msg00328.html
>
> However, looking at the tree, it doesn't look like this patch was added,
> am I missing something?  Does anyone know if we have command to get/set efi
> variables in GRUB2?
>
>
> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=tree;f=grub-core/commands/efi;h=005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=HEAD
>
>
I'm the author of that patch. No, it was never merged into the tree. As far
as I know, there is no equivalent functionality; GRUB does not support this
feature.


> Thanks,
> Mat
>

[-- Attachment #2: Type: text/html, Size: 1466 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-03 20:12 ` SevenBits
@ 2015-11-03 23:05   ` Mat Troi
  2015-11-04  5:38     ` Andrei Borzenkov
  0 siblings, 1 reply; 28+ messages in thread
From: Mat Troi @ 2015-11-03 23:05 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

Hi SevenBits,

Thanks for letting me know.  Out of curiosity, any particular reason why
this patch did not get merged?  It looks to be a useful feature.

Thanks,
Mat

On Tue, Nov 3, 2015 at 12:12 PM, SevenBits <sevenbitstech@gmail.com> wrote:

> On Tuesday, November 3, 2015, Mat Troi <mattroisang@gmail.com> wrote:
>
>> Hi,
>>
>> Searching through google, I see there was an email thread to add a patch
>> for getting and setting efi variable in GRUB2.
>> https://lists.gnu.org/archive/html/grub-devel/2013-11/msg00328.html
>>
>> However, looking at the tree, it doesn't look like this patch was added,
>> am I missing something?  Does anyone know if we have command to get/set efi
>> variables in GRUB2?
>>
>>
>> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=tree;f=grub-core/commands/efi;h=005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=HEAD
>>
>>
> I'm the author of that patch. No, it was never merged into the tree. As
> far as I know, there is no equivalent functionality; GRUB does not support
> this feature.
>
>
>> Thanks,
>> Mat
>>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>

[-- Attachment #2: Type: text/html, Size: 2406 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-03 23:05   ` Mat Troi
@ 2015-11-04  5:38     ` Andrei Borzenkov
  2015-11-05 18:25       ` SevenBits
  0 siblings, 1 reply; 28+ messages in thread
From: Andrei Borzenkov @ 2015-11-04  5:38 UTC (permalink / raw)
  To: The development of GNU GRUB

04.11.2015 02:05, Mat Troi пишет:
> Hi SevenBits,
>
> Thanks for letting me know.  Out of curiosity, any particular reason why
> this patch did not get merged?  It looks to be a useful feature.
>

First, this patch was reverse patch :)

I am not convinced making it easy to set EFI variable from within GRUB 
is good thing, because there are known reports about systems rendered 
unbootable by writing too much into EFI flash. What is your use case 
that absolutely requires setting EFI variables? How are you going to 
implement it on other platforms?

Reading does not harm and may be useful, but then it should provide 
generic interface to access arbitrary vendor namespace, not only EFI 
global variables, and handle arbitrary binary data, even if initial 
implementation handles only subset of them. Once someone starts to use 
it changing it will be much more difficult.

May be it should take hints how to interpret variable values, or have 
format option akin to printf.

> Thanks,
> Mat
>
> On Tue, Nov 3, 2015 at 12:12 PM, SevenBits <sevenbitstech@gmail.com> wrote:
>
>> On Tuesday, November 3, 2015, Mat Troi <mattroisang@gmail.com> wrote:
>>
>>> Hi,
>>>
>>> Searching through google, I see there was an email thread to add a patch
>>> for getting and setting efi variable in GRUB2.
>>> https://lists.gnu.org/archive/html/grub-devel/2013-11/msg00328.html
>>>
>>> However, looking at the tree, it doesn't look like this patch was added,
>>> am I missing something?  Does anyone know if we have command to get/set efi
>>> variables in GRUB2?
>>>
>>>
>>> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=tree;f=grub-core/commands/efi;h=005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=HEAD
>>>
>>>
>> I'm the author of that patch. No, it was never merged into the tree. As
>> far as I know, there is no equivalent functionality; GRUB does not support
>> this feature.
>>
>>
>>> Thanks,
>>> Mat
>>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>>
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-04  5:38     ` Andrei Borzenkov
@ 2015-11-05 18:25       ` SevenBits
  2015-11-06  2:00         ` Ignat Korchagin
  0 siblings, 1 reply; 28+ messages in thread
From: SevenBits @ 2015-11-05 18:25 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 3630 bytes --]

On Wednesday, November 4, 2015, Andrei Borzenkov <arvidjaar@gmail.com>
wrote:

> 04.11.2015 02:05, Mat Troi пишет:
>
>> Hi SevenBits,
>>
>> Thanks for letting me know.  Out of curiosity, any particular reason why
>> this patch did not get merged?  It looks to be a useful feature.
>>
>>
> First, this patch was reverse patch :)


Yeah, I think I had accidentally passed the arguments to the diff command
in the wrong order.


>
> I am not convinced making it easy to set EFI variable from within GRUB is
> good thing, because there are known reports about systems rendered
> unbootable by writing too much into EFI flash. What is your use case that
> absolutely requires setting EFI variables? How are you going to implement
> it on other platforms?


I should probably note that I wrote this patch for a specific project of
mine which required the ability to read UEFI variables. I added in write
functionality for good measure because I could. But I agree, this would
only encourage tinkering and users messing with their systems and
potentially bricking it, which would of course be blamed on GRUB.


>
> Reading does not harm and may be useful, but then it should provide
> generic interface to access arbitrary vendor namespace, not only EFI global
> variables, and handle arbitrary binary data, even if initial implementation
> handles only subset of them. Once someone starts to use it changing it will
> be much more difficult.
>
> Maybe it should take hints how to interpret variable values, or have
> format option akin to printf.


I would be happy to resume work on this, and debase it on the current code,
if GRUB has a clear need for such functionality. I would prefer to have it
be clear what the patch should consist of, though.

A key question would be how to e.g. handle arbitrary data stored in
variables if it is not something easily represent able like a string. Right
now, the patch stores it's value into an environment variable specified by
the user. Unless the powers that be think otherwise, I think this is the
best way to go.


>
> Thanks,
>> Mat
>>
>> On Tue, Nov 3, 2015 at 12:12 PM, SevenBits <sevenbitstech@gmail.com>
>> wrote:
>>
>> On Tuesday, November 3, 2015, Mat Troi <mattroisang@gmail.com> wrote:
>>>
>>> Hi,
>>>>
>>>> Searching through google, I see there was an email thread to add a patch
>>>> for getting and setting efi variable in GRUB2.
>>>> https://lists.gnu.org/archive/html/grub-devel/2013-11/msg00328.html
>>>>
>>>> However, looking at the tree, it doesn't look like this patch was added,
>>>> am I missing something?  Does anyone know if we have command to get/set
>>>> efi
>>>> variables in GRUB2?
>>>>
>>>>
>>>>
>>>> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=tree;f=grub-core/commands/efi;h=005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=HEAD
>>>>
>>>>
>>>> I'm the author of that patch. No, it was never merged into the tree. As
>>> far as I know, there is no equivalent functionality; GRUB does not
>>> support
>>> this feature.
>>>
>>>
>>> Thanks,
>>>> Mat
>>>>
>>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>>
>>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 5385 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-05 18:25       ` SevenBits
@ 2015-11-06  2:00         ` Ignat Korchagin
  2015-11-11 18:09           ` Andrei Borzenkov
  0 siblings, 1 reply; 28+ messages in thread
From: Ignat Korchagin @ 2015-11-06  2:00 UTC (permalink / raw)
  To: The development of GNU GRUB

Actually, I submitted similar patch recently as well. It provides read
function for variables and accepts a hint on how to process them. The
original patch is here:
https://lists.gnu.org/archive/html/grub-devel/2015-09/msg00043.html.

Probably, I forgot to enable plain-text mode, so it got there as a
binary attachment. I will repeat it here for convenience.

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 9764cd2..49fa3ec 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -728,6 +728,12 @@ module = {
 };

 module = {
+  name = efivar;
+  efi = commands/efi/efivar.c;
+  enable = efi;
+};
+
+module = {
   name = blocklist;
   common = commands/blocklist.c;
 };
diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
new file mode 100644
index 0000000..ca206eb
--- /dev/null
+++ b/grub-core/commands/efi/efivar.c
@@ -0,0 +1,146 @@
+/* efivar.c - Read EFI global variables. */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2015 Free Software Foundation, Inc.
+ *  Copyright (C) 2015 CloudFlare, 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/types.h>
+#include <grub/mm.h>
+#include <grub/misc.h>
+#include <grub/efi/api.h>
+#include <grub/efi/efi.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static const struct grub_arg_option options[] = {
+  {"type", 't', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR as
specific type (hex, uint8, string). Default: hex."), N_("TYPE"),
ARG_TYPE_STRING},
+  {0, 0, 0, 0, 0, 0}
+};
+
+enum efi_var_type
+  {
+    EFI_VAR_STRING = 0,
+    EFI_VAR_UINT8,
+    EFI_VAR_HEX,
+    EFI_VAR_INVALID = -1
+  };
+
+static enum efi_var_type
+parse_efi_var_type (const char *type)
+{
+  if (!grub_strncmp (type, "string", sizeof("string")))
+    return EFI_VAR_STRING;
+
+  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
+    return EFI_VAR_UINT8;
+
+  if (!grub_strncmp (type, "hex", sizeof("hex")))
+    return EFI_VAR_HEX;
+
+  return EFI_VAR_INVALID;
+}
+
+static grub_err_t
+grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
+  int argc, char **args)
+{
+  struct grub_arg_list *state = ctxt->state;
+  grub_err_t status;
+  void *efi_var = NULL;
+  grub_size_t efi_var_size = 0;
+  enum efi_var_type efi_type = EFI_VAR_HEX;
+  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  char *env_var = NULL;
+  grub_size_t i;
+
+  if (2 != argc)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
+
+  if (state[0].set)
+    efi_type = parse_efi_var_type (state[0].arg);
+
+  if (EFI_VAR_INVALID == efi_type)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid EFI variable type"));
+
+  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
+  if (!efi_var || !efi_var_size)
+    status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
+
+  switch (efi_type)
+  {
+    case EFI_VAR_STRING:
+      env_var = grub_malloc (efi_var_size + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      grub_memcpy(env_var, efi_var, efi_var_size);
+      env_var[efi_var_size] = '\0';
+      break;
+
+    case EFI_VAR_UINT8:
+      env_var = grub_malloc (4);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
+      break;
+
+    case EFI_VAR_HEX:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      for (i = 0; i < efi_var_size; i++)
+        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
*)efi_var)[i]);
+      break;
+
+    default:
+      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
in module?)"));
+  }
+
+  status = grub_env_set (args[1], env_var);
+
+  if (env_var)
+    grub_free (env_var);
+
+  if (efi_var)
+    grub_free (efi_var);
+
+  return status;
+}
+
+static grub_extcmd_t cmd = NULL;
+
+GRUB_MOD_INIT (efivar)
+{
+  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
N_("[-t TYPE] EFI_VAR ENV_VAR"),
+ N_("Read EFI variable and put its contents in environment
variable."), options);
+}
+
+GRUB_MOD_FINI (efivar)
+{
+  if (cmd)
+    grub_unregister_extcmd (cmd);
+}


On Thu, Nov 5, 2015 at 10:25 AM, SevenBits <sevenbitstech@gmail.com> wrote:
> On Wednesday, November 4, 2015, Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
>>
>> 04.11.2015 02:05, Mat Troi пишет:
>>>
>>> Hi SevenBits,
>>>
>>> Thanks for letting me know.  Out of curiosity, any particular reason why
>>> this patch did not get merged?  It looks to be a useful feature.
>>>
>>
>> First, this patch was reverse patch :)
>
>
> Yeah, I think I had accidentally passed the arguments to the diff command in
> the wrong order.
>
>>
>>
>> I am not convinced making it easy to set EFI variable from within GRUB is
>> good thing, because there are known reports about systems rendered
>> unbootable by writing too much into EFI flash. What is your use case that
>> absolutely requires setting EFI variables? How are you going to implement it
>> on other platforms?
>
>
> I should probably note that I wrote this patch for a specific project of
> mine which required the ability to read UEFI variables. I added in write
> functionality for good measure because I could. But I agree, this would only
> encourage tinkering and users messing with their systems and potentially
> bricking it, which would of course be blamed on GRUB.
>
>>
>>
>> Reading does not harm and may be useful, but then it should provide
>> generic interface to access arbitrary vendor namespace, not only EFI global
>> variables, and handle arbitrary binary data, even if initial implementation
>> handles only subset of them. Once someone starts to use it changing it will
>> be much more difficult.
>>
>> Maybe it should take hints how to interpret variable values, or have
>> format option akin to printf.
>
>
> I would be happy to resume work on this, and debase it on the current code,
> if GRUB has a clear need for such functionality. I would prefer to have it
> be clear what the patch should consist of, though.
>
> A key question would be how to e.g. handle arbitrary data stored in
> variables if it is not something easily represent able like a string. Right
> now, the patch stores it's value into an environment variable specified by
> the user. Unless the powers that be think otherwise, I think this is the
> best way to go.
>
>>
>>
>>> Thanks,
>>> Mat
>>>
>>> On Tue, Nov 3, 2015 at 12:12 PM, SevenBits <sevenbitstech@gmail.com>
>>> wrote:
>>>
>>>> On Tuesday, November 3, 2015, Mat Troi <mattroisang@gmail.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Searching through google, I see there was an email thread to add a
>>>>> patch
>>>>> for getting and setting efi variable in GRUB2.
>>>>> https://lists.gnu.org/archive/html/grub-devel/2013-11/msg00328.html
>>>>>
>>>>> However, looking at the tree, it doesn't look like this patch was
>>>>> added,
>>>>> am I missing something?  Does anyone know if we have command to get/set
>>>>> efi
>>>>> variables in GRUB2?
>>>>>
>>>>>
>>>>>
>>>>> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=tree;f=grub-core/commands/efi;h=005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=HEAD
>>>>>
>>>>>
>>>> I'm the author of that patch. No, it was never merged into the tree. As
>>>> far as I know, there is no equivalent functionality; GRUB does not
>>>> support
>>>> this feature.
>>>>
>>>>
>>>>> Thanks,
>>>>> Mat
>>>>>
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>
>>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-06  2:00         ` Ignat Korchagin
@ 2015-11-11 18:09           ` Andrei Borzenkov
  2015-11-13 19:34             ` Ignat Korchagin
  0 siblings, 1 reply; 28+ messages in thread
From: Andrei Borzenkov @ 2015-11-11 18:09 UTC (permalink / raw)
  To: The development of GNU GRUB

06.11.2015 05:00, Ignat Korchagin пишет:
> Actually, I submitted similar patch recently as well. It provides read
> function for variables and accepts a hint on how to process them. The
> original patch is here:
> https://lists.gnu.org/archive/html/grub-devel/2015-09/msg00043.html.
>

Yes, I was intending to comment on it, sorry.

I am still unsure whether printf-like format specifier could be more 
flexible, but otherwise I like it in that it provides for future 
extensions. see comments below.

> Probably, I forgot to enable plain-text mode, so it got there as a
> binary attachment. I will repeat it here for convenience.
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 9764cd2..49fa3ec 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -728,6 +728,12 @@ module = {
>   };
>
>   module = {
> +  name = efivar;
> +  efi = commands/efi/efivar.c;
> +  enable = efi;
> +};
> +
> +module = {
>     name = blocklist;
>     common = commands/blocklist.c;
>   };
> diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
> new file mode 100644
> index 0000000..ca206eb
> --- /dev/null
> +++ b/grub-core/commands/efi/efivar.c
> @@ -0,0 +1,146 @@
> +/* efivar.c - Read EFI global variables. */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015 Free Software Foundation, Inc.
> + *  Copyright (C) 2015 CloudFlare, 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/types.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +#include <grub/extcmd.h>
> +#include <grub/env.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static const struct grub_arg_option options[] = {
> +  {"type", 't', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR as
> specific type (hex, uint8, string). Default: hex."), N_("TYPE"),
> ARG_TYPE_STRING},
> +  {0, 0, 0, 0, 0, 0}
> +};
> +
> +enum efi_var_type
> +  {
> +    EFI_VAR_STRING = 0,
> +    EFI_VAR_UINT8,
> +    EFI_VAR_HEX,
> +    EFI_VAR_INVALID = -1
> +  };
> +
> +static enum efi_var_type
> +parse_efi_var_type (const char *type)
> +{
> +  if (!grub_strncmp (type, "string", sizeof("string")))
> +    return EFI_VAR_STRING;
> +

I think this should be "ascii" or "utf8". "string" is too ambiguous in 
UEFI environment, it can also mean sequence of UCS-2 characters.

> +  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
> +    return EFI_VAR_UINT8;
> +
> +  if (!grub_strncmp (type, "hex", sizeof("hex")))
> +    return EFI_VAR_HEX;
> +
> +  return EFI_VAR_INVALID;
> +}
> +
> +static grub_err_t
> +grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
> +  int argc, char **args)
> +{
> +  struct grub_arg_list *state = ctxt->state;
> +  grub_err_t status;
> +  void *efi_var = NULL;
> +  grub_size_t efi_var_size = 0;
> +  enum efi_var_type efi_type = EFI_VAR_HEX;
> +  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> +  char *env_var = NULL;
> +  grub_size_t i;
> +
> +  if (2 != argc)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
> +

May be follow the suite and use "--set var". It could be useful to 
simply display variable on screen, like

efivar OsIndicators

or even

efivar --all

> +  if (state[0].set)
> +    efi_type = parse_efi_var_type (state[0].arg);
> +
> +  if (EFI_VAR_INVALID == efi_type)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid EFI variable type"));
> +
> +  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
> +  if (!efi_var || !efi_var_size)
> +    status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
> +

Should it distinguish between non-existent variable and failure to read 
variable? Is non-existent variable an error?

> +  switch (efi_type)
> +  {
> +    case EFI_VAR_STRING:
> +      env_var = grub_malloc (efi_var_size + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +      grub_memcpy(env_var, efi_var, efi_var_size);
> +      env_var[efi_var_size] = '\0';
> +      break;
> +
> +    case EFI_VAR_UINT8:
> +      env_var = grub_malloc (4);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
> +      break;
> +
> +    case EFI_VAR_HEX:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +      for (i = 0; i < efi_var_size; i++)
> +        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
> *)efi_var)[i]);
> +      break;
> +
> +    default:
> +      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
> in module?)"));
> +  }
> +
> +  status = grub_env_set (args[1], env_var);
> +
> +  if (env_var)
> +    grub_free (env_var);
> +
> +  if (efi_var)
> +    grub_free (efi_var);
> +
> +  return status;
> +}
> +
> +static grub_extcmd_t cmd = NULL;
> +
> +GRUB_MOD_INIT (efivar)
> +{
> +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
> N_("[-t TYPE] EFI_VAR ENV_VAR"),
> + N_("Read EFI variable and put its contents in environment
> variable."), options);
> +}
> +
> +GRUB_MOD_FINI (efivar)
> +{
> +  if (cmd)
> +    grub_unregister_extcmd (cmd);
> +}
>
>
> On Thu, Nov 5, 2015 at 10:25 AM, SevenBits <sevenbitstech@gmail.com> wrote:
>> On Wednesday, November 4, 2015, Andrei Borzenkov <arvidjaar@gmail.com>
>> wrote:
>>>
>>> 04.11.2015 02:05, Mat Troi пишет:
>>>>
>>>> Hi SevenBits,
>>>>
>>>> Thanks for letting me know.  Out of curiosity, any particular reason why
>>>> this patch did not get merged?  It looks to be a useful feature.
>>>>
>>>
>>> First, this patch was reverse patch :)
>>
>>
>> Yeah, I think I had accidentally passed the arguments to the diff command in
>> the wrong order.
>>
>>>
>>>
>>> I am not convinced making it easy to set EFI variable from within GRUB is
>>> good thing, because there are known reports about systems rendered
>>> unbootable by writing too much into EFI flash. What is your use case that
>>> absolutely requires setting EFI variables? How are you going to implement it
>>> on other platforms?
>>
>>
>> I should probably note that I wrote this patch for a specific project of
>> mine which required the ability to read UEFI variables. I added in write
>> functionality for good measure because I could. But I agree, this would only
>> encourage tinkering and users messing with their systems and potentially
>> bricking it, which would of course be blamed on GRUB.
>>
>>>
>>>
>>> Reading does not harm and may be useful, but then it should provide
>>> generic interface to access arbitrary vendor namespace, not only EFI global
>>> variables, and handle arbitrary binary data, even if initial implementation
>>> handles only subset of them. Once someone starts to use it changing it will
>>> be much more difficult.
>>>
>>> Maybe it should take hints how to interpret variable values, or have
>>> format option akin to printf.
>>
>>
>> I would be happy to resume work on this, and debase it on the current code,
>> if GRUB has a clear need for such functionality. I would prefer to have it
>> be clear what the patch should consist of, though.
>>
>> A key question would be how to e.g. handle arbitrary data stored in
>> variables if it is not something easily represent able like a string. Right
>> now, the patch stores it's value into an environment variable specified by
>> the user. Unless the powers that be think otherwise, I think this is the
>> best way to go.
>>
>>>
>>>
>>>> Thanks,
>>>> Mat
>>>>
>>>> On Tue, Nov 3, 2015 at 12:12 PM, SevenBits <sevenbitstech@gmail.com>
>>>> wrote:
>>>>
>>>>> On Tuesday, November 3, 2015, Mat Troi <mattroisang@gmail.com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Searching through google, I see there was an email thread to add a
>>>>>> patch
>>>>>> for getting and setting efi variable in GRUB2.
>>>>>> https://lists.gnu.org/archive/html/grub-devel/2013-11/msg00328.html
>>>>>>
>>>>>> However, looking at the tree, it doesn't look like this patch was
>>>>>> added,
>>>>>> am I missing something?  Does anyone know if we have command to get/set
>>>>>> efi
>>>>>> variables in GRUB2?
>>>>>>
>>>>>>
>>>>>>
>>>>>> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=tree;f=grub-core/commands/efi;h=005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=HEAD
>>>>>>
>>>>>>
>>>>> I'm the author of that patch. No, it was never merged into the tree. As
>>>>> far as I know, there is no equivalent functionality; GRUB does not
>>>>> support
>>>>> this feature.
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Mat
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Grub-devel mailing list
>>>>> Grub-devel@gnu.org
>>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>
>>>
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-11 18:09           ` Andrei Borzenkov
@ 2015-11-13 19:34             ` Ignat Korchagin
  2015-11-13 19:42               ` Ignat Korchagin
  0 siblings, 1 reply; 28+ messages in thread
From: Ignat Korchagin @ 2015-11-13 19:34 UTC (permalink / raw)
  To: The development of GNU GRUB

On Wed, Nov 11, 2015 at 10:09 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 06.11.2015 05:00, Ignat Korchagin пишет:
>>
>> Actually, I submitted similar patch recently as well. It provides read
>> function for variables and accepts a hint on how to process them. The
>> original patch is here:
>> https://lists.gnu.org/archive/html/grub-devel/2015-09/msg00043.html.
>>
>
> Yes, I was intending to comment on it, sorry.
>
> I am still unsure whether printf-like format specifier could be more
> flexible, but otherwise I like it in that it provides for future extensions.
> see comments below.
>
>
>> Probably, I forgot to enable plain-text mode, so it got there as a
>> binary attachment. I will repeat it here for convenience.
>>
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index 9764cd2..49fa3ec 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -728,6 +728,12 @@ module = {
>>   };
>>
>>   module = {
>> +  name = efivar;
>> +  efi = commands/efi/efivar.c;
>> +  enable = efi;
>> +};
>> +
>> +module = {
>>     name = blocklist;
>>     common = commands/blocklist.c;
>>   };
>> diff --git a/grub-core/commands/efi/efivar.c
>> b/grub-core/commands/efi/efivar.c
>> new file mode 100644
>> index 0000000..ca206eb
>> --- /dev/null
>> +++ b/grub-core/commands/efi/efivar.c
>> @@ -0,0 +1,146 @@
>> +/* efivar.c - Read EFI global variables. */
>> +/*
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2015 Free Software Foundation, Inc.
>> + *  Copyright (C) 2015 CloudFlare, 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/types.h>
>> +#include <grub/mm.h>
>> +#include <grub/misc.h>
>> +#include <grub/efi/api.h>
>> +#include <grub/efi/efi.h>
>> +#include <grub/extcmd.h>
>> +#include <grub/env.h>
>> +
>> +GRUB_MOD_LICENSE ("GPLv3+");
>> +
>> +static const struct grub_arg_option options[] = {
>> +  {"type", 't', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR as
>> specific type (hex, uint8, string). Default: hex."), N_("TYPE"),
>> ARG_TYPE_STRING},
>> +  {0, 0, 0, 0, 0, 0}
>> +};
>> +
>> +enum efi_var_type
>> +  {
>> +    EFI_VAR_STRING = 0,
>> +    EFI_VAR_UINT8,
>> +    EFI_VAR_HEX,
>> +    EFI_VAR_INVALID = -1
>> +  };
>> +
>> +static enum efi_var_type
>> +parse_efi_var_type (const char *type)
>> +{
>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>> +    return EFI_VAR_STRING;
>> +
>
>
> I think this should be "ascii" or "utf8". "string" is too ambiguous in UEFI
> environment, it can also mean sequence of UCS-2 characters.
I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
when printing. Maybe, to avoid confusion, it might be better to
completely remove this option. Basically, you do not want to interpret
raw buffers as strings. For best compatibility "hex" mode should be
promoted, I guess. What do you think?
>
>> +  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
>> +    return EFI_VAR_UINT8;
>> +
>> +  if (!grub_strncmp (type, "hex", sizeof("hex")))
>> +    return EFI_VAR_HEX;
>> +
>> +  return EFI_VAR_INVALID;
>> +}
>> +
>> +static grub_err_t
>> +grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
>> +  int argc, char **args)
>> +{
>> +  struct grub_arg_list *state = ctxt->state;
>> +  grub_err_t status;
>> +  void *efi_var = NULL;
>> +  grub_size_t efi_var_size = 0;
>> +  enum efi_var_type efi_type = EFI_VAR_HEX;
>> +  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
>> +  char *env_var = NULL;
>> +  grub_size_t i;
>> +
>> +  if (2 != argc)
>> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments
>> expected"));
>> +
>
>
> May be follow the suite and use "--set var". It could be useful to simply
> display variable on screen, like
>
> efivar OsIndicators
>
> or even
>
> efivar --all
>
Yes. Agree.
>> +  if (state[0].set)
>> +    efi_type = parse_efi_var_type (state[0].arg);
>> +
>> +  if (EFI_VAR_INVALID == efi_type)
>> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid EFI variable
>> type"));
>> +
>> +  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
>> +  if (!efi_var || !efi_var_size)
>> +    status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read
>> variable"));
>> +
>
>
> Should it distinguish between non-existent variable and failure to read
> variable? Is non-existent variable an error?
>
>
grub_efi_get_variable itself does not report the difference. It should
be modified to do that. From possible use-cases does it really matter
why you did not get the variable?
>> +  switch (efi_type)
>> +  {
>> +    case EFI_VAR_STRING:
>> +      env_var = grub_malloc (efi_var_size + 1);
>> +      if (!env_var)
>> +        {
>> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
>> memory"));
>> +          break;
>> +        }
>> +      grub_memcpy(env_var, efi_var, efi_var_size);
>> +      env_var[efi_var_size] = '\0';
>> +      break;
>> +
>> +    case EFI_VAR_UINT8:
>> +      env_var = grub_malloc (4);
>> +      if (!env_var)
>> +        {
>> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
>> memory"));
>> +          break;
>> +        }
>> +      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
>> +      break;
>> +
>> +    case EFI_VAR_HEX:
>> +      env_var = grub_malloc (efi_var_size * 2 + 1);
>> +      if (!env_var)
>> +        {
>> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
>> memory"));
>> +          break;
>> +        }
>> +      for (i = 0; i < efi_var_size; i++)
>> +        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
>> *)efi_var)[i]);
>> +      break;
>> +
>> +    default:
>> +      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
>> in module?)"));
>> +  }
>> +
>> +  status = grub_env_set (args[1], env_var);
>> +
>> +  if (env_var)
>> +    grub_free (env_var);
>> +
>> +  if (efi_var)
>> +    grub_free (efi_var);
>> +
>> +  return status;
>> +}
>> +
>> +static grub_extcmd_t cmd = NULL;
>> +
>> +GRUB_MOD_INIT (efivar)
>> +{
>> +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
>> N_("[-t TYPE] EFI_VAR ENV_VAR"),
>> + N_("Read EFI variable and put its contents in environment
>> variable."), options);
>> +}
>> +
>> +GRUB_MOD_FINI (efivar)
>> +{
>> +  if (cmd)
>> +    grub_unregister_extcmd (cmd);
>> +}
>>
>>
>> On Thu, Nov 5, 2015 at 10:25 AM, SevenBits <sevenbitstech@gmail.com>
>> wrote:
>>>
>>> On Wednesday, November 4, 2015, Andrei Borzenkov <arvidjaar@gmail.com>
>>> wrote:
>>>>
>>>>
>>>> 04.11.2015 02:05, Mat Troi пишет:
>>>>>
>>>>>
>>>>> Hi SevenBits,
>>>>>
>>>>> Thanks for letting me know.  Out of curiosity, any particular reason
>>>>> why
>>>>> this patch did not get merged?  It looks to be a useful feature.
>>>>>
>>>>
>>>> First, this patch was reverse patch :)
>>>
>>>
>>>
>>> Yeah, I think I had accidentally passed the arguments to the diff command
>>> in
>>> the wrong order.
>>>
>>>>
>>>>
>>>> I am not convinced making it easy to set EFI variable from within GRUB
>>>> is
>>>> good thing, because there are known reports about systems rendered
>>>> unbootable by writing too much into EFI flash. What is your use case
>>>> that
>>>> absolutely requires setting EFI variables? How are you going to
>>>> implement it
>>>> on other platforms?
>>>
>>>
>>>
>>> I should probably note that I wrote this patch for a specific project of
>>> mine which required the ability to read UEFI variables. I added in write
>>> functionality for good measure because I could. But I agree, this would
>>> only
>>> encourage tinkering and users messing with their systems and potentially
>>> bricking it, which would of course be blamed on GRUB.
>>>
>>>>
>>>>
>>>> Reading does not harm and may be useful, but then it should provide
>>>> generic interface to access arbitrary vendor namespace, not only EFI
>>>> global
>>>> variables, and handle arbitrary binary data, even if initial
>>>> implementation
>>>> handles only subset of them. Once someone starts to use it changing it
>>>> will
>>>> be much more difficult.
>>>>
>>>> Maybe it should take hints how to interpret variable values, or have
>>>> format option akin to printf.
>>>
>>>
>>>
>>> I would be happy to resume work on this, and debase it on the current
>>> code,
>>> if GRUB has a clear need for such functionality. I would prefer to have
>>> it
>>> be clear what the patch should consist of, though.
>>>
>>> A key question would be how to e.g. handle arbitrary data stored in
>>> variables if it is not something easily represent able like a string.
>>> Right
>>> now, the patch stores it's value into an environment variable specified
>>> by
>>> the user. Unless the powers that be think otherwise, I think this is the
>>> best way to go.
>>>
>>>>
>>>>
>>>>> Thanks,
>>>>> Mat
>>>>>
>>>>> On Tue, Nov 3, 2015 at 12:12 PM, SevenBits <sevenbitstech@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> On Tuesday, November 3, 2015, Mat Troi <mattroisang@gmail.com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Searching through google, I see there was an email thread to add a
>>>>>>> patch
>>>>>>> for getting and setting efi variable in GRUB2.
>>>>>>> https://lists.gnu.org/archive/html/grub-devel/2013-11/msg00328.html
>>>>>>>
>>>>>>> However, looking at the tree, it doesn't look like this patch was
>>>>>>> added,
>>>>>>> am I missing something?  Does anyone know if we have command to
>>>>>>> get/set
>>>>>>> efi
>>>>>>> variables in GRUB2?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=tree;f=grub-core/commands/efi;h=005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=HEAD
>>>>>>>
>>>>>>>
>>>>>> I'm the author of that patch. No, it was never merged into the tree.
>>>>>> As
>>>>>> far as I know, there is no equivalent functionality; GRUB does not
>>>>>> support
>>>>>> this feature.
>>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>> Mat
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Grub-devel mailing list
>>>>>> Grub-devel@gnu.org
>>>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Grub-devel mailing list
>>>>> Grub-devel@gnu.org
>>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>>
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-13 19:34             ` Ignat Korchagin
@ 2015-11-13 19:42               ` Ignat Korchagin
  2015-11-14  4:03                 ` Andrei Borzenkov
  0 siblings, 1 reply; 28+ messages in thread
From: Ignat Korchagin @ 2015-11-13 19:42 UTC (permalink / raw)
  To: The development of GNU GRUB

On Fri, Nov 13, 2015 at 11:34 AM, Ignat Korchagin <ignat@cloudflare.com> wrote:
> On Wed, Nov 11, 2015 at 10:09 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>> 06.11.2015 05:00, Ignat Korchagin пишет:
>>>
>>> Actually, I submitted similar patch recently as well. It provides read
>>> function for variables and accepts a hint on how to process them. The
>>> original patch is here:
>>> https://lists.gnu.org/archive/html/grub-devel/2015-09/msg00043.html.
>>>
>>
>> Yes, I was intending to comment on it, sorry.
>>
>> I am still unsure whether printf-like format specifier could be more
>> flexible, but otherwise I like it in that it provides for future extensions.
>> see comments below.
>>
>>
>>> Probably, I forgot to enable plain-text mode, so it got there as a
>>> binary attachment. I will repeat it here for convenience.
>>>
>>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>>> index 9764cd2..49fa3ec 100644
>>> --- a/grub-core/Makefile.core.def
>>> +++ b/grub-core/Makefile.core.def
>>> @@ -728,6 +728,12 @@ module = {
>>>   };
>>>
>>>   module = {
>>> +  name = efivar;
>>> +  efi = commands/efi/efivar.c;
>>> +  enable = efi;
>>> +};
>>> +
>>> +module = {
>>>     name = blocklist;
>>>     common = commands/blocklist.c;
>>>   };
>>> diff --git a/grub-core/commands/efi/efivar.c
>>> b/grub-core/commands/efi/efivar.c
>>> new file mode 100644
>>> index 0000000..ca206eb
>>> --- /dev/null
>>> +++ b/grub-core/commands/efi/efivar.c
>>> @@ -0,0 +1,146 @@
>>> +/* efivar.c - Read EFI global variables. */
>>> +/*
>>> + *  GRUB  --  GRand Unified Bootloader
>>> + *  Copyright (C) 2015 Free Software Foundation, Inc.
>>> + *  Copyright (C) 2015 CloudFlare, 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/types.h>
>>> +#include <grub/mm.h>
>>> +#include <grub/misc.h>
>>> +#include <grub/efi/api.h>
>>> +#include <grub/efi/efi.h>
>>> +#include <grub/extcmd.h>
>>> +#include <grub/env.h>
>>> +
>>> +GRUB_MOD_LICENSE ("GPLv3+");
>>> +
>>> +static const struct grub_arg_option options[] = {
>>> +  {"type", 't', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR as
>>> specific type (hex, uint8, string). Default: hex."), N_("TYPE"),
>>> ARG_TYPE_STRING},
>>> +  {0, 0, 0, 0, 0, 0}
>>> +};
>>> +
>>> +enum efi_var_type
>>> +  {
>>> +    EFI_VAR_STRING = 0,
>>> +    EFI_VAR_UINT8,
>>> +    EFI_VAR_HEX,
>>> +    EFI_VAR_INVALID = -1
>>> +  };
>>> +
>>> +static enum efi_var_type
>>> +parse_efi_var_type (const char *type)
>>> +{
>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>> +    return EFI_VAR_STRING;
>>> +
>>
>>
>> I think this should be "ascii" or "utf8". "string" is too ambiguous in UEFI
>> environment, it can also mean sequence of UCS-2 characters.
> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
> when printing. Maybe, to avoid confusion, it might be better to
> completely remove this option. Basically, you do not want to interpret
> raw buffers as strings. For best compatibility "hex" mode should be
> promoted, I guess. What do you think?
Checked again the UEFI spec. For globally defined variables which are
strings they specify ASCII. So if we leave this option, ascii is the
best name.

>>
>>> +  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
>>> +    return EFI_VAR_UINT8;
>>> +
>>> +  if (!grub_strncmp (type, "hex", sizeof("hex")))
>>> +    return EFI_VAR_HEX;
>>> +
>>> +  return EFI_VAR_INVALID;
>>> +}
>>> +
>>> +static grub_err_t
>>> +grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
>>> +  int argc, char **args)
>>> +{
>>> +  struct grub_arg_list *state = ctxt->state;
>>> +  grub_err_t status;
>>> +  void *efi_var = NULL;
>>> +  grub_size_t efi_var_size = 0;
>>> +  enum efi_var_type efi_type = EFI_VAR_HEX;
>>> +  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
>>> +  char *env_var = NULL;
>>> +  grub_size_t i;
>>> +
>>> +  if (2 != argc)
>>> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments
>>> expected"));
>>> +
>>
>>
>> May be follow the suite and use "--set var". It could be useful to simply
>> display variable on screen, like
>>
>> efivar OsIndicators
>>
>> or even
>>
>> efivar --all
>>
> Yes. Agree.
>>> +  if (state[0].set)
>>> +    efi_type = parse_efi_var_type (state[0].arg);
>>> +
>>> +  if (EFI_VAR_INVALID == efi_type)
>>> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid EFI variable
>>> type"));
>>> +
>>> +  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
>>> +  if (!efi_var || !efi_var_size)
>>> +    status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read
>>> variable"));
>>> +
>>
>>
>> Should it distinguish between non-existent variable and failure to read
>> variable? Is non-existent variable an error?
>>
>>
> grub_efi_get_variable itself does not report the difference. It should
> be modified to do that. From possible use-cases does it really matter
> why you did not get the variable?
>>> +  switch (efi_type)
>>> +  {
>>> +    case EFI_VAR_STRING:
>>> +      env_var = grub_malloc (efi_var_size + 1);
>>> +      if (!env_var)
>>> +        {
>>> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
>>> memory"));
>>> +          break;
>>> +        }
>>> +      grub_memcpy(env_var, efi_var, efi_var_size);
>>> +      env_var[efi_var_size] = '\0';
>>> +      break;
>>> +
>>> +    case EFI_VAR_UINT8:
>>> +      env_var = grub_malloc (4);
>>> +      if (!env_var)
>>> +        {
>>> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
>>> memory"));
>>> +          break;
>>> +        }
>>> +      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
>>> +      break;
>>> +
>>> +    case EFI_VAR_HEX:
>>> +      env_var = grub_malloc (efi_var_size * 2 + 1);
>>> +      if (!env_var)
>>> +        {
>>> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
>>> memory"));
>>> +          break;
>>> +        }
>>> +      for (i = 0; i < efi_var_size; i++)
>>> +        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
>>> *)efi_var)[i]);
>>> +      break;
>>> +
>>> +    default:
>>> +      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
>>> in module?)"));
>>> +  }
>>> +
>>> +  status = grub_env_set (args[1], env_var);
>>> +
>>> +  if (env_var)
>>> +    grub_free (env_var);
>>> +
>>> +  if (efi_var)
>>> +    grub_free (efi_var);
>>> +
>>> +  return status;
>>> +}
>>> +
>>> +static grub_extcmd_t cmd = NULL;
>>> +
>>> +GRUB_MOD_INIT (efivar)
>>> +{
>>> +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
>>> N_("[-t TYPE] EFI_VAR ENV_VAR"),
>>> + N_("Read EFI variable and put its contents in environment
>>> variable."), options);
>>> +}
>>> +
>>> +GRUB_MOD_FINI (efivar)
>>> +{
>>> +  if (cmd)
>>> +    grub_unregister_extcmd (cmd);
>>> +}
>>>
>>>
>>> On Thu, Nov 5, 2015 at 10:25 AM, SevenBits <sevenbitstech@gmail.com>
>>> wrote:
>>>>
>>>> On Wednesday, November 4, 2015, Andrei Borzenkov <arvidjaar@gmail.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> 04.11.2015 02:05, Mat Troi пишет:
>>>>>>
>>>>>>
>>>>>> Hi SevenBits,
>>>>>>
>>>>>> Thanks for letting me know.  Out of curiosity, any particular reason
>>>>>> why
>>>>>> this patch did not get merged?  It looks to be a useful feature.
>>>>>>
>>>>>
>>>>> First, this patch was reverse patch :)
>>>>
>>>>
>>>>
>>>> Yeah, I think I had accidentally passed the arguments to the diff command
>>>> in
>>>> the wrong order.
>>>>
>>>>>
>>>>>
>>>>> I am not convinced making it easy to set EFI variable from within GRUB
>>>>> is
>>>>> good thing, because there are known reports about systems rendered
>>>>> unbootable by writing too much into EFI flash. What is your use case
>>>>> that
>>>>> absolutely requires setting EFI variables? How are you going to
>>>>> implement it
>>>>> on other platforms?
>>>>
>>>>
>>>>
>>>> I should probably note that I wrote this patch for a specific project of
>>>> mine which required the ability to read UEFI variables. I added in write
>>>> functionality for good measure because I could. But I agree, this would
>>>> only
>>>> encourage tinkering and users messing with their systems and potentially
>>>> bricking it, which would of course be blamed on GRUB.
>>>>
>>>>>
>>>>>
>>>>> Reading does not harm and may be useful, but then it should provide
>>>>> generic interface to access arbitrary vendor namespace, not only EFI
>>>>> global
>>>>> variables, and handle arbitrary binary data, even if initial
>>>>> implementation
>>>>> handles only subset of them. Once someone starts to use it changing it
>>>>> will
>>>>> be much more difficult.
>>>>>
>>>>> Maybe it should take hints how to interpret variable values, or have
>>>>> format option akin to printf.
>>>>
>>>>
>>>>
>>>> I would be happy to resume work on this, and debase it on the current
>>>> code,
>>>> if GRUB has a clear need for such functionality. I would prefer to have
>>>> it
>>>> be clear what the patch should consist of, though.
>>>>
>>>> A key question would be how to e.g. handle arbitrary data stored in
>>>> variables if it is not something easily represent able like a string.
>>>> Right
>>>> now, the patch stores it's value into an environment variable specified
>>>> by
>>>> the user. Unless the powers that be think otherwise, I think this is the
>>>> best way to go.
>>>>
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> Mat
>>>>>>
>>>>>> On Tue, Nov 3, 2015 at 12:12 PM, SevenBits <sevenbitstech@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> On Tuesday, November 3, 2015, Mat Troi <mattroisang@gmail.com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Searching through google, I see there was an email thread to add a
>>>>>>>> patch
>>>>>>>> for getting and setting efi variable in GRUB2.
>>>>>>>> https://lists.gnu.org/archive/html/grub-devel/2013-11/msg00328.html
>>>>>>>>
>>>>>>>> However, looking at the tree, it doesn't look like this patch was
>>>>>>>> added,
>>>>>>>> am I missing something?  Does anyone know if we have command to
>>>>>>>> get/set
>>>>>>>> efi
>>>>>>>> variables in GRUB2?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> http://git.savannah.gnu.org/gitweb/?p=grub.git;a=tree;f=grub-core/commands/efi;h=005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=HEAD
>>>>>>>>
>>>>>>>>
>>>>>>> I'm the author of that patch. No, it was never merged into the tree.
>>>>>>> As
>>>>>>> far as I know, there is no equivalent functionality; GRUB does not
>>>>>>> support
>>>>>>> this feature.
>>>>>>>
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Mat
>>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Grub-devel mailing list
>>>>>>> Grub-devel@gnu.org
>>>>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Grub-devel mailing list
>>>>>> Grub-devel@gnu.org
>>>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Grub-devel mailing list
>>>>> Grub-devel@gnu.org
>>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-13 19:42               ` Ignat Korchagin
@ 2015-11-14  4:03                 ` Andrei Borzenkov
  2015-11-17 11:48                   ` Ignat Korchagin
                                     ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Andrei Borzenkov @ 2015-11-14  4:03 UTC (permalink / raw)
  To: grub-devel

13.11.2015 22:42, Ignat Korchagin пишет:
>>>> +static enum efi_var_type
>>>> +parse_efi_var_type (const char *type)
>>>> +{
>>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>>> +    return EFI_VAR_STRING;
>>>> +
>>>
>>>
>>> I think this should be "ascii" or "utf8". "string" is too ambiguous in UEFI
>>> environment, it can also mean sequence of UCS-2 characters.
>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>> when printing. Maybe, to avoid confusion, it might be better to
>> completely remove this option. Basically, you do not want to interpret
>> raw buffers as strings. For best compatibility "hex" mode should be
>> promoted, I guess. What do you think?
> Checked again the UEFI spec. For globally defined variables which are
> strings they specify ASCII. So if we leave this option, ascii is the
> best name.
>

What about

   - ascii - print ASCII characters verbatim, escape non-ASCII in usual 
way (similar to "od -c")

   - raw - simply put raw variable without any interpretation.

This is better aligned with argument describing output formatting rather 
than attempting to "type" variable.

Alternative (or in addition to) ascii - dump which prints usual pretty 
hex dump of content (hexdump -C). This is handy to interactively look at 
variable content.

Or, and change name from --type to --format :)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-14  4:03                 ` Andrei Borzenkov
@ 2015-11-17 11:48                   ` Ignat Korchagin
  2015-11-29  8:57                     ` Andrei Borzenkov
  2015-11-24 19:23                   ` Mat Troi
  2015-11-27 14:07                   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2 siblings, 1 reply; 28+ messages in thread
From: Ignat Korchagin @ 2015-11-17 11:48 UTC (permalink / raw)
  To: The development of GNU GRUB

Please, see updated patch below. I tried to reuse GRUB hexdump
function, so as a result dump will not work with setting vatiables -
just dumping to stdout. But it doesn't make sense to assign dump to a
variable anyway.

diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
new file mode 100644
index 0000000..3ffd5ca
--- /dev/null
+++ b/grub-core/commands/efi/efivar.c
@@ -0,0 +1,253 @@
+/* efivar.c - Read EFI global variables. */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2015 Free Software Foundation, Inc.
+ *  Copyright (C) 2015 CloudFlare, 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/types.h>
+#include <grub/mm.h>
+#include <grub/misc.h>
+#include <grub/efi/api.h>
+#include <grub/efi/efi.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/lib/hexdump.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static const struct grub_arg_option options[] = {
+  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
specific format (hex, uint8, ascii, raw, dump). Default: hex."),
N_("FORMAT"), ARG_TYPE_STRING},
+  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
environment variable (does not work with dump)."), N_("ENV_VAR"),
ARG_TYPE_STRING},
+  {0, 0, 0, 0, 0, 0}
+};
+
+enum efi_var_type
+  {
+    EFI_VAR_ASCII = 0,
+    EFI_VAR_RAW,
+    EFI_VAR_UINT8,
+    EFI_VAR_HEX,
+    EFI_VAR_DUMP,
+    EFI_VAR_INVALID = -1
+  };
+
+static enum efi_var_type
+parse_efi_var_type (const char *type)
+{
+  if (!grub_strncmp (type, "ascii", sizeof("ascii")))
+    return EFI_VAR_ASCII;
+
+  if (!grub_strncmp (type, "raw", sizeof("raw")))
+    return EFI_VAR_ASCII;
+
+  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
+    return EFI_VAR_UINT8;
+
+  if (!grub_strncmp (type, "hex", sizeof("hex")))
+    return EFI_VAR_HEX;
+
+  if (!grub_strncmp (type, "dump", sizeof("dump")))
+    return EFI_VAR_DUMP;
+
+  return EFI_VAR_INVALID;
+}
+
+static int
+grub_print_ascii (char *str, char c)
+{
+  if (grub_iscntrl (c))
+  {
+    switch (c)
+      {
+        case '\0':
+          str[0] = '\\';
+          str[1] = '0';
+          return 2;
+
+        case '\a':
+          str[0] = '\\';
+          str[1] = 'a';
+          return 2;
+
+        case '\b':
+          str[0] = '\\';
+          str[1] = 'b';
+          return 2;
+
+        case '\f':
+          str[0] = '\\';
+          str[1] = 'f';
+          return 2;
+
+        case '\n':
+          str[0] = '\\';
+          str[1] = 'n';
+          return 2;
+
+        case '\r':
+          str[0] = '\\';
+          str[1] = 'r';
+          return 2;
+
+        case '\t':
+          str[0] = '\\';
+          str[1] = 't';
+          return 2;
+
+        case '\v':
+          str[0] = '\\';
+          str[1] = 'v';
+          return 2;
+
+        default:
+          str[0] = '.'; /* as in hexdump -C */
+          return 1;
+      }
+  }
+
+  str[0] = c;
+  return 1;
+}
+
+static grub_err_t
+grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
+  int argc, char **args)
+{
+  struct grub_arg_list *state = ctxt->state;
+  grub_err_t status;
+  void *efi_var = NULL;
+  grub_size_t efi_var_size = 0;
+  enum efi_var_type efi_type = EFI_VAR_HEX;
+  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  char *env_var = NULL;
+  grub_size_t i;
+  char *ptr;
+
+  if (1 != argc)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
+
+  if (state[0].set)
+    efi_type = parse_efi_var_type (state[0].arg);
+
+  if (EFI_VAR_INVALID == efi_type)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
+
+  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
+  if (!efi_var || !efi_var_size)
+    {
+      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
+      goto err;
+    }
+
+  switch (efi_type)
+  {
+    case EFI_VAR_ASCII:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+
+      ptr = env_var;
+
+      for (i = 0; i < efi_var_size; i++)
+        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
+      *ptr = '\0';
+      break;
+
+    case EFI_VAR_RAW:
+      env_var = grub_malloc (efi_var_size + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      grub_memcpy (env_var, efi_var, efi_var_size);
+      env_var[efi_var_size] = '\0';
+      break;
+
+    case EFI_VAR_UINT8:
+      env_var = grub_malloc (4);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
+      break;
+
+    case EFI_VAR_HEX:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      for (i = 0; i < efi_var_size; i++)
+        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
*)efi_var)[i]);
+      break;
+
+    case EFI_VAR_DUMP:
+      if (state[1].set)
+        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
variable with dump format specifier"));
+      else
+        {
+          hexdump (0, (char *)efi_var, efi_var_size);
+          status = GRUB_ERR_NONE;
+        }
+      break;
+
+    default:
+      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
in module?)"));
+  }
+
+  if (efi_type != EFI_VAR_DUMP)
+    {
+      if (state[1].set)
+        status = grub_env_set (state[1].arg, env_var);
+      else
+        {
+          grub_printf ("%s\n", (const char *)env_var);
+          status = GRUB_ERR_NONE;
+        }
+    }
+
+err:
+
+  if (env_var)
+    grub_free (env_var);
+
+  if (efi_var)
+    grub_free (efi_var);
+
+  return status;
+}
+
+static grub_extcmd_t cmd = NULL;
+
+GRUB_MOD_INIT (efivar)
+{
+  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
+ N_("Read EFI variable and print it or save its contents to
environment variable."), options);
+}
+
+GRUB_MOD_FINI (efivar)
+{
+  if (cmd)
+    grub_unregister_extcmd (cmd);
+}
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index a6101de..2c8f023 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -728,6 +728,12 @@
 };

 module = {
+  name = efivar;
+  efi = commands/efi/efivar.c;
+  enable = efi;
+};
+
+module = {
   name = blocklist;
   common = commands/blocklist.c;
 };

On Sat, Nov 14, 2015 at 4:03 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 13.11.2015 22:42, Ignat Korchagin пишет:
>>>>>
>>>>> +static enum efi_var_type
>>>>> +parse_efi_var_type (const char *type)
>>>>> +{
>>>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>>>> +    return EFI_VAR_STRING;
>>>>> +
>>>>
>>>>
>>>>
>>>> I think this should be "ascii" or "utf8". "string" is too ambiguous in
>>>> UEFI
>>>> environment, it can also mean sequence of UCS-2 characters.
>>>
>>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>>> when printing. Maybe, to avoid confusion, it might be better to
>>> completely remove this option. Basically, you do not want to interpret
>>> raw buffers as strings. For best compatibility "hex" mode should be
>>> promoted, I guess. What do you think?
>>
>> Checked again the UEFI spec. For globally defined variables which are
>> strings they specify ASCII. So if we leave this option, ascii is the
>> best name.
>>
>
> What about
>
>   - ascii - print ASCII characters verbatim, escape non-ASCII in usual way
> (similar to "od -c")
>
>   - raw - simply put raw variable without any interpretation.
>
> This is better aligned with argument describing output formatting rather
> than attempting to "type" variable.
>
> Alternative (or in addition to) ascii - dump which prints usual pretty hex
> dump of content (hexdump -C). This is handy to interactively look at
> variable content.
>
> Or, and change name from --type to --format :)
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-14  4:03                 ` Andrei Borzenkov
  2015-11-17 11:48                   ` Ignat Korchagin
@ 2015-11-24 19:23                   ` Mat Troi
  2015-11-24 20:48                     ` Seth Goldberg
  2015-11-27 14:07                   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2 siblings, 1 reply; 28+ messages in thread
From: Mat Troi @ 2015-11-24 19:23 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 2150 bytes --]

For a specific project implementation, I need to be able to write and read
one EFI variable during boot only.  So I have written a grub command to do
just that.  But the issue is with setting these attributes to the variable
(GRUB_EFI_VARIABLE_NON_VOLATILE, GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS), I
am not able to modify it during boot on some platform.  Since this is so
specific, I am wondering if anyone has seen similar issue?

On Fri, Nov 13, 2015 at 8:03 PM, Andrei Borzenkov <arvidjaar@gmail.com>
wrote:

> 13.11.2015 22:42, Ignat Korchagin пишет:
>
>> +static enum efi_var_type
>>>>> +parse_efi_var_type (const char *type)
>>>>> +{
>>>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>>>> +    return EFI_VAR_STRING;
>>>>> +
>>>>>
>>>>
>>>>
>>>> I think this should be "ascii" or "utf8". "string" is too ambiguous in
>>>> UEFI
>>>> environment, it can also mean sequence of UCS-2 characters.
>>>>
>>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>>> when printing. Maybe, to avoid confusion, it might be better to
>>> completely remove this option. Basically, you do not want to interpret
>>> raw buffers as strings. For best compatibility "hex" mode should be
>>> promoted, I guess. What do you think?
>>>
>> Checked again the UEFI spec. For globally defined variables which are
>> strings they specify ASCII. So if we leave this option, ascii is the
>> best name.
>>
>>
> What about
>
>   - ascii - print ASCII characters verbatim, escape non-ASCII in usual way
> (similar to "od -c")
>
>   - raw - simply put raw variable without any interpretation.
>
> This is better aligned with argument describing output formatting rather
> than attempting to "type" variable.
>
> Alternative (or in addition to) ascii - dump which prints usual pretty hex
> dump of content (hexdump -C). This is handy to interactively look at
> variable content.
>
> Or, and change name from --type to --format :)
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 3226 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-24 19:23                   ` Mat Troi
@ 2015-11-24 20:48                     ` Seth Goldberg
  0 siblings, 0 replies; 28+ messages in thread
From: Seth Goldberg @ 2015-11-24 20:48 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 2543 bytes --]

That sounds like a firmware implementation bug, unfortunately.  I've seen it be hit or miss setting boot service only variables on some platforms.  

   --S

> On Nov 24, 2015, at 11:23 AM, Mat Troi <mattroisang@gmail.com> wrote:
> 
> For a specific project implementation, I need to be able to write and read one EFI variable during boot only.  So I have written a grub command to do just that.  But the issue is with setting these attributes to the variable (GRUB_EFI_VARIABLE_NON_VOLATILE, GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS), I am not able to modify it during boot on some platform.  Since this is so specific, I am wondering if anyone has seen similar issue?
> 
>> On Fri, Nov 13, 2015 at 8:03 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>> 13.11.2015 22:42, Ignat Korchagin пишет:
>>>>>> +static enum efi_var_type
>>>>>> +parse_efi_var_type (const char *type)
>>>>>> +{
>>>>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>>>>> +    return EFI_VAR_STRING;
>>>>>> +
>>>>> 
>>>>> 
>>>>> I think this should be "ascii" or "utf8". "string" is too ambiguous in UEFI
>>>>> environment, it can also mean sequence of UCS-2 characters.
>>>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>>>> when printing. Maybe, to avoid confusion, it might be better to
>>>> completely remove this option. Basically, you do not want to interpret
>>>> raw buffers as strings. For best compatibility "hex" mode should be
>>>> promoted, I guess. What do you think?
>>> Checked again the UEFI spec. For globally defined variables which are
>>> strings they specify ASCII. So if we leave this option, ascii is the
>>> best name.
>> 
>> What about
>> 
>>   - ascii - print ASCII characters verbatim, escape non-ASCII in usual way (similar to "od -c")
>> 
>>   - raw - simply put raw variable without any interpretation.
>> 
>> This is better aligned with argument describing output formatting rather than attempting to "type" variable.
>> 
>> Alternative (or in addition to) ascii - dump which prints usual pretty hex dump of content (hexdump -C). This is handy to interactively look at variable content.
>> 
>> Or, and change name from --type to --format :)
>> 
>> 
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

[-- Attachment #2: Type: text/html, Size: 4049 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-14  4:03                 ` Andrei Borzenkov
  2015-11-17 11:48                   ` Ignat Korchagin
  2015-11-24 19:23                   ` Mat Troi
@ 2015-11-27 14:07                   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2015-11-27 14:25                     ` Ignat Korchagin
  2 siblings, 1 reply; 28+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2015-11-27 14:07 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 1996 bytes --]

On 14.11.2015 05:03, Andrei Borzenkov wrote:
> 13.11.2015 22:42, Ignat Korchagin пишет:
>>>>> +static enum efi_var_type
>>>>> +parse_efi_var_type (const char *type)
>>>>> +{
>>>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>>>> +    return EFI_VAR_STRING;
>>>>> +
>>>>
>>>>
>>>> I think this should be "ascii" or "utf8". "string" is too ambiguous
>>>> in UEFI
>>>> environment, it can also mean sequence of UCS-2 characters.
>>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>>> when printing. Maybe, to avoid confusion, it might be better to
>>> completely remove this option. Basically, you do not want to interpret
>>> raw buffers as strings. For best compatibility "hex" mode should be
>>> promoted, I guess. What do you think?
>> Checked again the UEFI spec. For globally defined variables which are
>> strings they specify ASCII. So if we leave this option, ascii is the
>> best name.
>>
> 
> What about
> 
>   - ascii - print ASCII characters verbatim, escape non-ASCII in usual
> way (similar to "od -c")
> 
>   - raw - simply put raw variable without any interpretation.
> 
I would add:
* hex. To print and store in hex form
* utf16. Decode utf16 into utf8. utf16 is a common encoding in EFI land.
I would also skip on raw, at least until we have a real need for it as
we currently are not equiped to handle raw strings in variable contents,
including but not limited to \0 being considered a terminator
> This is better aligned with argument describing output formatting rather
> than attempting to "type" variable.
> 
> Alternative (or in addition to) ascii - dump which prints usual pretty
> hex dump of content (hexdump -C). This is handy to interactively look at
> variable content.
> 
> Or, and change name from --type to --format :)
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-27 14:07                   ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2015-11-27 14:25                     ` Ignat Korchagin
  2015-11-29  9:00                       ` Andrei Borzenkov
  0 siblings, 1 reply; 28+ messages in thread
From: Ignat Korchagin @ 2015-11-27 14:25 UTC (permalink / raw)
  To: The development of GNU GRUB

> I would add:
> * hex. To print and store in hex form
> * utf16. Decode utf16 into utf8. utf16 is a common encoding in EFI land.

hex is already part of the latest proposed patch:
>> +    case EFI_VAR_HEX:

I'm not sure whether utf16 will be useful, as all standard UEFI
variables now are using ASCII for string data.

On Fri, Nov 27, 2015 at 2:07 PM, Vladimir 'φ-coder/phcoder' Serbinenko
<phcoder@gmail.com> wrote:
> On 14.11.2015 05:03, Andrei Borzenkov wrote:
>> 13.11.2015 22:42, Ignat Korchagin пишет:
>>>>>> +static enum efi_var_type
>>>>>> +parse_efi_var_type (const char *type)
>>>>>> +{
>>>>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>>>>> +    return EFI_VAR_STRING;
>>>>>> +
>>>>>
>>>>>
>>>>> I think this should be "ascii" or "utf8". "string" is too ambiguous
>>>>> in UEFI
>>>>> environment, it can also mean sequence of UCS-2 characters.
>>>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>>>> when printing. Maybe, to avoid confusion, it might be better to
>>>> completely remove this option. Basically, you do not want to interpret
>>>> raw buffers as strings. For best compatibility "hex" mode should be
>>>> promoted, I guess. What do you think?
>>> Checked again the UEFI spec. For globally defined variables which are
>>> strings they specify ASCII. So if we leave this option, ascii is the
>>> best name.
>>>
>>
>> What about
>>
>>   - ascii - print ASCII characters verbatim, escape non-ASCII in usual
>> way (similar to "od -c")
>>
>>   - raw - simply put raw variable without any interpretation.
>>
> I would add:
> * hex. To print and store in hex form
> * utf16. Decode utf16 into utf8. utf16 is a common encoding in EFI land.
> I would also skip on raw, at least until we have a real need for it as
> we currently are not equiped to handle raw strings in variable contents,
> including but not limited to \0 being considered a terminator
>> This is better aligned with argument describing output formatting rather
>> than attempting to "type" variable.
>>
>> Alternative (or in addition to) ascii - dump which prints usual pretty
>> hex dump of content (hexdump -C). This is handy to interactively look at
>> variable content.
>>
>> Or, and change name from --type to --format :)
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-17 11:48                   ` Ignat Korchagin
@ 2015-11-29  8:57                     ` Andrei Borzenkov
  0 siblings, 0 replies; 28+ messages in thread
From: Andrei Borzenkov @ 2015-11-29  8:57 UTC (permalink / raw)
  To: The development of GNU GRUB

17.11.2015 14:48, Ignat Korchagin пишет:
> Please, see updated patch below. I tried to reuse GRUB hexdump
> function, so as a result dump will not work with setting vatiables -
> just dumping to stdout. But it doesn't make sense to assign dump to a
> variable anyway.
> 
> diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
> new file mode 100644
> index 0000000..3ffd5ca
> --- /dev/null
> +++ b/grub-core/commands/efi/efivar.c
> @@ -0,0 +1,253 @@
> +/* efivar.c - Read EFI global variables. */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015 Free Software Foundation, Inc.
> + *  Copyright (C) 2015 CloudFlare, 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/types.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +#include <grub/extcmd.h>
> +#include <grub/env.h>
> +#include <grub/lib/hexdump.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static const struct grub_arg_option options[] = {
> +  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
> specific format (hex, uint8, ascii, raw, dump). Default: hex."),
> N_("FORMAT"), ARG_TYPE_STRING},
> +  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
> environment variable (does not work with dump)."), N_("ENV_VAR"),
> ARG_TYPE_STRING},
> +  {0, 0, 0, 0, 0, 0}
> +};
> +
> +enum efi_var_type
> +  {
> +    EFI_VAR_ASCII = 0,
> +    EFI_VAR_RAW,
> +    EFI_VAR_UINT8,
> +    EFI_VAR_HEX,
> +    EFI_VAR_DUMP,
> +    EFI_VAR_INVALID = -1
> +  };
> +
> +static enum efi_var_type
> +parse_efi_var_type (const char *type)
> +{
> +  if (!grub_strncmp (type, "ascii", sizeof("ascii")))
> +    return EFI_VAR_ASCII;
> +
> +  if (!grub_strncmp (type, "raw", sizeof("raw")))
> +    return EFI_VAR_ASCII;
> +
> +  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
> +    return EFI_VAR_UINT8;
> +
> +  if (!grub_strncmp (type, "hex", sizeof("hex")))
> +    return EFI_VAR_HEX;
> +
> +  if (!grub_strncmp (type, "dump", sizeof("dump")))
> +    return EFI_VAR_DUMP;
> +

Should not this be strcmp, not str*n*cmp?

> +  return EFI_VAR_INVALID;
> +}
> +
> +static int
> +grub_print_ascii (char *str, char c)
> +{
> +  if (grub_iscntrl (c))
> +  {
> +    switch (c)
> +      {
> +        case '\0':
> +          str[0] = '\\';
> +          str[1] = '0';
> +          return 2;
> +
> +        case '\a':
> +          str[0] = '\\';
> +          str[1] = 'a';
> +          return 2;
> +
> +        case '\b':
> +          str[0] = '\\';
> +          str[1] = 'b';
> +          return 2;
> +
> +        case '\f':
> +          str[0] = '\\';
> +          str[1] = 'f';
> +          return 2;
> +
> +        case '\n':
> +          str[0] = '\\';
> +          str[1] = 'n';
> +          return 2;
> +
> +        case '\r':
> +          str[0] = '\\';
> +          str[1] = 'r';
> +          return 2;
> +
> +        case '\t':
> +          str[0] = '\\';
> +          str[1] = 't';
> +          return 2;
> +
> +        case '\v':
> +          str[0] = '\\';
> +          str[1] = 'v';
> +          return 2;
> +
> +        default:
> +          str[0] = '.'; /* as in hexdump -C */
> +          return 1;
> +      }
> +  }
> +
> +  str[0] = c;
> +  return 1;
> +}
> +
> +static grub_err_t
> +grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
> +  int argc, char **args)
> +{
> +  struct grub_arg_list *state = ctxt->state;
> +  grub_err_t status;
> +  void *efi_var = NULL;
> +  grub_size_t efi_var_size = 0;
> +  enum efi_var_type efi_type = EFI_VAR_HEX;
> +  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> +  char *env_var = NULL;
> +  grub_size_t i;
> +  char *ptr;
> +
> +  if (1 != argc)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));

One argument expected

> +
> +  if (state[0].set)
> +    efi_type = parse_efi_var_type (state[0].arg);
> +
> +  if (EFI_VAR_INVALID == efi_type)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
> +
> +  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
> +  if (!efi_var || !efi_var_size)
> +    {
> +      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
> +      goto err;
> +    }
> +
> +  switch (efi_type)
> +  {
> +    case EFI_VAR_ASCII:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +
> +      ptr = env_var;
> +
> +      for (i = 0; i < efi_var_size; i++)
> +        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
> +      *ptr = '\0';
> +      break;
> +
> +    case EFI_VAR_RAW:
> +      env_var = grub_malloc (efi_var_size + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +      grub_memcpy (env_var, efi_var, efi_var_size);
> +      env_var[efi_var_size] = '\0';

That would be wrong ("raw" implies no modification) but I am not keen on
keeping "raw" as was discussed.

> +      break;
> +
> +    case EFI_VAR_UINT8:
> +      env_var = grub_malloc (4);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
> +      break;
> +
> +    case EFI_VAR_HEX:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +      for (i = 0; i < efi_var_size; i++)
> +        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
> *)efi_var)[i]);
> +      break;
> +
> +    case EFI_VAR_DUMP:
> +      if (state[1].set)
> +        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
> variable with dump format specifier"));
> +      else
> +        {
> +          hexdump (0, (char *)efi_var, efi_var_size);
> +          status = GRUB_ERR_NONE;
> +        }
> +      break;
> +
> +    default:
> +      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
> in module?)"));
> +  }
> +
> +  if (efi_type != EFI_VAR_DUMP)
> +    {
> +      if (state[1].set)
> +        status = grub_env_set (state[1].arg, env_var);
> +      else
> +        {
> +          grub_printf ("%s\n", (const char *)env_var);
> +          status = GRUB_ERR_NONE;
> +        }
> +    }
> +
> +err:
> +
> +  if (env_var)
> +    grub_free (env_var);
> +
> +  if (efi_var)
> +    grub_free (efi_var);
> +
> +  return status;
> +}
> +
> +static grub_extcmd_t cmd = NULL;
> +
> +GRUB_MOD_INIT (efivar)
> +{
> +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
> N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
> + N_("Read EFI variable and print it or save its contents to
> environment variable."), options);
> +}
> +
> +GRUB_MOD_FINI (efivar)
> +{
> +  if (cmd)
> +    grub_unregister_extcmd (cmd);
> +}
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index a6101de..2c8f023 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -728,6 +728,12 @@
>  };
> 
>  module = {
> +  name = efivar;
> +  efi = commands/efi/efivar.c;
> +  enable = efi;
> +};
> +
> +module = {
>    name = blocklist;
>    common = commands/blocklist.c;
>  };
> 
> On Sat, Nov 14, 2015 at 4:03 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>> 13.11.2015 22:42, Ignat Korchagin пишет:
>>>>>>
>>>>>> +static enum efi_var_type
>>>>>> +parse_efi_var_type (const char *type)
>>>>>> +{
>>>>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>>>>> +    return EFI_VAR_STRING;
>>>>>> +
>>>>>
>>>>>
>>>>>
>>>>> I think this should be "ascii" or "utf8". "string" is too ambiguous in
>>>>> UEFI
>>>>> environment, it can also mean sequence of UCS-2 characters.
>>>>
>>>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>>>> when printing. Maybe, to avoid confusion, it might be better to
>>>> completely remove this option. Basically, you do not want to interpret
>>>> raw buffers as strings. For best compatibility "hex" mode should be
>>>> promoted, I guess. What do you think?
>>>
>>> Checked again the UEFI spec. For globally defined variables which are
>>> strings they specify ASCII. So if we leave this option, ascii is the
>>> best name.
>>>
>>
>> What about
>>
>>   - ascii - print ASCII characters verbatim, escape non-ASCII in usual way
>> (similar to "od -c")
>>
>>   - raw - simply put raw variable without any interpretation.
>>
>> This is better aligned with argument describing output formatting rather
>> than attempting to "type" variable.
>>
>> Alternative (or in addition to) ascii - dump which prints usual pretty hex
>> dump of content (hexdump -C). This is handy to interactively look at
>> variable content.
>>
>> Or, and change name from --type to --format :)
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-27 14:25                     ` Ignat Korchagin
@ 2015-11-29  9:00                       ` Andrei Borzenkov
  2015-12-02 12:53                         ` Ignat Korchagin
  0 siblings, 1 reply; 28+ messages in thread
From: Andrei Borzenkov @ 2015-11-29  9:00 UTC (permalink / raw)
  To: grub-devel

27.11.2015 17:25, Ignat Korchagin пишет:
>> I would add:
>> * hex. To print and store in hex form
>> * utf16. Decode utf16 into utf8. utf16 is a common encoding in EFI land.
> 
> hex is already part of the latest proposed patch:
>>> +    case EFI_VAR_HEX:
> 
> I'm not sure whether utf16 will be useful, as all standard UEFI
> variables now are using ASCII for string data.
> 

I agree; it can be added later if use case emerges. So far all
"interesting" cases that use UTF-16 are not limited to strings, and need
much more elaborate parsing. That is why I mentioned generic format
specifier before. But this also can be added if needed, overall syntax
seems to be flexible enough.

> On Fri, Nov 27, 2015 at 2:07 PM, Vladimir 'φ-coder/phcoder' Serbinenko
> <phcoder@gmail.com> wrote:
>> On 14.11.2015 05:03, Andrei Borzenkov wrote:
>>> 13.11.2015 22:42, Ignat Korchagin пишет:
>>>>>>> +static enum efi_var_type
>>>>>>> +parse_efi_var_type (const char *type)
>>>>>>> +{
>>>>>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>>>>>> +    return EFI_VAR_STRING;
>>>>>>> +
>>>>>>
>>>>>>
>>>>>> I think this should be "ascii" or "utf8". "string" is too ambiguous
>>>>>> in UEFI
>>>>>> environment, it can also mean sequence of UCS-2 characters.
>>>>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>>>>> when printing. Maybe, to avoid confusion, it might be better to
>>>>> completely remove this option. Basically, you do not want to interpret
>>>>> raw buffers as strings. For best compatibility "hex" mode should be
>>>>> promoted, I guess. What do you think?
>>>> Checked again the UEFI spec. For globally defined variables which are
>>>> strings they specify ASCII. So if we leave this option, ascii is the
>>>> best name.
>>>>
>>>
>>> What about
>>>
>>>   - ascii - print ASCII characters verbatim, escape non-ASCII in usual
>>> way (similar to "od -c")
>>>
>>>   - raw - simply put raw variable without any interpretation.
>>>
>> I would add:
>> * hex. To print and store in hex form
>> * utf16. Decode utf16 into utf8. utf16 is a common encoding in EFI land.
>> I would also skip on raw, at least until we have a real need for it as
>> we currently are not equiped to handle raw strings in variable contents,
>> including but not limited to \0 being considered a terminator
>>> This is better aligned with argument describing output formatting rather
>>> than attempting to "type" variable.
>>>
>>> Alternative (or in addition to) ascii - dump which prints usual pretty
>>> hex dump of content (hexdump -C). This is handy to interactively look at
>>> variable content.
>>>
>>> Or, and change name from --type to --format :)
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-11-29  9:00                       ` Andrei Borzenkov
@ 2015-12-02 12:53                         ` Ignat Korchagin
  2015-12-02 13:00                           ` Andrei Borzenkov
  0 siblings, 1 reply; 28+ messages in thread
From: Ignat Korchagin @ 2015-12-02 12:53 UTC (permalink / raw)
  To: The development of GNU GRUB

Let's add utf16 later then (if needed). I also noticed a typo in one
error string in my above patch (it was checking that command receives
one argument, but prints that two arguments expected), so here is the
updated one.

So, finally, should I remove the "raw" parser (as per Vladimir's
request). Personally, I would keep it. It might be useful to modify
the hexdump command, that it can dump the contents of a variable. Then
those pieces will fit together: storing raw value in the variable and
examining it with hexdump cmd.

diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
new file mode 100644
index 0000000..3ffd5ca
--- /dev/null
+++ b/grub-core/commands/efi/efivar.c
@@ -0,0 +1,253 @@
+/* efivar.c - Read EFI global variables. */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2015 Free Software Foundation, Inc.
+ *  Copyright (C) 2015 CloudFlare, 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/types.h>
+#include <grub/mm.h>
+#include <grub/misc.h>
+#include <grub/efi/api.h>
+#include <grub/efi/efi.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/lib/hexdump.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static const struct grub_arg_option options[] = {
+  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
specific format (hex, uint8, ascii, raw, dump). Default: hex."),
N_("FORMAT"), ARG_TYPE_STRING},
+  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
environment variable (does not work with dump)."), N_("ENV_VAR"),
ARG_TYPE_STRING},
+  {0, 0, 0, 0, 0, 0}
+};
+
+enum efi_var_type
+  {
+    EFI_VAR_ASCII = 0,
+    EFI_VAR_RAW,
+    EFI_VAR_UINT8,
+    EFI_VAR_HEX,
+    EFI_VAR_DUMP,
+    EFI_VAR_INVALID = -1
+  };
+
+static enum efi_var_type
+parse_efi_var_type (const char *type)
+{
+  if (!grub_strncmp (type, "ascii", sizeof("ascii")))
+    return EFI_VAR_ASCII;
+
+  if (!grub_strncmp (type, "raw", sizeof("raw")))
+    return EFI_VAR_ASCII;
+
+  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
+    return EFI_VAR_UINT8;
+
+  if (!grub_strncmp (type, "hex", sizeof("hex")))
+    return EFI_VAR_HEX;
+
+  if (!grub_strncmp (type, "dump", sizeof("dump")))
+    return EFI_VAR_DUMP;
+
+  return EFI_VAR_INVALID;
+}
+
+static int
+grub_print_ascii (char *str, char c)
+{
+  if (grub_iscntrl (c))
+  {
+    switch (c)
+      {
+        case '\0':
+          str[0] = '\\';
+          str[1] = '0';
+          return 2;
+
+        case '\a':
+          str[0] = '\\';
+          str[1] = 'a';
+          return 2;
+
+        case '\b':
+          str[0] = '\\';
+          str[1] = 'b';
+          return 2;
+
+        case '\f':
+          str[0] = '\\';
+          str[1] = 'f';
+          return 2;
+
+        case '\n':
+          str[0] = '\\';
+          str[1] = 'n';
+          return 2;
+
+        case '\r':
+          str[0] = '\\';
+          str[1] = 'r';
+          return 2;
+
+        case '\t':
+          str[0] = '\\';
+          str[1] = 't';
+          return 2;
+
+        case '\v':
+          str[0] = '\\';
+          str[1] = 'v';
+          return 2;
+
+        default:
+          str[0] = '.'; /* as in hexdump -C */
+          return 1;
+      }
+  }
+
+  str[0] = c;
+  return 1;
+}
+
+static grub_err_t
+grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
+  int argc, char **args)
+{
+  struct grub_arg_list *state = ctxt->state;
+  grub_err_t status;
+  void *efi_var = NULL;
+  grub_size_t efi_var_size = 0;
+  enum efi_var_type efi_type = EFI_VAR_HEX;
+  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  char *env_var = NULL;
+  grub_size_t i;
+  char *ptr;
+
+  if (1 != argc)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
+
+  if (state[0].set)
+    efi_type = parse_efi_var_type (state[0].arg);
+
+  if (EFI_VAR_INVALID == efi_type)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
+
+  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
+  if (!efi_var || !efi_var_size)
+    {
+      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
+      goto err;
+    }
+
+  switch (efi_type)
+  {
+    case EFI_VAR_ASCII:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+
+      ptr = env_var;
+
+      for (i = 0; i < efi_var_size; i++)
+        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
+      *ptr = '\0';
+      break;
+
+    case EFI_VAR_RAW:
+      env_var = grub_malloc (efi_var_size + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      grub_memcpy (env_var, efi_var, efi_var_size);
+      env_var[efi_var_size] = '\0';
+      break;
+
+    case EFI_VAR_UINT8:
+      env_var = grub_malloc (4);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
+      break;
+
+    case EFI_VAR_HEX:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      for (i = 0; i < efi_var_size; i++)
+        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
*)efi_var)[i]);
+      break;
+
+    case EFI_VAR_DUMP:
+      if (state[1].set)
+        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
variable with dump format specifier"));
+      else
+        {
+          hexdump (0, (char *)efi_var, efi_var_size);
+          status = GRUB_ERR_NONE;
+        }
+      break;
+
+    default:
+      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
in module?)"));
+  }
+
+  if (efi_type != EFI_VAR_DUMP)
+    {
+      if (state[1].set)
+        status = grub_env_set (state[1].arg, env_var);
+      else
+        {
+          grub_printf ("%s\n", (const char *)env_var);
+          status = GRUB_ERR_NONE;
+        }
+    }
+
+err:
+
+  if (env_var)
+    grub_free (env_var);
+
+  if (efi_var)
+    grub_free (efi_var);
+
+  return status;
+}
+
+static grub_extcmd_t cmd = NULL;
+
+GRUB_MOD_INIT (efivar)
+{
+  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
+ N_("Read EFI variable and print it or save its contents to
environment variable."), options);
+}
+
+GRUB_MOD_FINI (efivar)
+{
+  if (cmd)
+    grub_unregister_extcmd (cmd);
+}
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index a6101de..2c8f023 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -728,6 +728,12 @@
 };

 module = {
+  name = efivar;
+  efi = commands/efi/efivar.c;
+  enable = efi;
+};
+
+module = {
   name = blocklist;
   common = commands/blocklist.c;
 };



On Sun, Nov 29, 2015 at 9:00 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 27.11.2015 17:25, Ignat Korchagin пишет:
>>> I would add:
>>> * hex. To print and store in hex form
>>> * utf16. Decode utf16 into utf8. utf16 is a common encoding in EFI land.
>>
>> hex is already part of the latest proposed patch:
>>>> +    case EFI_VAR_HEX:
>>
>> I'm not sure whether utf16 will be useful, as all standard UEFI
>> variables now are using ASCII for string data.
>>
>
> I agree; it can be added later if use case emerges. So far all
> "interesting" cases that use UTF-16 are not limited to strings, and need
> much more elaborate parsing. That is why I mentioned generic format
> specifier before. But this also can be added if needed, overall syntax
> seems to be flexible enough.
>
>> On Fri, Nov 27, 2015 at 2:07 PM, Vladimir 'φ-coder/phcoder' Serbinenko
>> <phcoder@gmail.com> wrote:
>>> On 14.11.2015 05:03, Andrei Borzenkov wrote:
>>>> 13.11.2015 22:42, Ignat Korchagin пишет:
>>>>>>>> +static enum efi_var_type
>>>>>>>> +parse_efi_var_type (const char *type)
>>>>>>>> +{
>>>>>>>> +  if (!grub_strncmp (type, "string", sizeof("string")))
>>>>>>>> +    return EFI_VAR_STRING;
>>>>>>>> +
>>>>>>>
>>>>>>>
>>>>>>> I think this should be "ascii" or "utf8". "string" is too ambiguous
>>>>>>> in UEFI
>>>>>>> environment, it can also mean sequence of UCS-2 characters.
>>>>>> I'm still not sure how exactly GRUB + UEFI interprets "raw buffers"
>>>>>> when printing. Maybe, to avoid confusion, it might be better to
>>>>>> completely remove this option. Basically, you do not want to interpret
>>>>>> raw buffers as strings. For best compatibility "hex" mode should be
>>>>>> promoted, I guess. What do you think?
>>>>> Checked again the UEFI spec. For globally defined variables which are
>>>>> strings they specify ASCII. So if we leave this option, ascii is the
>>>>> best name.
>>>>>
>>>>
>>>> What about
>>>>
>>>>   - ascii - print ASCII characters verbatim, escape non-ASCII in usual
>>>> way (similar to "od -c")
>>>>
>>>>   - raw - simply put raw variable without any interpretation.
>>>>
>>> I would add:
>>> * hex. To print and store in hex form
>>> * utf16. Decode utf16 into utf8. utf16 is a common encoding in EFI land.
>>> I would also skip on raw, at least until we have a real need for it as
>>> we currently are not equiped to handle raw strings in variable contents,
>>> including but not limited to \0 being considered a terminator
>>>> This is better aligned with argument describing output formatting rather
>>>> than attempting to "type" variable.
>>>>
>>>> Alternative (or in addition to) ascii - dump which prints usual pretty
>>>> hex dump of content (hexdump -C). This is handy to interactively look at
>>>> variable content.
>>>>
>>>> Or, and change name from --type to --format :)
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>>
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-02 12:53                         ` Ignat Korchagin
@ 2015-12-02 13:00                           ` Andrei Borzenkov
  2015-12-02 13:05                             ` Vladimir 'phcoder' Serbinenko
  0 siblings, 1 reply; 28+ messages in thread
From: Andrei Borzenkov @ 2015-12-02 13:00 UTC (permalink / raw)
  To: The development of GNU GRUB

On Wed, Dec 2, 2015 at 3:53 PM, Ignat Korchagin <ignat@cloudflare.com> wrote:
> Let's add utf16 later then (if needed). I also noticed a typo in one
> error string in my above patch (it was checking that command receives
> one argument, but prints that two arguments expected), so here is the
> updated one.
>

It is much more difficult to remove anything than to add it later.
Unless we have use case right now, I'd skip it.

> So, finally, should I remove the "raw" parser (as per Vladimir's
> request). Personally, I would keep it. It might be useful to modify
> the hexdump command, that it can dump the contents of a variable. Then
> those pieces will fit together: storing raw value in the variable and
> examining it with hexdump cmd.
>

hexdump does not support showing variable if that is what you mean. By
the same logic, unless we have current use case we probably should not
add it initially.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-02 13:00                           ` Andrei Borzenkov
@ 2015-12-02 13:05                             ` Vladimir 'phcoder' Serbinenko
  2015-12-02 14:52                               ` Ignat Korchagin
  0 siblings, 1 reply; 28+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2015-12-02 13:05 UTC (permalink / raw)
  To: The development of GRUB 2

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

Le 2 déc. 2015 2:01 PM, "Andrei Borzenkov" <arvidjaar@gmail.com> a écrit :

>

> On Wed, Dec 2, 2015 at 3:53 PM, Ignat Korchagin <ignat@cloudflare.com>
wrote:

> > Let's add utf16 later then (if needed). I also noticed a typo in one

> > error string in my above patch (it was checking that command receives

> > one argument, but prints that two arguments expected), so here is the

> > updated one.

> >

>

> It is much more difficult to remove anything than to add it later.

> Unless we have use case right now, I'd skip it.

>

> > So, finally, should I remove the "raw" parser (as per Vladimir's

> > request). Personally, I would keep it. It might be useful to modify

> > the hexdump command, that it can dump the contents of a variable. Then

> > those pieces will fit together: storing raw value in the variable and

> > examining it with hexdump cmd.

> >

>

> hexdump does not support showing variable if that is what you mean. By

> the same logic, unless we have current use case we probably should not

> add it initially.

>

We don't even support storing non-utf8 in a variable. So please skip it

> _______________________________________________

> Grub-devel mailing list

> Grub-devel@gnu.org

> https://lists.gnu.org/mailman/listinfo/grub-devel

[-- Attachment #2: Type: text/html, Size: 1769 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-02 13:05                             ` Vladimir 'phcoder' Serbinenko
@ 2015-12-02 14:52                               ` Ignat Korchagin
  2015-12-03 16:43                                 ` Andrei Borzenkov
  0 siblings, 1 reply; 28+ messages in thread
From: Ignat Korchagin @ 2015-12-02 14:52 UTC (permalink / raw)
  To: The development of GNU GRUB

OK. Updated patch below:

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 0cc40bb..aa7b927 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -735,6 +735,12 @@ module = {
 };

 module = {
+  name = efivar;
+  efi = commands/efi/efivar.c;
+  enable = efi;
+};
+
+module = {
   name = blocklist;
   common = commands/blocklist.c;
 };
diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
new file mode 100644
index 0000000..8acd0c2
--- /dev/null
+++ b/grub-core/commands/efi/efivar.c
@@ -0,0 +1,238 @@
+/* efivar.c - Read EFI global variables. */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2015 Free Software Foundation, Inc.
+ *  Copyright (C) 2015 CloudFlare, 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/types.h>
+#include <grub/mm.h>
+#include <grub/misc.h>
+#include <grub/efi/api.h>
+#include <grub/efi/efi.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/lib/hexdump.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static const struct grub_arg_option options[] = {
+  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
specific format (hex, uint8, ascii, dump). Default: hex."),
N_("FORMAT"), ARG_TYPE_STRING},
+  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
environment variable (does not work with dump)."), N_("ENV_VAR"),
ARG_TYPE_STRING},
+  {0, 0, 0, 0, 0, 0}
+};
+
+enum efi_var_type
+  {
+    EFI_VAR_ASCII = 0,
+    EFI_VAR_UINT8,
+    EFI_VAR_HEX,
+    EFI_VAR_DUMP,
+    EFI_VAR_INVALID = -1
+  };
+
+static enum efi_var_type
+parse_efi_var_type (const char *type)
+{
+  if (!grub_strncmp (type, "ascii", sizeof("ascii")))
+    return EFI_VAR_ASCII;
+
+  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
+    return EFI_VAR_UINT8;
+
+  if (!grub_strncmp (type, "hex", sizeof("hex")))
+    return EFI_VAR_HEX;
+
+  if (!grub_strncmp (type, "dump", sizeof("dump")))
+    return EFI_VAR_DUMP;
+
+  return EFI_VAR_INVALID;
+}
+
+static int
+grub_print_ascii (char *str, char c)
+{
+  if (grub_iscntrl (c))
+  {
+    switch (c)
+      {
+        case '\0':
+          str[0] = '\\';
+          str[1] = '0';
+          return 2;
+
+        case '\a':
+          str[0] = '\\';
+          str[1] = 'a';
+          return 2;
+
+        case '\b':
+          str[0] = '\\';
+          str[1] = 'b';
+          return 2;
+
+        case '\f':
+          str[0] = '\\';
+          str[1] = 'f';
+          return 2;
+
+        case '\n':
+          str[0] = '\\';
+          str[1] = 'n';
+          return 2;
+
+        case '\r':
+          str[0] = '\\';
+          str[1] = 'r';
+          return 2;
+
+        case '\t':
+          str[0] = '\\';
+          str[1] = 't';
+          return 2;
+
+        case '\v':
+          str[0] = '\\';
+          str[1] = 'v';
+          return 2;
+
+        default:
+          str[0] = '.'; /* as in hexdump -C */
+          return 1;
+      }
+  }
+
+  str[0] = c;
+  return 1;
+}
+
+static grub_err_t
+grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
+  int argc, char **args)
+{
+  struct grub_arg_list *state = ctxt->state;
+  grub_err_t status;
+  void *efi_var = NULL;
+  grub_size_t efi_var_size = 0;
+  enum efi_var_type efi_type = EFI_VAR_HEX;
+  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  char *env_var = NULL;
+  grub_size_t i;
+  char *ptr;
+
+  if (1 != argc)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
+
+  if (state[0].set)
+    efi_type = parse_efi_var_type (state[0].arg);
+
+  if (EFI_VAR_INVALID == efi_type)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
+
+  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
+  if (!efi_var || !efi_var_size)
+    {
+      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
+      goto err;
+    }
+
+  switch (efi_type)
+  {
+    case EFI_VAR_ASCII:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+
+      ptr = env_var;
+
+      for (i = 0; i < efi_var_size; i++)
+        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
+      *ptr = '\0';
+      break;
+
+    case EFI_VAR_UINT8:
+      env_var = grub_malloc (4);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
+      break;
+
+    case EFI_VAR_HEX:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      for (i = 0; i < efi_var_size; i++)
+        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
*)efi_var)[i]);
+      break;
+
+    case EFI_VAR_DUMP:
+      if (state[1].set)
+        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
variable with dump format specifier"));
+      else
+        {
+          hexdump (0, (char *)efi_var, efi_var_size);
+          status = GRUB_ERR_NONE;
+        }
+      break;
+
+    default:
+      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
in module?)"));
+  }
+
+  if (efi_type != EFI_VAR_DUMP)
+    {
+      if (state[1].set)
+        status = grub_env_set (state[1].arg, env_var);
+      else
+        {
+          grub_printf ("%s\n", (const char *)env_var);
+          status = GRUB_ERR_NONE;
+        }
+    }
+
+err:
+
+  if (env_var)
+    grub_free (env_var);
+
+  if (efi_var)
+    grub_free (efi_var);
+
+  return status;
+}
+
+static grub_extcmd_t cmd = NULL;
+
+GRUB_MOD_INIT (efivar)
+{
+  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
+ N_("Read EFI variable and print it or save its contents to
environment variable."), options);
+}
+
+GRUB_MOD_FINI (efivar)
+{
+  if (cmd)
+    grub_unregister_extcmd (cmd);
+}

On Wed, Dec 2, 2015 at 1:05 PM, Vladimir 'phcoder' Serbinenko
<phcoder@gmail.com> wrote:
> Le 2 déc. 2015 2:01 PM, "Andrei Borzenkov" <arvidjaar@gmail.com> a écrit :
>
>>
>
>> On Wed, Dec 2, 2015 at 3:53 PM, Ignat Korchagin <ignat@cloudflare.com>
>> wrote:
>
>> > Let's add utf16 later then (if needed). I also noticed a typo in one
>
>> > error string in my above patch (it was checking that command receives
>
>> > one argument, but prints that two arguments expected), so here is the
>
>> > updated one.
>
>> >
>
>>
>
>> It is much more difficult to remove anything than to add it later.
>
>> Unless we have use case right now, I'd skip it.
>
>>
>
>> > So, finally, should I remove the "raw" parser (as per Vladimir's
>
>> > request). Personally, I would keep it. It might be useful to modify
>
>> > the hexdump command, that it can dump the contents of a variable. Then
>
>> > those pieces will fit together: storing raw value in the variable and
>
>> > examining it with hexdump cmd.
>
>> >
>
>>
>
>> hexdump does not support showing variable if that is what you mean. By
>
>> the same logic, unless we have current use case we probably should not
>
>> add it initially.
>
>>
>
> We don't even support storing non-utf8 in a variable. So please skip it
>
>> _______________________________________________
>
>> Grub-devel mailing list
>
>> Grub-devel@gnu.org
>
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-02 14:52                               ` Ignat Korchagin
@ 2015-12-03 16:43                                 ` Andrei Borzenkov
  2015-12-03 17:19                                   ` Ignat Korchagin
  0 siblings, 1 reply; 28+ messages in thread
From: Andrei Borzenkov @ 2015-12-03 16:43 UTC (permalink / raw)
  To: The development of GNU GRUB

02.12.2015 17:52, Ignat Korchagin пишет:
> OK. Updated patch below:
> 
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 0cc40bb..aa7b927 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -735,6 +735,12 @@ module = {
>  };
> 
>  module = {
> +  name = efivar;
> +  efi = commands/efi/efivar.c;
> +  enable = efi;
> +};
> +
> +module = {
>    name = blocklist;
>    common = commands/blocklist.c;
>  };
> diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
> new file mode 100644
> index 0000000..8acd0c2
> --- /dev/null
> +++ b/grub-core/commands/efi/efivar.c
> @@ -0,0 +1,238 @@
> +/* efivar.c - Read EFI global variables. */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015 Free Software Foundation, Inc.
> + *  Copyright (C) 2015 CloudFlare, 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/types.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +#include <grub/extcmd.h>
> +#include <grub/env.h>
> +#include <grub/lib/hexdump.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static const struct grub_arg_option options[] = {
> +  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
> specific format (hex, uint8, ascii, dump). Default: hex."),
> N_("FORMAT"), ARG_TYPE_STRING},
> +  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
> environment variable (does not work with dump)."), N_("ENV_VAR"),
> ARG_TYPE_STRING},
> +  {0, 0, 0, 0, 0, 0}
> +};
> +
> +enum efi_var_type
> +  {
> +    EFI_VAR_ASCII = 0,
> +    EFI_VAR_UINT8,
> +    EFI_VAR_HEX,
> +    EFI_VAR_DUMP,
> +    EFI_VAR_INVALID = -1
> +  };
> +
> +static enum efi_var_type
> +parse_efi_var_type (const char *type)
> +{
> +  if (!grub_strncmp (type, "ascii", sizeof("ascii")))
> +    return EFI_VAR_ASCII;
> +
> +  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
> +    return EFI_VAR_UINT8;
> +
> +  if (!grub_strncmp (type, "hex", sizeof("hex")))
> +    return EFI_VAR_HEX;
> +
> +  if (!grub_strncmp (type, "dump", sizeof("dump")))
> +    return EFI_VAR_DUMP;
> +
> +  return EFI_VAR_INVALID;
> +}
> +

This should be grub_strcmp everywhere, there is no reason to use
grub_strncmp.
> +
> +GRUB_MOD_INIT (efivar)
> +{
> +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
> N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
> + N_("Read EFI variable and print it or save its contents to

I believe it should be "content" (without final `s') but English is not
native.

Otherwise OK from my side.

Could you also add documentation part for it?



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-03 16:43                                 ` Andrei Borzenkov
@ 2015-12-03 17:19                                   ` Ignat Korchagin
  2015-12-09 14:42                                     ` Ignat Korchagin
  2015-12-12  8:36                                     ` Andrei Borzenkov
  0 siblings, 2 replies; 28+ messages in thread
From: Ignat Korchagin @ 2015-12-03 17:19 UTC (permalink / raw)
  To: The development of GNU GRUB

> This should be grub_strcmp everywhere, there is no reason to use grub_strncmp.
Yes, you are right. Please, find updated patch below

> I believe it should be "content" (without final `s') but English is not native
Mine not as well, but asked my native colleagues and they confirmed
"contents". Also this aligns with Google search.

> Could you also add documentation part for it?
Yes, sure. Could you give me a hint how the project handles
documentation? I didn't find the overall module descriptions, but,
maybe, I didn't look very well :)

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 0cc40bb..aa7b927 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -735,6 +735,12 @@ module = {
 };

 module = {
+  name = efivar;
+  efi = commands/efi/efivar.c;
+  enable = efi;
+};
+
+module = {
   name = blocklist;
   common = commands/blocklist.c;
 };
diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
new file mode 100644
index 0000000..8acd0c2
--- /dev/null
+++ b/grub-core/commands/efi/efivar.c
@@ -0,0 +1,238 @@
+/* efivar.c - Read EFI global variables. */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2015 Free Software Foundation, Inc.
+ *  Copyright (C) 2015 CloudFlare, 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/types.h>
+#include <grub/mm.h>
+#include <grub/misc.h>
+#include <grub/efi/api.h>
+#include <grub/efi/efi.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/lib/hexdump.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static const struct grub_arg_option options[] = {
+  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
specific format (hex, uint8, ascii, dump). Default: hex."),
N_("FORMAT"), ARG_TYPE_STRING},
+  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
environment variable (does not work with dump)."), N_("ENV_VAR"),
ARG_TYPE_STRING},
+  {0, 0, 0, 0, 0, 0}
+};
+
+enum efi_var_type
+  {
+    EFI_VAR_ASCII = 0,
+    EFI_VAR_UINT8,
+    EFI_VAR_HEX,
+    EFI_VAR_DUMP,
+    EFI_VAR_INVALID = -1
+  };
+
+static enum efi_var_type
+parse_efi_var_type (const char *type)
+{
+  if (!grub_strcmp (type, "ascii"))
+    return EFI_VAR_ASCII;
+
+  if (!grub_strcmp (type, "uint8"))
+    return EFI_VAR_UINT8;
+
+  if (!grub_strcmp (type, "hex"))
+    return EFI_VAR_HEX;
+
+  if (!grub_strcmp (type, "dump"))
+    return EFI_VAR_DUMP;
+
+  return EFI_VAR_INVALID;
+}
+
+static int
+grub_print_ascii (char *str, char c)
+{
+  if (grub_iscntrl (c))
+  {
+    switch (c)
+      {
+        case '\0':
+          str[0] = '\\';
+          str[1] = '0';
+          return 2;
+
+        case '\a':
+          str[0] = '\\';
+          str[1] = 'a';
+          return 2;
+
+        case '\b':
+          str[0] = '\\';
+          str[1] = 'b';
+          return 2;
+
+        case '\f':
+          str[0] = '\\';
+          str[1] = 'f';
+          return 2;
+
+        case '\n':
+          str[0] = '\\';
+          str[1] = 'n';
+          return 2;
+
+        case '\r':
+          str[0] = '\\';
+          str[1] = 'r';
+          return 2;
+
+        case '\t':
+          str[0] = '\\';
+          str[1] = 't';
+          return 2;
+
+        case '\v':
+          str[0] = '\\';
+          str[1] = 'v';
+          return 2;
+
+        default:
+          str[0] = '.'; /* as in hexdump -C */
+          return 1;
+      }
+  }
+
+  str[0] = c;
+  return 1;
+}
+
+static grub_err_t
+grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
+  int argc, char **args)
+{
+  struct grub_arg_list *state = ctxt->state;
+  grub_err_t status;
+  void *efi_var = NULL;
+  grub_size_t efi_var_size = 0;
+  enum efi_var_type efi_type = EFI_VAR_HEX;
+  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  char *env_var = NULL;
+  grub_size_t i;
+  char *ptr;
+
+  if (1 != argc)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
+
+  if (state[0].set)
+    efi_type = parse_efi_var_type (state[0].arg);
+
+  if (EFI_VAR_INVALID == efi_type)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
+
+  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
+  if (!efi_var || !efi_var_size)
+    {
+      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
+      goto err;
+    }
+
+  switch (efi_type)
+  {
+    case EFI_VAR_ASCII:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+
+      ptr = env_var;
+
+      for (i = 0; i < efi_var_size; i++)
+        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
+      *ptr = '\0';
+      break;
+
+    case EFI_VAR_UINT8:
+      env_var = grub_malloc (4);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
+      break;
+
+    case EFI_VAR_HEX:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          break;
+        }
+      for (i = 0; i < efi_var_size; i++)
+        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
*)efi_var)[i]);
+      break;
+
+    case EFI_VAR_DUMP:
+      if (state[1].set)
+        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
variable with dump format specifier"));
+      else
+        {
+          hexdump (0, (char *)efi_var, efi_var_size);
+          status = GRUB_ERR_NONE;
+        }
+      break;
+
+    default:
+      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
in module?)"));
+  }
+
+  if (efi_type != EFI_VAR_DUMP)
+    {
+      if (state[1].set)
+        status = grub_env_set (state[1].arg, env_var);
+      else
+        {
+          grub_printf ("%s\n", (const char *)env_var);
+          status = GRUB_ERR_NONE;
+        }
+    }
+
+err:
+
+  if (env_var)
+    grub_free (env_var);
+
+  if (efi_var)
+    grub_free (efi_var);
+
+  return status;
+}
+
+static grub_extcmd_t cmd = NULL;
+
+GRUB_MOD_INIT (efivar)
+{
+  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
+ N_("Read EFI variable and print it or save its contents to
environment variable."), options);
+}
+
+GRUB_MOD_FINI (efivar)
+{
+  if (cmd)
+    grub_unregister_extcmd (cmd);
+}


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-03 17:19                                   ` Ignat Korchagin
@ 2015-12-09 14:42                                     ` Ignat Korchagin
  2015-12-12  8:36                                     ` Andrei Borzenkov
  1 sibling, 0 replies; 28+ messages in thread
From: Ignat Korchagin @ 2015-12-09 14:42 UTC (permalink / raw)
  To: The development of GNU GRUB

Is this still considered?

On Thu, Dec 3, 2015 at 5:19 PM, Ignat Korchagin <ignat@cloudflare.com> wrote:
>> This should be grub_strcmp everywhere, there is no reason to use grub_strncmp.
> Yes, you are right. Please, find updated patch below
>
>> I believe it should be "content" (without final `s') but English is not native
> Mine not as well, but asked my native colleagues and they confirmed
> "contents". Also this aligns with Google search.
>
>> Could you also add documentation part for it?
> Yes, sure. Could you give me a hint how the project handles
> documentation? I didn't find the overall module descriptions, but,
> maybe, I didn't look very well :)
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 0cc40bb..aa7b927 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -735,6 +735,12 @@ module = {
>  };
>
>  module = {
> +  name = efivar;
> +  efi = commands/efi/efivar.c;
> +  enable = efi;
> +};
> +
> +module = {
>    name = blocklist;
>    common = commands/blocklist.c;
>  };
> diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
> new file mode 100644
> index 0000000..8acd0c2
> --- /dev/null
> +++ b/grub-core/commands/efi/efivar.c
> @@ -0,0 +1,238 @@
> +/* efivar.c - Read EFI global variables. */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015 Free Software Foundation, Inc.
> + *  Copyright (C) 2015 CloudFlare, 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/types.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +#include <grub/extcmd.h>
> +#include <grub/env.h>
> +#include <grub/lib/hexdump.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static const struct grub_arg_option options[] = {
> +  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
> specific format (hex, uint8, ascii, dump). Default: hex."),
> N_("FORMAT"), ARG_TYPE_STRING},
> +  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
> environment variable (does not work with dump)."), N_("ENV_VAR"),
> ARG_TYPE_STRING},
> +  {0, 0, 0, 0, 0, 0}
> +};
> +
> +enum efi_var_type
> +  {
> +    EFI_VAR_ASCII = 0,
> +    EFI_VAR_UINT8,
> +    EFI_VAR_HEX,
> +    EFI_VAR_DUMP,
> +    EFI_VAR_INVALID = -1
> +  };
> +
> +static enum efi_var_type
> +parse_efi_var_type (const char *type)
> +{
> +  if (!grub_strcmp (type, "ascii"))
> +    return EFI_VAR_ASCII;
> +
> +  if (!grub_strcmp (type, "uint8"))
> +    return EFI_VAR_UINT8;
> +
> +  if (!grub_strcmp (type, "hex"))
> +    return EFI_VAR_HEX;
> +
> +  if (!grub_strcmp (type, "dump"))
> +    return EFI_VAR_DUMP;
> +
> +  return EFI_VAR_INVALID;
> +}
> +
> +static int
> +grub_print_ascii (char *str, char c)
> +{
> +  if (grub_iscntrl (c))
> +  {
> +    switch (c)
> +      {
> +        case '\0':
> +          str[0] = '\\';
> +          str[1] = '0';
> +          return 2;
> +
> +        case '\a':
> +          str[0] = '\\';
> +          str[1] = 'a';
> +          return 2;
> +
> +        case '\b':
> +          str[0] = '\\';
> +          str[1] = 'b';
> +          return 2;
> +
> +        case '\f':
> +          str[0] = '\\';
> +          str[1] = 'f';
> +          return 2;
> +
> +        case '\n':
> +          str[0] = '\\';
> +          str[1] = 'n';
> +          return 2;
> +
> +        case '\r':
> +          str[0] = '\\';
> +          str[1] = 'r';
> +          return 2;
> +
> +        case '\t':
> +          str[0] = '\\';
> +          str[1] = 't';
> +          return 2;
> +
> +        case '\v':
> +          str[0] = '\\';
> +          str[1] = 'v';
> +          return 2;
> +
> +        default:
> +          str[0] = '.'; /* as in hexdump -C */
> +          return 1;
> +      }
> +  }
> +
> +  str[0] = c;
> +  return 1;
> +}
> +
> +static grub_err_t
> +grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
> +  int argc, char **args)
> +{
> +  struct grub_arg_list *state = ctxt->state;
> +  grub_err_t status;
> +  void *efi_var = NULL;
> +  grub_size_t efi_var_size = 0;
> +  enum efi_var_type efi_type = EFI_VAR_HEX;
> +  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> +  char *env_var = NULL;
> +  grub_size_t i;
> +  char *ptr;
> +
> +  if (1 != argc)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
> +
> +  if (state[0].set)
> +    efi_type = parse_efi_var_type (state[0].arg);
> +
> +  if (EFI_VAR_INVALID == efi_type)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
> +
> +  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
> +  if (!efi_var || !efi_var_size)
> +    {
> +      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
> +      goto err;
> +    }
> +
> +  switch (efi_type)
> +  {
> +    case EFI_VAR_ASCII:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +
> +      ptr = env_var;
> +
> +      for (i = 0; i < efi_var_size; i++)
> +        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
> +      *ptr = '\0';
> +      break;
> +
> +    case EFI_VAR_UINT8:
> +      env_var = grub_malloc (4);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
> +      break;
> +
> +    case EFI_VAR_HEX:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +      for (i = 0; i < efi_var_size; i++)
> +        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
> *)efi_var)[i]);
> +      break;
> +
> +    case EFI_VAR_DUMP:
> +      if (state[1].set)
> +        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
> variable with dump format specifier"));
> +      else
> +        {
> +          hexdump (0, (char *)efi_var, efi_var_size);
> +          status = GRUB_ERR_NONE;
> +        }
> +      break;
> +
> +    default:
> +      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
> in module?)"));
> +  }
> +
> +  if (efi_type != EFI_VAR_DUMP)
> +    {
> +      if (state[1].set)
> +        status = grub_env_set (state[1].arg, env_var);
> +      else
> +        {
> +          grub_printf ("%s\n", (const char *)env_var);
> +          status = GRUB_ERR_NONE;
> +        }
> +    }
> +
> +err:
> +
> +  if (env_var)
> +    grub_free (env_var);
> +
> +  if (efi_var)
> +    grub_free (efi_var);
> +
> +  return status;
> +}
> +
> +static grub_extcmd_t cmd = NULL;
> +
> +GRUB_MOD_INIT (efivar)
> +{
> +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
> N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
> + N_("Read EFI variable and print it or save its contents to
> environment variable."), options);
> +}
> +
> +GRUB_MOD_FINI (efivar)
> +{
> +  if (cmd)
> +    grub_unregister_extcmd (cmd);
> +}


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-03 17:19                                   ` Ignat Korchagin
  2015-12-09 14:42                                     ` Ignat Korchagin
@ 2015-12-12  8:36                                     ` Andrei Borzenkov
  2015-12-14 11:08                                       ` Ignat Korchagin
  1 sibling, 1 reply; 28+ messages in thread
From: Andrei Borzenkov @ 2015-12-12  8:36 UTC (permalink / raw)
  To: grub-devel

03.12.2015 20:19, Ignat Korchagin пишет:
>> This should be grub_strcmp everywhere, there is no reason to use grub_strncmp.
> Yes, you are right. Please, find updated patch below
> 
>> I believe it should be "content" (without final `s') but English is not native
> Mine not as well, but asked my native colleagues and they confirmed
> "contents". Also this aligns with Google search.
> 
>> Could you also add documentation part for it?
> Yes, sure. Could you give me a hint how the project handles
> documentation? I didn't find the overall module descriptions, but,
> maybe, I didn't look very well :)
> 

Documentation is maintained in texinfo format in doc/grub.texi. It is
lacking, but there is list of available commands with index, so just
adding it would be good. It should ideally be a bit more verbose
(examples are welcome) than usage printed at run time.

> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 0cc40bb..aa7b927 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -735,6 +735,12 @@ module = {
>  };
> 
>  module = {
> +  name = efivar;
> +  efi = commands/efi/efivar.c;
> +  enable = efi;
> +};
> +
> +module = {
>    name = blocklist;
>    common = commands/blocklist.c;
>  };
> diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
> new file mode 100644
> index 0000000..8acd0c2
> --- /dev/null
> +++ b/grub-core/commands/efi/efivar.c
> @@ -0,0 +1,238 @@
> +/* efivar.c - Read EFI global variables. */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015 Free Software Foundation, Inc.
> + *  Copyright (C) 2015 CloudFlare, 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/types.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +#include <grub/extcmd.h>
> +#include <grub/env.h>
> +#include <grub/lib/hexdump.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static const struct grub_arg_option options[] = {
> +  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
> specific format (hex, uint8, ascii, dump). Default: hex."),

Do we really need unit8 at all? "hex" already provides exactly the same
functionality, not? Do you think there are cases when uint8 is really
required?

If we remove unit8, this becomes really "Display EFI_VAR" not "Parse
EFI_VAR", as no parsing really happens.

> N_("FORMAT"), ARG_TYPE_STRING},
> +  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
> environment variable (does not work with dump)."), N_("ENV_VAR"),
> ARG_TYPE_STRING},
> +  {0, 0, 0, 0, 0, 0}
> +};
> +
> +enum efi_var_type
> +  {
> +    EFI_VAR_ASCII = 0,
> +    EFI_VAR_UINT8,
> +    EFI_VAR_HEX,
> +    EFI_VAR_DUMP,
> +    EFI_VAR_INVALID = -1
> +  };
> +
> +static enum efi_var_type
> +parse_efi_var_type (const char *type)
> +{
> +  if (!grub_strcmp (type, "ascii"))
> +    return EFI_VAR_ASCII;
> +
> +  if (!grub_strcmp (type, "uint8"))
> +    return EFI_VAR_UINT8;
> +
> +  if (!grub_strcmp (type, "hex"))
> +    return EFI_VAR_HEX;
> +
> +  if (!grub_strcmp (type, "dump"))
> +    return EFI_VAR_DUMP;
> +
> +  return EFI_VAR_INVALID;
> +}
> +
> +static int
> +grub_print_ascii (char *str, char c)
> +{
> +  if (grub_iscntrl (c))
> +  {
> +    switch (c)
> +      {
> +        case '\0':
> +          str[0] = '\\';
> +          str[1] = '0';
> +          return 2;
> +
> +        case '\a':
> +          str[0] = '\\';
> +          str[1] = 'a';
> +          return 2;
> +
> +        case '\b':
> +          str[0] = '\\';
> +          str[1] = 'b';
> +          return 2;
> +
> +        case '\f':
> +          str[0] = '\\';
> +          str[1] = 'f';
> +          return 2;
> +
> +        case '\n':
> +          str[0] = '\\';
> +          str[1] = 'n';
> +          return 2;
> +
> +        case '\r':
> +          str[0] = '\\';
> +          str[1] = 'r';
> +          return 2;
> +
> +        case '\t':
> +          str[0] = '\\';
> +          str[1] = 't';
> +          return 2;
> +
> +        case '\v':
> +          str[0] = '\\';
> +          str[1] = 'v';
> +          return 2;
> +
> +        default:
> +          str[0] = '.'; /* as in hexdump -C */
> +          return 1;
> +      }
> +  }
> +
> +  str[0] = c;
> +  return 1;
> +}
> +
> +static grub_err_t
> +grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
> +  int argc, char **args)
> +{
> +  struct grub_arg_list *state = ctxt->state;
> +  grub_err_t status;
> +  void *efi_var = NULL;
> +  grub_size_t efi_var_size = 0;
> +  enum efi_var_type efi_type = EFI_VAR_HEX;
> +  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> +  char *env_var = NULL;
> +  grub_size_t i;
> +  char *ptr;
> +
> +  if (1 != argc)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
> +
> +  if (state[0].set)
> +    efi_type = parse_efi_var_type (state[0].arg);
> +
> +  if (EFI_VAR_INVALID == efi_type)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
> +
> +  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
> +  if (!efi_var || !efi_var_size)
> +    {
> +      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
> +      goto err;
> +    }
> +
> +  switch (efi_type)
> +  {
> +    case EFI_VAR_ASCII:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;

goto err; see below

> +        }
> +
> +      ptr = env_var;
> +
> +      for (i = 0; i < efi_var_size; i++)
> +        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
> +      *ptr = '\0';
> +      break;
> +
> +    case EFI_VAR_UINT8:
> +      env_var = grub_malloc (4);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;
> +        }
> +      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));

Assuming uint8 remains - should not you check that variable size is
exactly 1 byte in this case?

> +      break;
> +
> +    case EFI_VAR_HEX:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          break;

goto err

> +        }
> +      for (i = 0; i < efi_var_size; i++)
> +        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
> *)efi_var)[i]);
> +      break;
> +
> +    case EFI_VAR_DUMP:
> +      if (state[1].set)
> +        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
> variable with dump format specifier"));
> +      else
> +        {
> +          hexdump (0, (char *)efi_var, efi_var_size);
> +          status = GRUB_ERR_NONE;
> +        }
> +      break;
> +
> +    default:
> +      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
> in module?)"));

goto err

> +  }
> +
> +  if (efi_type != EFI_VAR_DUMP)
> +    {
> +      if (state[1].set)
> +        status = grub_env_set (state[1].arg, env_var);
> +      else
> +        {
> +          grub_printf ("%s\n", (const char *)env_var);
> +          status = GRUB_ERR_NONE;
> +        }
> +    }
> +

In OOM case you access NULL pointer here.

> +err:
> +
> +  if (env_var)
> +    grub_free (env_var);
> +
> +  if (efi_var)
> +    grub_free (efi_var);
> +

grub_free already checks for NULL, no need to do it extra.

> +  return status;
> +}
> +
> +static grub_extcmd_t cmd = NULL;
> +
> +GRUB_MOD_INIT (efivar)
> +{
> +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
> N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
> + N_("Read EFI variable and print it or save its contents to
> environment variable."), options);
> +}
> +
> +GRUB_MOD_FINI (efivar)
> +{
> +  if (cmd)
> +    grub_unregister_extcmd (cmd);
> +}
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-12  8:36                                     ` Andrei Borzenkov
@ 2015-12-14 11:08                                       ` Ignat Korchagin
  2015-12-14 11:17                                         ` Ignat Korchagin
  0 siblings, 1 reply; 28+ messages in thread
From: Ignat Korchagin @ 2015-12-14 11:08 UTC (permalink / raw)
  To: The development of GNU GRUB

> Assuming uint8 remains - should not you check that variable size is exactly 1 byte in this case?
There are reports of a buggy firmware returning 4 bytes size for uint8
variables, however did not encounter them myself.

> Do we really need unit8 at all? "hex" already provides exactly the same functionality, not? Do you think there are cases when uint8 is really required?
Well, when checking for SecureBoot variable in grub configuration file
hex mode makes it look weird and creates a point of confusion. For
example to check if SecureBoot (suppose the result of the our command
is stored in secure_boot env variable in hex mode) is enabled one
should write:
if [ secure_boot = "01" ]
...
uint8 just allows to do a more straightforward config
if [ secure_boot = 1] - this case would be false for hex mode -
possible security breach
...

Added goto err in the module as pointed, see patch below. I will do a
follow-up patch for documentation once we get this confirmed.

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 0cc40bb..aa7b927 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -735,6 +735,12 @@ module = {
 };

 module = {
+  name = efivar;
+  efi = commands/efi/efivar.c;
+  enable = efi;
+};
+
+module = {
   name = blocklist;
   common = commands/blocklist.c;
 };
diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
new file mode 100644
index 0000000..7f5a957
--- /dev/null
+++ b/grub-core/commands/efi/efivar.c
@@ -0,0 +1,251 @@
+/* efivar.c - Read EFI global variables. */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2015 Free Software Foundation, Inc.
+ *  Copyright (C) 2015 CloudFlare, 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/types.h>
+#include <grub/mm.h>
+#include <grub/misc.h>
+#include <grub/efi/api.h>
+#include <grub/efi/efi.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/lib/hexdump.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static const struct grub_arg_option options[] = {
+  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
specific format (hex, uint8, ascii, raw, dump). Default: hex."),
N_("FORMAT"), ARG_TYPE_STRING},
+  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
environment variable (does not work with dump)."), N_("ENV_VAR"),
ARG_TYPE_STRING},
+  {0, 0, 0, 0, 0, 0}
+};
+
+enum efi_var_type
+  {
+    EFI_VAR_ASCII = 0,
+    EFI_VAR_RAW,
+    EFI_VAR_UINT8,
+    EFI_VAR_HEX,
+    EFI_VAR_DUMP,
+    EFI_VAR_INVALID = -1
+  };
+
+static enum efi_var_type
+parse_efi_var_type (const char *type)
+{
+  if (!grub_strncmp (type, "ascii", sizeof("ascii")))
+    return EFI_VAR_ASCII;
+
+  if (!grub_strncmp (type, "raw", sizeof("raw")))
+    return EFI_VAR_ASCII;
+
+  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
+    return EFI_VAR_UINT8;
+
+  if (!grub_strncmp (type, "hex", sizeof("hex")))
+    return EFI_VAR_HEX;
+
+  if (!grub_strncmp (type, "dump", sizeof("dump")))
+    return EFI_VAR_DUMP;
+
+  return EFI_VAR_INVALID;
+}
+
+static int
+grub_print_ascii (char *str, char c)
+{
+  if (grub_iscntrl (c))
+  {
+    switch (c)
+      {
+        case '\0':
+          str[0] = '\\';
+          str[1] = '0';
+          return 2;
+
+        case '\a':
+          str[0] = '\\';
+          str[1] = 'a';
+          return 2;
+
+        case '\b':
+          str[0] = '\\';
+          str[1] = 'b';
+          return 2;
+
+        case '\f':
+          str[0] = '\\';
+          str[1] = 'f';
+          return 2;
+
+        case '\n':
+          str[0] = '\\';
+          str[1] = 'n';
+          return 2;
+
+        case '\r':
+          str[0] = '\\';
+          str[1] = 'r';
+          return 2;
+
+        case '\t':
+          str[0] = '\\';
+          str[1] = 't';
+          return 2;
+
+        case '\v':
+          str[0] = '\\';
+          str[1] = 'v';
+          return 2;
+
+        default:
+          str[0] = '.'; /* as in hexdump -C */
+          return 1;
+      }
+  }
+
+  str[0] = c;
+  return 1;
+}
+
+static grub_err_t
+grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
+  int argc, char **args)
+{
+  struct grub_arg_list *state = ctxt->state;
+  grub_err_t status;
+  void *efi_var = NULL;
+  grub_size_t efi_var_size = 0;
+  enum efi_var_type efi_type = EFI_VAR_HEX;
+  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  char *env_var = NULL;
+  grub_size_t i;
+  char *ptr;
+
+  if (1 != argc)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
+
+  if (state[0].set)
+    efi_type = parse_efi_var_type (state[0].arg);
+
+  if (EFI_VAR_INVALID == efi_type)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
+
+  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
+  if (!efi_var || !efi_var_size)
+    {
+      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
+      goto err;
+    }
+
+  switch (efi_type)
+  {
+    case EFI_VAR_ASCII:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          goto err;
+        }
+
+      ptr = env_var;
+
+      for (i = 0; i < efi_var_size; i++)
+        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
+      *ptr = '\0';
+      break;
+
+    case EFI_VAR_RAW:
+      env_var = grub_malloc (efi_var_size + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          goto err;
+        }
+      grub_memcpy (env_var, efi_var, efi_var_size);
+      env_var[efi_var_size] = '\0';
+      break;
+
+    case EFI_VAR_UINT8:
+      env_var = grub_malloc (4);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          goto err;
+        }
+      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
+      break;
+
+    case EFI_VAR_HEX:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          goto err;
+        }
+      for (i = 0; i < efi_var_size; i++)
+        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
*)efi_var)[i]);
+      break;
+
+    case EFI_VAR_DUMP:
+      if (state[1].set)
+        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
variable with dump format specifier"));
+      else
+        {
+          hexdump (0, (char *)efi_var, efi_var_size);
+          status = GRUB_ERR_NONE;
+        }
+      break;
+
+    default:
+      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
in module?)"));
+      goto err;
+  }
+
+  if (efi_type != EFI_VAR_DUMP)
+    {
+      if (state[1].set)
+        status = grub_env_set (state[1].arg, env_var);
+      else
+        {
+          grub_printf ("%s\n", (const char *)env_var);
+          status = GRUB_ERR_NONE;
+        }
+    }
+
+err:
+
+  grub_free (env_var);
+  grub_free (efi_var);
+
+  return status;
+}
+
+static grub_extcmd_t cmd = NULL;
+
+GRUB_MOD_INIT (efivar)
+{
+  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
+ N_("Read EFI variable and print it or save its contents to
environment variable."), options);
+}
+
+GRUB_MOD_FINI (efivar)
+{
+  if (cmd)
+    grub_unregister_extcmd (cmd);
+}


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-14 11:08                                       ` Ignat Korchagin
@ 2015-12-14 11:17                                         ` Ignat Korchagin
  2016-01-29 16:25                                           ` Ignat Korchagin
  0 siblings, 1 reply; 28+ messages in thread
From: Ignat Korchagin @ 2015-12-14 11:17 UTC (permalink / raw)
  To: The development of GNU GRUB

Sorry, pasted wrong file. Here is the correct one:

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 0cc40bb..aa7b927 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -735,6 +735,12 @@ module = {
 };

 module = {
+  name = efivar;
+  efi = commands/efi/efivar.c;
+  enable = efi;
+};
+
+module = {
   name = blocklist;
   common = commands/blocklist.c;
 };
diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
new file mode 100644
index 0000000..7fe7bda
--- /dev/null
+++ b/grub-core/commands/efi/efivar.c
@@ -0,0 +1,236 @@
+/* efivar.c - Read EFI global variables. */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2015 Free Software Foundation, Inc.
+ *  Copyright (C) 2015 CloudFlare, 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/types.h>
+#include <grub/mm.h>
+#include <grub/misc.h>
+#include <grub/efi/api.h>
+#include <grub/efi/efi.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/lib/hexdump.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static const struct grub_arg_option options[] = {
+  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
specific format (hex, uint8, ascii, dump). Default: hex."),
N_("FORMAT"), ARG_TYPE_STRING},
+  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
environment variable (does not work with dump)."), N_("ENV_VAR"),
ARG_TYPE_STRING},
+  {0, 0, 0, 0, 0, 0}
+};
+
+enum efi_var_type
+  {
+    EFI_VAR_ASCII = 0,
+    EFI_VAR_UINT8,
+    EFI_VAR_HEX,
+    EFI_VAR_DUMP,
+    EFI_VAR_INVALID = -1
+  };
+
+static enum efi_var_type
+parse_efi_var_type (const char *type)
+{
+  if (!grub_strcmp (type, "ascii"))
+    return EFI_VAR_ASCII;
+
+  if (!grub_strcmp (type, "uint8"))
+    return EFI_VAR_UINT8;
+
+  if (!grub_strcmp (type, "hex"))
+    return EFI_VAR_HEX;
+
+  if (!grub_strcmp (type, "dump"))
+    return EFI_VAR_DUMP;
+
+  return EFI_VAR_INVALID;
+}
+
+static int
+grub_print_ascii (char *str, char c)
+{
+  if (grub_iscntrl (c))
+  {
+    switch (c)
+      {
+        case '\0':
+          str[0] = '\\';
+          str[1] = '0';
+          return 2;
+
+        case '\a':
+          str[0] = '\\';
+          str[1] = 'a';
+          return 2;
+
+        case '\b':
+          str[0] = '\\';
+          str[1] = 'b';
+          return 2;
+
+        case '\f':
+          str[0] = '\\';
+          str[1] = 'f';
+          return 2;
+
+        case '\n':
+          str[0] = '\\';
+          str[1] = 'n';
+          return 2;
+
+        case '\r':
+          str[0] = '\\';
+          str[1] = 'r';
+          return 2;
+
+        case '\t':
+          str[0] = '\\';
+          str[1] = 't';
+          return 2;
+
+        case '\v':
+          str[0] = '\\';
+          str[1] = 'v';
+          return 2;
+
+        default:
+          str[0] = '.'; /* as in hexdump -C */
+          return 1;
+      }
+  }
+
+  str[0] = c;
+  return 1;
+}
+
+static grub_err_t
+grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
+  int argc, char **args)
+{
+  struct grub_arg_list *state = ctxt->state;
+  grub_err_t status;
+  void *efi_var = NULL;
+  grub_size_t efi_var_size = 0;
+  enum efi_var_type efi_type = EFI_VAR_HEX;
+  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+  char *env_var = NULL;
+  grub_size_t i;
+  char *ptr;
+
+  if (1 != argc)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
+
+  if (state[0].set)
+    efi_type = parse_efi_var_type (state[0].arg);
+
+  if (EFI_VAR_INVALID == efi_type)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
+
+  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
+  if (!efi_var || !efi_var_size)
+    {
+      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
+      goto err;
+    }
+
+  switch (efi_type)
+  {
+    case EFI_VAR_ASCII:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          goto err;
+        }
+
+      ptr = env_var;
+
+      for (i = 0; i < efi_var_size; i++)
+        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
+      *ptr = '\0';
+      break;
+
+    case EFI_VAR_UINT8:
+      env_var = grub_malloc (4);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          goto err;
+        }
+      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
+      break;
+
+    case EFI_VAR_HEX:
+      env_var = grub_malloc (efi_var_size * 2 + 1);
+      if (!env_var)
+        {
+          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
+          goto err;
+        }
+      for (i = 0; i < efi_var_size; i++)
+        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
*)efi_var)[i]);
+      break;
+
+    case EFI_VAR_DUMP:
+      if (state[1].set)
+        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
variable with dump format specifier"));
+      else
+        {
+          hexdump (0, (char *)efi_var, efi_var_size);
+          status = GRUB_ERR_NONE;
+        }
+      break;
+
+    default:
+      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
in module?)"));
+      goto err;
+  }
+
+  if (efi_type != EFI_VAR_DUMP)
+    {
+      if (state[1].set)
+        status = grub_env_set (state[1].arg, env_var);
+      else
+        {
+          grub_printf ("%s\n", (const char *)env_var);
+          status = GRUB_ERR_NONE;
+        }
+    }
+
+err:
+
+  grub_free (env_var);
+  grub_free (efi_var);
+
+  return status;
+}
+
+static grub_extcmd_t cmd = NULL;
+
+GRUB_MOD_INIT (efivar)
+{
+  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
+ N_("Read EFI variable and print it or save its contents to
environment variable."), options);
+}
+
+GRUB_MOD_FINI (efivar)
+{
+  if (cmd)
+    grub_unregister_extcmd (cmd);
+}


On Mon, Dec 14, 2015 at 11:08 AM, Ignat Korchagin <ignat@cloudflare.com> wrote:
>> Assuming uint8 remains - should not you check that variable size is exactly 1 byte in this case?
> There are reports of a buggy firmware returning 4 bytes size for uint8
> variables, however did not encounter them myself.
>
>> Do we really need unit8 at all? "hex" already provides exactly the same functionality, not? Do you think there are cases when uint8 is really required?
> Well, when checking for SecureBoot variable in grub configuration file
> hex mode makes it look weird and creates a point of confusion. For
> example to check if SecureBoot (suppose the result of the our command
> is stored in secure_boot env variable in hex mode) is enabled one
> should write:
> if [ secure_boot = "01" ]
> ...
> uint8 just allows to do a more straightforward config
> if [ secure_boot = 1] - this case would be false for hex mode -
> possible security breach
> ...
>
> Added goto err in the module as pointed, see patch below. I will do a
> follow-up patch for documentation once we get this confirmed.
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 0cc40bb..aa7b927 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -735,6 +735,12 @@ module = {
>  };
>
>  module = {
> +  name = efivar;
> +  efi = commands/efi/efivar.c;
> +  enable = efi;
> +};
> +
> +module = {
>    name = blocklist;
>    common = commands/blocklist.c;
>  };
> diff --git a/grub-core/commands/efi/efivar.c b/grub-core/commands/efi/efivar.c
> new file mode 100644
> index 0000000..7f5a957
> --- /dev/null
> +++ b/grub-core/commands/efi/efivar.c
> @@ -0,0 +1,251 @@
> +/* efivar.c - Read EFI global variables. */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015 Free Software Foundation, Inc.
> + *  Copyright (C) 2015 CloudFlare, 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/types.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +#include <grub/extcmd.h>
> +#include <grub/env.h>
> +#include <grub/lib/hexdump.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static const struct grub_arg_option options[] = {
> +  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
> specific format (hex, uint8, ascii, raw, dump). Default: hex."),
> N_("FORMAT"), ARG_TYPE_STRING},
> +  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
> environment variable (does not work with dump)."), N_("ENV_VAR"),
> ARG_TYPE_STRING},
> +  {0, 0, 0, 0, 0, 0}
> +};
> +
> +enum efi_var_type
> +  {
> +    EFI_VAR_ASCII = 0,
> +    EFI_VAR_RAW,
> +    EFI_VAR_UINT8,
> +    EFI_VAR_HEX,
> +    EFI_VAR_DUMP,
> +    EFI_VAR_INVALID = -1
> +  };
> +
> +static enum efi_var_type
> +parse_efi_var_type (const char *type)
> +{
> +  if (!grub_strncmp (type, "ascii", sizeof("ascii")))
> +    return EFI_VAR_ASCII;
> +
> +  if (!grub_strncmp (type, "raw", sizeof("raw")))
> +    return EFI_VAR_ASCII;
> +
> +  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
> +    return EFI_VAR_UINT8;
> +
> +  if (!grub_strncmp (type, "hex", sizeof("hex")))
> +    return EFI_VAR_HEX;
> +
> +  if (!grub_strncmp (type, "dump", sizeof("dump")))
> +    return EFI_VAR_DUMP;
> +
> +  return EFI_VAR_INVALID;
> +}
> +
> +static int
> +grub_print_ascii (char *str, char c)
> +{
> +  if (grub_iscntrl (c))
> +  {
> +    switch (c)
> +      {
> +        case '\0':
> +          str[0] = '\\';
> +          str[1] = '0';
> +          return 2;
> +
> +        case '\a':
> +          str[0] = '\\';
> +          str[1] = 'a';
> +          return 2;
> +
> +        case '\b':
> +          str[0] = '\\';
> +          str[1] = 'b';
> +          return 2;
> +
> +        case '\f':
> +          str[0] = '\\';
> +          str[1] = 'f';
> +          return 2;
> +
> +        case '\n':
> +          str[0] = '\\';
> +          str[1] = 'n';
> +          return 2;
> +
> +        case '\r':
> +          str[0] = '\\';
> +          str[1] = 'r';
> +          return 2;
> +
> +        case '\t':
> +          str[0] = '\\';
> +          str[1] = 't';
> +          return 2;
> +
> +        case '\v':
> +          str[0] = '\\';
> +          str[1] = 'v';
> +          return 2;
> +
> +        default:
> +          str[0] = '.'; /* as in hexdump -C */
> +          return 1;
> +      }
> +  }
> +
> +  str[0] = c;
> +  return 1;
> +}
> +
> +static grub_err_t
> +grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
> +  int argc, char **args)
> +{
> +  struct grub_arg_list *state = ctxt->state;
> +  grub_err_t status;
> +  void *efi_var = NULL;
> +  grub_size_t efi_var_size = 0;
> +  enum efi_var_type efi_type = EFI_VAR_HEX;
> +  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> +  char *env_var = NULL;
> +  grub_size_t i;
> +  char *ptr;
> +
> +  if (1 != argc)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
> +
> +  if (state[0].set)
> +    efi_type = parse_efi_var_type (state[0].arg);
> +
> +  if (EFI_VAR_INVALID == efi_type)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format specifier"));
> +
> +  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
> +  if (!efi_var || !efi_var_size)
> +    {
> +      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read variable"));
> +      goto err;
> +    }
> +
> +  switch (efi_type)
> +  {
> +    case EFI_VAR_ASCII:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          goto err;
> +        }
> +
> +      ptr = env_var;
> +
> +      for (i = 0; i < efi_var_size; i++)
> +        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
> +      *ptr = '\0';
> +      break;
> +
> +    case EFI_VAR_RAW:
> +      env_var = grub_malloc (efi_var_size + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          goto err;
> +        }
> +      grub_memcpy (env_var, efi_var, efi_var_size);
> +      env_var[efi_var_size] = '\0';
> +      break;
> +
> +    case EFI_VAR_UINT8:
> +      env_var = grub_malloc (4);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          goto err;
> +        }
> +      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
> +      break;
> +
> +    case EFI_VAR_HEX:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory"));
> +          goto err;
> +        }
> +      for (i = 0; i < efi_var_size; i++)
> +        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
> *)efi_var)[i]);
> +      break;
> +
> +    case EFI_VAR_DUMP:
> +      if (state[1].set)
> +        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
> variable with dump format specifier"));
> +      else
> +        {
> +          hexdump (0, (char *)efi_var, efi_var_size);
> +          status = GRUB_ERR_NONE;
> +        }
> +      break;
> +
> +    default:
> +      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
> in module?)"));
> +      goto err;
> +  }
> +
> +  if (efi_type != EFI_VAR_DUMP)
> +    {
> +      if (state[1].set)
> +        status = grub_env_set (state[1].arg, env_var);
> +      else
> +        {
> +          grub_printf ("%s\n", (const char *)env_var);
> +          status = GRUB_ERR_NONE;
> +        }
> +    }
> +
> +err:
> +
> +  grub_free (env_var);
> +  grub_free (efi_var);
> +
> +  return status;
> +}
> +
> +static grub_extcmd_t cmd = NULL;
> +
> +GRUB_MOD_INIT (efivar)
> +{
> +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
> N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
> + N_("Read EFI variable and print it or save its contents to
> environment variable."), options);
> +}
> +
> +GRUB_MOD_FINI (efivar)
> +{
> +  if (cmd)
> +    grub_unregister_extcmd (cmd);
> +}


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: Grub get and set efi variables
  2015-12-14 11:17                                         ` Ignat Korchagin
@ 2016-01-29 16:25                                           ` Ignat Korchagin
  0 siblings, 0 replies; 28+ messages in thread
From: Ignat Korchagin @ 2016-01-29 16:25 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 16677 bytes --]

Hi. Are we still considering this?

On Mon, Dec 14, 2015 at 11:17 AM, Ignat Korchagin <ignat@cloudflare.com>
wrote:

> Sorry, pasted wrong file. Here is the correct one:
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 0cc40bb..aa7b927 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -735,6 +735,12 @@ module = {
>  };
>
>  module = {
> +  name = efivar;
> +  efi = commands/efi/efivar.c;
> +  enable = efi;
> +};
> +
> +module = {
>    name = blocklist;
>    common = commands/blocklist.c;
>  };
> diff --git a/grub-core/commands/efi/efivar.c
> b/grub-core/commands/efi/efivar.c
> new file mode 100644
> index 0000000..7fe7bda
> --- /dev/null
> +++ b/grub-core/commands/efi/efivar.c
> @@ -0,0 +1,236 @@
> +/* efivar.c - Read EFI global variables. */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015 Free Software Foundation, Inc.
> + *  Copyright (C) 2015 CloudFlare, 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/types.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +#include <grub/efi/api.h>
> +#include <grub/efi/efi.h>
> +#include <grub/extcmd.h>
> +#include <grub/env.h>
> +#include <grub/lib/hexdump.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static const struct grub_arg_option options[] = {
> +  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
> specific format (hex, uint8, ascii, dump). Default: hex."),
> N_("FORMAT"), ARG_TYPE_STRING},
> +  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
> environment variable (does not work with dump)."), N_("ENV_VAR"),
> ARG_TYPE_STRING},
> +  {0, 0, 0, 0, 0, 0}
> +};
> +
> +enum efi_var_type
> +  {
> +    EFI_VAR_ASCII = 0,
> +    EFI_VAR_UINT8,
> +    EFI_VAR_HEX,
> +    EFI_VAR_DUMP,
> +    EFI_VAR_INVALID = -1
> +  };
> +
> +static enum efi_var_type
> +parse_efi_var_type (const char *type)
> +{
> +  if (!grub_strcmp (type, "ascii"))
> +    return EFI_VAR_ASCII;
> +
> +  if (!grub_strcmp (type, "uint8"))
> +    return EFI_VAR_UINT8;
> +
> +  if (!grub_strcmp (type, "hex"))
> +    return EFI_VAR_HEX;
> +
> +  if (!grub_strcmp (type, "dump"))
> +    return EFI_VAR_DUMP;
> +
> +  return EFI_VAR_INVALID;
> +}
> +
> +static int
> +grub_print_ascii (char *str, char c)
> +{
> +  if (grub_iscntrl (c))
> +  {
> +    switch (c)
> +      {
> +        case '\0':
> +          str[0] = '\\';
> +          str[1] = '0';
> +          return 2;
> +
> +        case '\a':
> +          str[0] = '\\';
> +          str[1] = 'a';
> +          return 2;
> +
> +        case '\b':
> +          str[0] = '\\';
> +          str[1] = 'b';
> +          return 2;
> +
> +        case '\f':
> +          str[0] = '\\';
> +          str[1] = 'f';
> +          return 2;
> +
> +        case '\n':
> +          str[0] = '\\';
> +          str[1] = 'n';
> +          return 2;
> +
> +        case '\r':
> +          str[0] = '\\';
> +          str[1] = 'r';
> +          return 2;
> +
> +        case '\t':
> +          str[0] = '\\';
> +          str[1] = 't';
> +          return 2;
> +
> +        case '\v':
> +          str[0] = '\\';
> +          str[1] = 'v';
> +          return 2;
> +
> +        default:
> +          str[0] = '.'; /* as in hexdump -C */
> +          return 1;
> +      }
> +  }
> +
> +  str[0] = c;
> +  return 1;
> +}
> +
> +static grub_err_t
> +grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
> +  int argc, char **args)
> +{
> +  struct grub_arg_list *state = ctxt->state;
> +  grub_err_t status;
> +  void *efi_var = NULL;
> +  grub_size_t efi_var_size = 0;
> +  enum efi_var_type efi_type = EFI_VAR_HEX;
> +  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> +  char *env_var = NULL;
> +  grub_size_t i;
> +  char *ptr;
> +
> +  if (1 != argc)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument
> expected"));
> +
> +  if (state[0].set)
> +    efi_type = parse_efi_var_type (state[0].arg);
> +
> +  if (EFI_VAR_INVALID == efi_type)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format
> specifier"));
> +
> +  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
> +  if (!efi_var || !efi_var_size)
> +    {
> +      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read
> variable"));
> +      goto err;
> +    }
> +
> +  switch (efi_type)
> +  {
> +    case EFI_VAR_ASCII:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
> memory"));
> +          goto err;
> +        }
> +
> +      ptr = env_var;
> +
> +      for (i = 0; i < efi_var_size; i++)
> +        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
> +      *ptr = '\0';
> +      break;
> +
> +    case EFI_VAR_UINT8:
> +      env_var = grub_malloc (4);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
> memory"));
> +          goto err;
> +        }
> +      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
> +      break;
> +
> +    case EFI_VAR_HEX:
> +      env_var = grub_malloc (efi_var_size * 2 + 1);
> +      if (!env_var)
> +        {
> +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
> memory"));
> +          goto err;
> +        }
> +      for (i = 0; i < efi_var_size; i++)
> +        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
> *)efi_var)[i]);
> +      break;
> +
> +    case EFI_VAR_DUMP:
> +      if (state[1].set)
> +        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
> variable with dump format specifier"));
> +      else
> +        {
> +          hexdump (0, (char *)efi_var, efi_var_size);
> +          status = GRUB_ERR_NONE;
> +        }
> +      break;
> +
> +    default:
> +      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
> in module?)"));
> +      goto err;
> +  }
> +
> +  if (efi_type != EFI_VAR_DUMP)
> +    {
> +      if (state[1].set)
> +        status = grub_env_set (state[1].arg, env_var);
> +      else
> +        {
> +          grub_printf ("%s\n", (const char *)env_var);
> +          status = GRUB_ERR_NONE;
> +        }
> +    }
> +
> +err:
> +
> +  grub_free (env_var);
> +  grub_free (efi_var);
> +
> +  return status;
> +}
> +
> +static grub_extcmd_t cmd = NULL;
> +
> +GRUB_MOD_INIT (efivar)
> +{
> +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
> N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
> + N_("Read EFI variable and print it or save its contents to
> environment variable."), options);
> +}
> +
> +GRUB_MOD_FINI (efivar)
> +{
> +  if (cmd)
> +    grub_unregister_extcmd (cmd);
> +}
>
>
> On Mon, Dec 14, 2015 at 11:08 AM, Ignat Korchagin <ignat@cloudflare.com>
> wrote:
> >> Assuming uint8 remains - should not you check that variable size is
> exactly 1 byte in this case?
> > There are reports of a buggy firmware returning 4 bytes size for uint8
> > variables, however did not encounter them myself.
> >
> >> Do we really need unit8 at all? "hex" already provides exactly the same
> functionality, not? Do you think there are cases when uint8 is really
> required?
> > Well, when checking for SecureBoot variable in grub configuration file
> > hex mode makes it look weird and creates a point of confusion. For
> > example to check if SecureBoot (suppose the result of the our command
> > is stored in secure_boot env variable in hex mode) is enabled one
> > should write:
> > if [ secure_boot = "01" ]
> > ...
> > uint8 just allows to do a more straightforward config
> > if [ secure_boot = 1] - this case would be false for hex mode -
> > possible security breach
> > ...
> >
> > Added goto err in the module as pointed, see patch below. I will do a
> > follow-up patch for documentation once we get this confirmed.
> >
> > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> > index 0cc40bb..aa7b927 100644
> > --- a/grub-core/Makefile.core.def
> > +++ b/grub-core/Makefile.core.def
> > @@ -735,6 +735,12 @@ module = {
> >  };
> >
> >  module = {
> > +  name = efivar;
> > +  efi = commands/efi/efivar.c;
> > +  enable = efi;
> > +};
> > +
> > +module = {
> >    name = blocklist;
> >    common = commands/blocklist.c;
> >  };
> > diff --git a/grub-core/commands/efi/efivar.c
> b/grub-core/commands/efi/efivar.c
> > new file mode 100644
> > index 0000000..7f5a957
> > --- /dev/null
> > +++ b/grub-core/commands/efi/efivar.c
> > @@ -0,0 +1,251 @@
> > +/* efivar.c - Read EFI global variables. */
> > +/*
> > + *  GRUB  --  GRand Unified Bootloader
> > + *  Copyright (C) 2015 Free Software Foundation, Inc.
> > + *  Copyright (C) 2015 CloudFlare, 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/types.h>
> > +#include <grub/mm.h>
> > +#include <grub/misc.h>
> > +#include <grub/efi/api.h>
> > +#include <grub/efi/efi.h>
> > +#include <grub/extcmd.h>
> > +#include <grub/env.h>
> > +#include <grub/lib/hexdump.h>
> > +
> > +GRUB_MOD_LICENSE ("GPLv3+");
> > +
> > +static const struct grub_arg_option options[] = {
> > +  {"format", 'f', GRUB_ARG_OPTION_OPTIONAL, N_("Parse EFI_VAR in
> > specific format (hex, uint8, ascii, raw, dump). Default: hex."),
> > N_("FORMAT"), ARG_TYPE_STRING},
> > +  {"set", 's', GRUB_ARG_OPTION_OPTIONAL, N_("Save parsed result to
> > environment variable (does not work with dump)."), N_("ENV_VAR"),
> > ARG_TYPE_STRING},
> > +  {0, 0, 0, 0, 0, 0}
> > +};
> > +
> > +enum efi_var_type
> > +  {
> > +    EFI_VAR_ASCII = 0,
> > +    EFI_VAR_RAW,
> > +    EFI_VAR_UINT8,
> > +    EFI_VAR_HEX,
> > +    EFI_VAR_DUMP,
> > +    EFI_VAR_INVALID = -1
> > +  };
> > +
> > +static enum efi_var_type
> > +parse_efi_var_type (const char *type)
> > +{
> > +  if (!grub_strncmp (type, "ascii", sizeof("ascii")))
> > +    return EFI_VAR_ASCII;
> > +
> > +  if (!grub_strncmp (type, "raw", sizeof("raw")))
> > +    return EFI_VAR_ASCII;
> > +
> > +  if (!grub_strncmp (type, "uint8", sizeof("uint8")))
> > +    return EFI_VAR_UINT8;
> > +
> > +  if (!grub_strncmp (type, "hex", sizeof("hex")))
> > +    return EFI_VAR_HEX;
> > +
> > +  if (!grub_strncmp (type, "dump", sizeof("dump")))
> > +    return EFI_VAR_DUMP;
> > +
> > +  return EFI_VAR_INVALID;
> > +}
> > +
> > +static int
> > +grub_print_ascii (char *str, char c)
> > +{
> > +  if (grub_iscntrl (c))
> > +  {
> > +    switch (c)
> > +      {
> > +        case '\0':
> > +          str[0] = '\\';
> > +          str[1] = '0';
> > +          return 2;
> > +
> > +        case '\a':
> > +          str[0] = '\\';
> > +          str[1] = 'a';
> > +          return 2;
> > +
> > +        case '\b':
> > +          str[0] = '\\';
> > +          str[1] = 'b';
> > +          return 2;
> > +
> > +        case '\f':
> > +          str[0] = '\\';
> > +          str[1] = 'f';
> > +          return 2;
> > +
> > +        case '\n':
> > +          str[0] = '\\';
> > +          str[1] = 'n';
> > +          return 2;
> > +
> > +        case '\r':
> > +          str[0] = '\\';
> > +          str[1] = 'r';
> > +          return 2;
> > +
> > +        case '\t':
> > +          str[0] = '\\';
> > +          str[1] = 't';
> > +          return 2;
> > +
> > +        case '\v':
> > +          str[0] = '\\';
> > +          str[1] = 'v';
> > +          return 2;
> > +
> > +        default:
> > +          str[0] = '.'; /* as in hexdump -C */
> > +          return 1;
> > +      }
> > +  }
> > +
> > +  str[0] = c;
> > +  return 1;
> > +}
> > +
> > +static grub_err_t
> > +grub_cmd_get_efi_var (struct grub_extcmd_context *ctxt,
> > +  int argc, char **args)
> > +{
> > +  struct grub_arg_list *state = ctxt->state;
> > +  grub_err_t status;
> > +  void *efi_var = NULL;
> > +  grub_size_t efi_var_size = 0;
> > +  enum efi_var_type efi_type = EFI_VAR_HEX;
> > +  grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> > +  char *env_var = NULL;
> > +  grub_size_t i;
> > +  char *ptr;
> > +
> > +  if (1 != argc)
> > +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument
> expected"));
> > +
> > +  if (state[0].set)
> > +    efi_type = parse_efi_var_type (state[0].arg);
> > +
> > +  if (EFI_VAR_INVALID == efi_type)
> > +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid format
> specifier"));
> > +
> > +  efi_var = grub_efi_get_variable (args[0], &global, &efi_var_size);
> > +  if (!efi_var || !efi_var_size)
> > +    {
> > +      status = grub_error (GRUB_ERR_READ_ERROR, N_("cannot read
> variable"));
> > +      goto err;
> > +    }
> > +
> > +  switch (efi_type)
> > +  {
> > +    case EFI_VAR_ASCII:
> > +      env_var = grub_malloc (efi_var_size * 2 + 1);
> > +      if (!env_var)
> > +        {
> > +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
> memory"));
> > +          goto err;
> > +        }
> > +
> > +      ptr = env_var;
> > +
> > +      for (i = 0; i < efi_var_size; i++)
> > +        ptr += grub_print_ascii (ptr, ((const char *)efi_var)[i]);
> > +      *ptr = '\0';
> > +      break;
> > +
> > +    case EFI_VAR_RAW:
> > +      env_var = grub_malloc (efi_var_size + 1);
> > +      if (!env_var)
> > +        {
> > +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
> memory"));
> > +          goto err;
> > +        }
> > +      grub_memcpy (env_var, efi_var, efi_var_size);
> > +      env_var[efi_var_size] = '\0';
> > +      break;
> > +
> > +    case EFI_VAR_UINT8:
> > +      env_var = grub_malloc (4);
> > +      if (!env_var)
> > +        {
> > +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
> memory"));
> > +          goto err;
> > +        }
> > +      grub_snprintf (env_var, 4, "%u", *((grub_uint8_t *)efi_var));
> > +      break;
> > +
> > +    case EFI_VAR_HEX:
> > +      env_var = grub_malloc (efi_var_size * 2 + 1);
> > +      if (!env_var)
> > +        {
> > +          status = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of
> memory"));
> > +          goto err;
> > +        }
> > +      for (i = 0; i < efi_var_size; i++)
> > +        grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t
> > *)efi_var)[i]);
> > +      break;
> > +
> > +    case EFI_VAR_DUMP:
> > +      if (state[1].set)
> > +        status = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set
> > variable with dump format specifier"));
> > +      else
> > +        {
> > +          hexdump (0, (char *)efi_var, efi_var_size);
> > +          status = GRUB_ERR_NONE;
> > +        }
> > +      break;
> > +
> > +    default:
> > +      status = grub_error (GRUB_ERR_BUG, N_("should not happen (bug
> > in module?)"));
> > +      goto err;
> > +  }
> > +
> > +  if (efi_type != EFI_VAR_DUMP)
> > +    {
> > +      if (state[1].set)
> > +        status = grub_env_set (state[1].arg, env_var);
> > +      else
> > +        {
> > +          grub_printf ("%s\n", (const char *)env_var);
> > +          status = GRUB_ERR_NONE;
> > +        }
> > +    }
> > +
> > +err:
> > +
> > +  grub_free (env_var);
> > +  grub_free (efi_var);
> > +
> > +  return status;
> > +}
> > +
> > +static grub_extcmd_t cmd = NULL;
> > +
> > +GRUB_MOD_INIT (efivar)
> > +{
> > +  cmd = grub_register_extcmd ("get_efivar", grub_cmd_get_efi_var, 0,
> > N_("[-f FORMAT] [-s ENV_VAR] EFI_VAR"),
> > + N_("Read EFI variable and print it or save its contents to
> > environment variable."), options);
> > +}
> > +
> > +GRUB_MOD_FINI (efivar)
> > +{
> > +  if (cmd)
> > +    grub_unregister_extcmd (cmd);
> > +}
>

[-- Attachment #2: Type: text/html, Size: 22168 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2016-01-29 16:25 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-03 19:39 Grub get and set efi variables Mat Troi
2015-11-03 20:12 ` SevenBits
2015-11-03 23:05   ` Mat Troi
2015-11-04  5:38     ` Andrei Borzenkov
2015-11-05 18:25       ` SevenBits
2015-11-06  2:00         ` Ignat Korchagin
2015-11-11 18:09           ` Andrei Borzenkov
2015-11-13 19:34             ` Ignat Korchagin
2015-11-13 19:42               ` Ignat Korchagin
2015-11-14  4:03                 ` Andrei Borzenkov
2015-11-17 11:48                   ` Ignat Korchagin
2015-11-29  8:57                     ` Andrei Borzenkov
2015-11-24 19:23                   ` Mat Troi
2015-11-24 20:48                     ` Seth Goldberg
2015-11-27 14:07                   ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-11-27 14:25                     ` Ignat Korchagin
2015-11-29  9:00                       ` Andrei Borzenkov
2015-12-02 12:53                         ` Ignat Korchagin
2015-12-02 13:00                           ` Andrei Borzenkov
2015-12-02 13:05                             ` Vladimir 'phcoder' Serbinenko
2015-12-02 14:52                               ` Ignat Korchagin
2015-12-03 16:43                                 ` Andrei Borzenkov
2015-12-03 17:19                                   ` Ignat Korchagin
2015-12-09 14:42                                     ` Ignat Korchagin
2015-12-12  8:36                                     ` Andrei Borzenkov
2015-12-14 11:08                                       ` Ignat Korchagin
2015-12-14 11:17                                         ` Ignat Korchagin
2016-01-29 16:25                                           ` Ignat Korchagin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.