[PATCH v2 3/3] discover/grub: Improve BLS grub environment variables expansion
Samuel Mendoza-Jonas
sam at mendozajonas.com
Tue Jun 12 14:32:06 AEST 2018
On Thu, 2018-06-07 at 19:35 +0200, Javier Martinez Canillas wrote:
> The fields from a BootLoaderSpec file can contain environment variables,
> in GRUB 2 these are show verbatim and are evaluated later when an entry
> is selected. But on Petitboot these have to be expanded before creating
> the GRUB 2 resources and show in the UI the values after the evaluation.
>
> The current blscfg handler had a very limited support for variables, it
> only had support for the options field and also didn't take into account
> that variables could be mixed with literal values.
>
> So for example the following fields were not expanded correctly:
>
> linux $bootprefix/vmlinuz
>
> options $kernelopts foo=bar
>
> options foo=bar $kernelopts
>
> options $kernelopts $debugopts
>
> Also change some of the tests to cover mixing variables and literals.
>
> Signed-off-by: Javier Martinez Canillas <javierm at redhat.com>
>
> ---
>
> discover/grub2/blscfg.c | 86 ++++++++++++++++----
> test/parser/test-grub2-blscfg-multiple-bls.c | 5 +-
> test/parser/test-grub2-blscfg-opts-grubenv.c | 4 +-
> 3 files changed, 75 insertions(+), 20 deletions(-)
>
> diff --git a/discover/grub2/blscfg.c b/discover/grub2/blscfg.c
> index a3813064a0a..fdbe2468952 100644
> --- a/discover/grub2/blscfg.c
> +++ b/discover/grub2/blscfg.c
> @@ -2,6 +2,7 @@
> #define _GNU_SOURCE
>
> #include <assert.h>
> +#include <ctype.h>
> #include <stdlib.h>
> #include <string.h>
> #include <dirent.h>
> @@ -34,54 +35,107 @@ struct bls_state {
> const char *dtb;
> };
>
> +static char *field_append(struct bls_state *state, int type, char *buffer,
> + char *start, char *end)
> +{
> + char *temp = talloc_strndup(state, start, end - start + 1);
> + temp[end - start + 1] = '\0';
This can go one byte past the end of temp depending on what start and end
are; _strndup() should terminate this buffer itself anyway.
> + const char *field = temp;
Also mostly for looks try not to mix declarations and code :)
Cheers,
Sam
> +
> + if (type == GRUB2_WORD_VAR) {
> + field = script_env_get(state->script, temp);
> + if (!field)
> + return buffer;
> + }
> +
> + if (!buffer)
> + buffer = talloc_strdup(state->opt, field);
> + else
> + buffer = talloc_asprintf_append(buffer, "%s", field);
> +
> + return buffer;
> +}
> +
> +static char *expand_field(struct bls_state *state, char *value)
> +{
> + char *buffer = NULL;
> + char *start = value;
> + char *end = value;
> + int type = GRUB2_WORD_TEXT;
> +
> + while (*value) {
> + if (*value == '$') {
> + if (start != end) {
> + buffer = field_append(state, type, buffer,
> + start, end);
> + if (!buffer)
> + return NULL;
> + }
> +
> + type = GRUB2_WORD_VAR;
> + start = value + 1;
> + } else if (type == GRUB2_WORD_VAR) {
> + if (!isalnum(*value) && *value != '_') {
> + buffer = field_append(state, type, buffer,
> + start, end);
> + type = GRUB2_WORD_TEXT;
> + start = value;
> + }
> + }
> +
> + end = value;
> + value++;
> + }
> +
> + if (start != end) {
> + buffer = field_append(state, type, buffer,
> + start, end);
> + if (!buffer)
> + return NULL;
> + }
> +
> + return buffer;
> +}
> +
> static void bls_process_pair(struct conf_context *conf, const char *name,
> char *value)
> {
> struct bls_state *state = conf->parser_info;
> struct discover_boot_option *opt = state->opt;
> struct boot_option *option = opt->option;
> - const char *boot_args;
>
> if (streq(name, "title")) {
> - state->title = talloc_strdup(state, value);
> + state->title = expand_field(state, value);
> return;
> }
>
> if (streq(name, "version")) {
> - state->version = talloc_strdup(state, value);
> + state->version = expand_field(state, value);
> return;
> }
>
> if (streq(name, "machine-id")) {
> - state->machine_id = talloc_strdup(state, value);
> + state->machine_id = expand_field(state, value);
> return;
> }
>
> if (streq(name, "linux")) {
> - state->image = talloc_strdup(state, value);
> + state->image = expand_field(state, value);
> return;
> }
>
> if (streq(name, "initrd")) {
> - state->initrd = talloc_strdup(state, value);
> + state->initrd = expand_field(state, value);
> return;
> }
>
> if (streq(name, "devicetree")) {
> - state->dtb = talloc_strdup(state, value);
> + state->dtb = expand_field(state, value);
> return;
> }
>
> if (streq(name, "options")) {
> - if (value[0] == '$') {
> - boot_args = script_env_get(state->script, value + 1);
> - if (!boot_args)
> - return;
> -
> - option->boot_args = talloc_strdup(opt, boot_args);
> - } else {
> - option->boot_args = talloc_strdup(opt, value);
> - }
> + option->boot_args = expand_field(state, value);
> return;
> }
> }
> diff --git a/test/parser/test-grub2-blscfg-multiple-bls.c b/test/parser/test-grub2-blscfg-multiple-bls.c
> index 94f40d191fa..d15fb24fd7e 100644
> --- a/test/parser/test-grub2-blscfg-multiple-bls.c
> +++ b/test/parser/test-grub2-blscfg-multiple-bls.c
> @@ -1,6 +1,7 @@
> #include "parser-test.h"
>
> #if 0 /* PARSER_EMBEDDED_CONFIG */
> +set os_name=Fedora
> blscfg
> #endif
>
> @@ -13,14 +14,14 @@ void run_test(struct parser_test *test)
>
> test_add_file_string(test, test->ctx->device,
> "/loader/entries/6c063c8e48904f2684abde8eea303f41-4.15.2-302.fc28.x86_64.conf",
> - "title Fedora (4.15.2-302.fc28.x86_64) 28 (Twenty Eight)\n"
> + "title $os_name (4.15.2-302.fc28.x86_64) 28 (Twenty Eight)\n"
> "linux /vmlinuz-4.15.2-302.fc28.x86_64\n"
> "initrd /initramfs-4.15.2-302.fc28.x86_64.img\n"
> "options root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root\n\n");
>
> test_add_file_string(test, test->ctx->device,
> "/loader/entries/6c063c8e48904f2684abde8eea303f41-4.14.18-300.fc28.x86_64.conf",
> - "title Fedora (4.14.18-300.fc28.x86_64) 28 (Twenty Eight)\n"
> + "title $os_name (4.14.18-300.fc28.x86_64) 28 (Twenty Eight)\n"
> "linux /vmlinuz-4.14.18-300.fc28.x86_64\n"
> "initrd /initramfs-4.14.18-300.fc28.x86_64.img\n"
> "options root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root\n");
> diff --git a/test/parser/test-grub2-blscfg-opts-grubenv.c b/test/parser/test-grub2-blscfg-opts-grubenv.c
> index 544a5de4d23..2dffd1b730c 100644
> --- a/test/parser/test-grub2-blscfg-opts-grubenv.c
> +++ b/test/parser/test-grub2-blscfg-opts-grubenv.c
> @@ -22,7 +22,7 @@ void run_test(struct parser_test *test)
> "title Fedora (4.15.2-302.fc28.x86_64) 28 (Twenty Eight)\n"
> "linux /vmlinuz-4.15.2-302.fc28.x86_64\n"
> "initrd /initramfs-4.15.2-302.fc28.x86_64.img\n"
> - "options $kernelopts\n");
> + "options $kernelopts debug\n");
>
> test_read_conf_embedded(test, "/boot/grub2/grub.cfg");
>
> @@ -32,5 +32,5 @@ void run_test(struct parser_test *test)
>
> opt = get_boot_option(ctx, 0);
>
> - check_args(opt, "root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root");
> + check_args(opt, "root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root debug");
> }
More information about the Petitboot
mailing list