diff mbox series

discover/syslinux-parser: consistent cmdline arg processing

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

Commit Message

Grandbois, Brett June 10, 2018, 9:54 p.m. UTC
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(-)

Comments

samjonas June 12, 2018, 5:19 a.m. UTC | #1
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 mbox series

Patch

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");
 }