| Message ID | 1528667648-12991-1-git-send-email-brett.grandbois@opengear.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | discover/syslinux-parser: consistent cmdline arg processing | expand |
On Mon, 2018-06-11 at 07:54 +1000, Brett Grandbois wrote: > In signed-boot environments consistent handling of kernel commandline > options is essential as they must be pre-signed. In the syslinux parser > ensure that in the absence of a global APPEND they are processed > exactly as found and not with the leading space that the current APPEND > processing has as a shortcut. > > Signed-off-by: Brett Grandbois <brett.grandbois@opengear.com> Merged as 0e9f4d38 > --- > discover/syslinux-parser.c | 11 +++++------ > test/parser/test-syslinux-explicit.c | 2 +- > test/parser/test-syslinux-global-append.c | 19 +++++++++++++++---- > test/parser/test-syslinux-single-yocto.c | 2 +- > 4 files changed, 22 insertions(+), 12 deletions(-) > > diff --git a/discover/syslinux-parser.c b/discover/syslinux-parser.c > index be7b94a..4b82433 100644 > --- a/discover/syslinux-parser.c > +++ b/discover/syslinux-parser.c > @@ -338,11 +338,13 @@ static void syslinux_finalize(struct conf_context *conf) > /* '-' can signal do not use global APPEND */ > if (!strcmp(syslinux_opt->append, "-")) > opt->boot_args = talloc_strdup(opt, ""); > - else > + else if (global_append) > opt->boot_args = talloc_asprintf(opt, "%s %s", > global_append, > syslinux_opt->append); > - } else > + else > + opt->boot_args = talloc_strdup(opt, syslinux_opt->append); > + } else if (global_append) > opt->boot_args = talloc_strdup(opt, global_append); > > if (!opt->boot_args) > @@ -454,13 +456,10 @@ static int syslinux_parse(struct discover_context *dc) > /* > * set the global defaults > * by spec 'default' defaults to 'linux' and > - * 'implicit' defaults to '1', we also just set > - * and empty string in 'append' to make it easier > - * in syslinux_finish > + * 'implicit' defaults to '1' > */ > conf_set_global_option(conf, "default", "linux"); > conf_set_global_option(conf, "implicit", "1"); > - conf_set_global_option(conf, "append", ""); > > for (filename = syslinux_conf_files; *filename; filename++) { > /* > diff --git a/test/parser/test-syslinux-explicit.c b/test/parser/test-syslinux-explicit.c > index 5d23f50..82030bf 100644 > --- a/test/parser/test-syslinux-explicit.c > +++ b/test/parser/test-syslinux-explicit.c > @@ -36,6 +36,6 @@ void run_test(struct parser_test *test) > > check_name(opt, "backup"); > check_resolved_local_resource(opt->boot_image, ctx->device, "/backup/vmlinuz"); > - check_args(opt, " root=/dev/sdb"); > + check_args(opt, "root=/dev/sdb"); > check_resolved_local_resource(opt->initrd, ctx->device, "/boot/initrd"); > } > diff --git a/test/parser/test-syslinux-global-append.c b/test/parser/test-syslinux-global-append.c > index 18af99a..af2e9af 100644 > --- a/test/parser/test-syslinux-global-append.c > +++ b/test/parser/test-syslinux-global-append.c > @@ -18,6 +18,9 @@ LABEL hyphen > KERNEL /test/vmlinuz > APPEND - > > +LABEL onlyglobal > +KERNEL /only/vmlinuz > + > #endif > > void run_test(struct parser_test *test) > @@ -31,8 +34,9 @@ void run_test(struct parser_test *test) > > ctx = test->ctx; > > - check_boot_option_count(ctx, 3); > - opt = get_boot_option(ctx, 2); > + check_boot_option_count(ctx, 4); > + > + opt = get_boot_option(ctx, 3); > > check_name(opt, "linux"); > check_resolved_local_resource(opt->boot_image, ctx->device, "/vmlinuz"); > @@ -40,17 +44,24 @@ void run_test(struct parser_test *test) > check_args(opt, "console=ttyS0 console=tty0"); > check_not_present_resource(opt->initrd); > > - opt = get_boot_option(ctx, 1); > + opt = get_boot_option(ctx, 2); > > check_name(opt, "backup"); > check_resolved_local_resource(opt->boot_image, ctx->device, "/backup/vmlinuz"); > check_args(opt, "console=ttyS0 root=/dev/sdb"); > check_resolved_local_resource(opt->initrd, ctx->device, "/boot/initrd"); > > - opt = get_boot_option(ctx, 0); > + opt = get_boot_option(ctx, 1); > > check_name(opt, "hyphen"); > check_resolved_local_resource(opt->boot_image, ctx->device, "/test/vmlinuz"); > check_args(opt, ""); > check_not_present_resource(opt->initrd); > + > + opt = get_boot_option(ctx, 0); > + > + check_name(opt, "onlyglobal"); > + check_resolved_local_resource(opt->boot_image, ctx->device, "/only/vmlinuz"); > + check_args(opt, "console=ttyS0"); > + check_not_present_resource(opt->initrd); > } > diff --git a/test/parser/test-syslinux-single-yocto.c b/test/parser/test-syslinux-single-yocto.c > index e5e084d..dd26577 100644 > --- a/test/parser/test-syslinux-single-yocto.c > +++ b/test/parser/test-syslinux-single-yocto.c > @@ -32,5 +32,5 @@ void run_test(struct parser_test *test) > check_name(opt, "boot"); > check_resolved_local_resource(opt->boot_image, ctx->device, "/vmlinuz"); > check_is_default(opt); > - check_args(opt, " console=ttyS0,115200n8 console=tty0"); > + check_args(opt, "console=ttyS0,115200n8 console=tty0"); > }
diff --git a/discover/syslinux-parser.c b/discover/syslinux-parser.c index be7b94a..4b82433 100644 --- a/discover/syslinux-parser.c +++ b/discover/syslinux-parser.c @@ -338,11 +338,13 @@ static void syslinux_finalize(struct conf_context *conf) /* '-' can signal do not use global APPEND */ if (!strcmp(syslinux_opt->append, "-")) opt->boot_args = talloc_strdup(opt, ""); - else + else if (global_append) opt->boot_args = talloc_asprintf(opt, "%s %s", global_append, syslinux_opt->append); - } else + else + opt->boot_args = talloc_strdup(opt, syslinux_opt->append); + } else if (global_append) opt->boot_args = talloc_strdup(opt, global_append); if (!opt->boot_args) @@ -454,13 +456,10 @@ static int syslinux_parse(struct discover_context *dc) /* * set the global defaults * by spec 'default' defaults to 'linux' and - * 'implicit' defaults to '1', we also just set - * and empty string in 'append' to make it easier - * in syslinux_finish + * 'implicit' defaults to '1' */ conf_set_global_option(conf, "default", "linux"); conf_set_global_option(conf, "implicit", "1"); - conf_set_global_option(conf, "append", ""); for (filename = syslinux_conf_files; *filename; filename++) { /* diff --git a/test/parser/test-syslinux-explicit.c b/test/parser/test-syslinux-explicit.c index 5d23f50..82030bf 100644 --- a/test/parser/test-syslinux-explicit.c +++ b/test/parser/test-syslinux-explicit.c @@ -36,6 +36,6 @@ void run_test(struct parser_test *test) check_name(opt, "backup"); check_resolved_local_resource(opt->boot_image, ctx->device, "/backup/vmlinuz"); - check_args(opt, " root=/dev/sdb"); + check_args(opt, "root=/dev/sdb"); check_resolved_local_resource(opt->initrd, ctx->device, "/boot/initrd"); } diff --git a/test/parser/test-syslinux-global-append.c b/test/parser/test-syslinux-global-append.c index 18af99a..af2e9af 100644 --- a/test/parser/test-syslinux-global-append.c +++ b/test/parser/test-syslinux-global-append.c @@ -18,6 +18,9 @@ LABEL hyphen KERNEL /test/vmlinuz APPEND - +LABEL onlyglobal +KERNEL /only/vmlinuz + #endif void run_test(struct parser_test *test) @@ -31,8 +34,9 @@ void run_test(struct parser_test *test) ctx = test->ctx; - check_boot_option_count(ctx, 3); - opt = get_boot_option(ctx, 2); + check_boot_option_count(ctx, 4); + + opt = get_boot_option(ctx, 3); check_name(opt, "linux"); check_resolved_local_resource(opt->boot_image, ctx->device, "/vmlinuz"); @@ -40,17 +44,24 @@ void run_test(struct parser_test *test) check_args(opt, "console=ttyS0 console=tty0"); check_not_present_resource(opt->initrd); - opt = get_boot_option(ctx, 1); + opt = get_boot_option(ctx, 2); check_name(opt, "backup"); check_resolved_local_resource(opt->boot_image, ctx->device, "/backup/vmlinuz"); check_args(opt, "console=ttyS0 root=/dev/sdb"); check_resolved_local_resource(opt->initrd, ctx->device, "/boot/initrd"); - opt = get_boot_option(ctx, 0); + opt = get_boot_option(ctx, 1); check_name(opt, "hyphen"); check_resolved_local_resource(opt->boot_image, ctx->device, "/test/vmlinuz"); check_args(opt, ""); check_not_present_resource(opt->initrd); + + opt = get_boot_option(ctx, 0); + + check_name(opt, "onlyglobal"); + check_resolved_local_resource(opt->boot_image, ctx->device, "/only/vmlinuz"); + check_args(opt, "console=ttyS0"); + check_not_present_resource(opt->initrd); } diff --git a/test/parser/test-syslinux-single-yocto.c b/test/parser/test-syslinux-single-yocto.c index e5e084d..dd26577 100644 --- a/test/parser/test-syslinux-single-yocto.c +++ b/test/parser/test-syslinux-single-yocto.c @@ -32,5 +32,5 @@ void run_test(struct parser_test *test) check_name(opt, "boot"); check_resolved_local_resource(opt->boot_image, ctx->device, "/vmlinuz"); check_is_default(opt); - check_args(opt, " console=ttyS0,115200n8 console=tty0"); + check_args(opt, "console=ttyS0,115200n8 console=tty0"); }
In signed-boot environments consistent handling of kernel commandline options is essential as they must be pre-signed. In the syslinux parser ensure that in the absence of a global APPEND they are processed exactly as found and not with the leading space that the current APPEND processing has as a shortcut. Signed-off-by: Brett Grandbois <brett.grandbois@opengear.com> --- discover/syslinux-parser.c | 11 +++++------ test/parser/test-syslinux-explicit.c | 2 +- test/parser/test-syslinux-global-append.c | 19 +++++++++++++++---- test/parser/test-syslinux-single-yocto.c | 2 +- 4 files changed, 22 insertions(+), 12 deletions(-)