From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZxKFQ-0005Ag-IH for mharc-grub-devel@gnu.org; Fri, 13 Nov 2015 14:43:00 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59668) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZxKFM-00058E-OG for grub-devel@gnu.org; Fri, 13 Nov 2015 14:42:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZxKFK-0004YQ-Tr for grub-devel@gnu.org; Fri, 13 Nov 2015 14:42:56 -0500 Received: from mail-yk0-x22b.google.com ([2607:f8b0:4002:c07::22b]:34753) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZxKFK-0004YM-OI for grub-devel@gnu.org; Fri, 13 Nov 2015 14:42:54 -0500 Received: by ykfs79 with SMTP id s79so165146381ykf.1 for ; Fri, 13 Nov 2015 11:42:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cloudflare.com; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type:content-transfer-encoding; bh=+HkRzFaZ3fuvecMXJPbhoaT7l9mrxrpTMHKYd48Nj/A=; b=jl2teypwrfMEXgiZNP7bCC//vIlrmmCvkwXLXZZkYPcccNoQL5EdJG641+LfeCLZ2L v7HlmF5qVm1LBCUOHntb4EsTrStiuFHxMC5xgmAfjN5tX+Y4NdrtAIpB291k8uOOM+93 R9FQqaMZFX9NBitgznvmQpAsKim6kNIXNFQPE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:content-type:content-transfer-encoding; bh=+HkRzFaZ3fuvecMXJPbhoaT7l9mrxrpTMHKYd48Nj/A=; b=ELIP/U9MqSoJgKvWWGK+/xKD3x3JRbIxW+x22nuYcBdAvapL/siBsnvCv4NDuK4RER 08NpBrLFCq2kUKEq3fG2RM9aHoKwfG04HIjuQHQ06kNP15ySvcRFr2r9lARa2yVuecS3 PGSc/ajjaFwMUA8xVk8GtrOrFrtpJL54W3+1o8i1FXJ6sEIE24ocrLvCELuh2mwgHw9o oaQeTJ1E2NdC5rUUFvCQkyGKv6wcVDDKZWdiBZE9xOB/i+mrW+BfK6zXuTfkuaWV8dz+ v7ql+1KYMNzdu4Yy8yYhcLQYKwaIbXK346JTMmgd+vDBzasRbtGIn46OJ/VvWXYW9DGt Y89A== X-Gm-Message-State: ALoCoQkvOHbwixc+Lj83whgVfXChJXZwkw2KFjGBmFCe+lVJ8UwA5g7sx+3Merabn0kcPCcon7d7 MIME-Version: 1.0 X-Received: by 10.13.226.142 with SMTP id l136mr23949744ywe.313.1447443774309; Fri, 13 Nov 2015 11:42:54 -0800 (PST) Received: by 10.129.113.84 with HTTP; Fri, 13 Nov 2015 11:42:54 -0800 (PST) In-Reply-To: References: <563999B9.7020108@gmail.com> <5643845E.9060204@gmail.com> Date: Fri, 13 Nov 2015 11:42:54 -0800 Message-ID: Subject: Re: Grub get and set efi variables From: Ignat Korchagin To: The development of GNU GRUB Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:4002:c07::22b X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Nov 2015 19:42:58 -0000 On Fri, Nov 13, 2015 at 11:34 AM, Ignat Korchagin wr= ote: > On Wed, Nov 11, 2015 at 10:09 AM, Andrei Borzenkov = wrote: >> 06.11.2015 05:00, Ignat Korchagin =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>> >>> 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 extensi= ons. >> 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 =3D { >>> }; >>> >>> module =3D { >>> + name =3D efivar; >>> + efi =3D commands/efi/efivar.c; >>> + enable =3D efi; >>> +}; >>> + >>> +module =3D { >>> name =3D blocklist; >>> common =3D 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 . >>> + */ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +GRUB_MOD_LICENSE ("GPLv3+"); >>> + >>> +static const struct grub_arg_option options[] =3D { >>> + {"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 =3D 0, >>> + EFI_VAR_UINT8, >>> + EFI_VAR_HEX, >>> + EFI_VAR_INVALID =3D -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 U= EFI >> 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 =3D ctxt->state; >>> + grub_err_t status; >>> + void *efi_var =3D NULL; >>> + grub_size_t efi_var_size =3D 0; >>> + enum efi_var_type efi_type =3D EFI_VAR_HEX; >>> + grub_efi_guid_t global =3D GRUB_EFI_GLOBAL_VARIABLE_GUID; >>> + char *env_var =3D NULL; >>> + grub_size_t i; >>> + >>> + if (2 !=3D 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 simpl= y >> display variable on screen, like >> >> efivar OsIndicators >> >> or even >> >> efivar --all >> > Yes. Agree. >>> + if (state[0].set) >>> + efi_type =3D parse_efi_var_type (state[0].arg); >>> + >>> + if (EFI_VAR_INVALID =3D=3D efi_type) >>> + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid EFI variable >>> type")); >>> + >>> + efi_var =3D grub_efi_get_variable (args[0], &global, &efi_var_size); >>> + if (!efi_var || !efi_var_size) >>> + status =3D 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 =3D grub_malloc (efi_var_size + 1); >>> + if (!env_var) >>> + { >>> + status =3D 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] =3D '\0'; >>> + break; >>> + >>> + case EFI_VAR_UINT8: >>> + env_var =3D grub_malloc (4); >>> + if (!env_var) >>> + { >>> + status =3D 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 =3D grub_malloc (efi_var_size * 2 + 1); >>> + if (!env_var) >>> + { >>> + status =3D grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of >>> memory")); >>> + break; >>> + } >>> + for (i =3D 0; i < efi_var_size; i++) >>> + grub_snprintf (env_var + (i * 2), 3, "%02x", ((grub_uint8_t >>> *)efi_var)[i]); >>> + break; >>> + >>> + default: >>> + status =3D grub_error (GRUB_ERR_BUG, N_("should not happen (bug >>> in module?)")); >>> + } >>> + >>> + status =3D 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 =3D NULL; >>> + >>> +GRUB_MOD_INIT (efivar) >>> +{ >>> + cmd =3D 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 >>> wrote: >>>> >>>> On Wednesday, November 4, 2015, Andrei Borzenkov >>>> wrote: >>>>> >>>>> >>>>> 04.11.2015 02:05, Mat Troi =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>>>> >>>>>> >>>>>> 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 comm= and >>>> in >>>> the wrong order. >>>> >>>>> >>>>> >>>>> I am not convinced making it easy to set EFI variable from within GRU= B >>>>> 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 wri= te >>>> functionality for good measure because I could. But I agree, this woul= d >>>> only >>>> encourage tinkering and users messing with their systems and potential= ly >>>> 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 i= t >>>>> 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 hav= e >>>> 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 specifie= d >>>> by >>>> the user. Unless the powers that be think otherwise, I think this is t= he >>>> best way to go. >>>> >>>>> >>>>> >>>>>> Thanks, >>>>>> Mat >>>>>> >>>>>> On Tue, Nov 3, 2015 at 12:12 PM, SevenBits >>>>>> wrote: >>>>>> >>>>>>> On Tuesday, November 3, 2015, Mat Troi wrot= e: >>>>>>> >>>>>>>> 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.htm= l >>>>>>>> >>>>>>>> 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=3Dgrub.git;a=3Dtree;f=3Dgrub= -core/commands/efi;h=3D005fd2efc9a2eede2a1eb1cab8081c360219107b;hb=3DHEAD >>>>>>>> >>>>>>>> >>>>>>> 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