diff mbox series

[2/2] discover/boot: unify verification failure messages

Message ID 20191029091540.6767-2-jk@ozlabs.org
State Under Review
Headers show
Series [1/2] discover/boot: add support for `kexec -s` for kexec_file_load | expand

Commit Message

Jeremy Kerr Oct. 29, 2019, 9:15 a.m. UTC
Currently, we have two sites where the result of validate_boot_files is
interpreted: in kexec_load, and boot_process. In the former, we generate
the pb_log message, and in the latter we generate the status message.

This means we have separate places to maintain similar error messages,
which is prone to future errors. This change does all of the
interpretation directly after calling validate_boot_files().

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 discover/boot.c | 45 ++++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

Comments

Joel Stanley Jan. 23, 2020, 10:38 a.m. UTC | #1
On Tue, 29 Oct 2019 at 09:26, Jeremy Kerr <jk@ozlabs.org> wrote:
>
> Currently, we have two sites where the result of validate_boot_files is
> interpreted: in kexec_load, and boot_process. In the former, we generate
> the pb_log message, and in the latter we generate the status message.
>
> This means we have separate places to maintain similar error messages,
> which is prone to future errors. This change does all of the
> interpretation directly after calling validate_boot_files().
>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

Acked-by: Joel Stanley <joel@jms.id.au>

> ---
>  discover/boot.c | 45 ++++++++++++++++++++++-----------------------
>  1 file changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/discover/boot.c b/discover/boot.c
> index a6b88f0..9e7054b 100644
> --- a/discover/boot.c
> +++ b/discover/boot.c
> @@ -75,16 +75,30 @@ static int kexec_load(struct boot_task *boot_task)
>         boot_task->local_dtb_override = NULL;
>         boot_task->local_image_override = NULL;
>
> -       if ((result = validate_boot_files(boot_task))) {
> -               if (result == KEXEC_LOAD_DECRYPTION_FALURE) {
> -                       pb_log("%s: Aborting kexec due to"
> -                               " decryption failure\n", __func__);
> -               }
> -               if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
> -                       pb_log("%s: Aborting kexec due to signature"
> -                               " verification failure\n", __func__);
> +       result = validate_boot_files(boot_task);
> +       if (result) {
> +               const char *msg;
> +
> +               switch (result) {
> +               case KEXEC_LOAD_DECRYPTION_FALURE:
> +                       msg = _("decryption failed");
> +                       break;
> +               case KEXEC_LOAD_SIGNATURE_FAILURE:
> +                       msg = _("signature verification failed");
> +                       break;
> +               case KEXEC_LOAD_SIG_SETUP_INVALID:
> +                       msg = _("invalid signature configuration");
> +                       break;
> +               default:
> +                       msg = _("unknown verification failure");
>                 }
>
> +               update_status(boot_task->status_fn, boot_task->status_arg,
> +                               STATUS_ERROR,
> +                               _("Boot verification failure: %s"), msg);
> +               pb_log_fn("Aborting kexec due to verification failure: %s",
> +                               msg);
> +
>                 validate_boot_files_cleanup(boot_task);
>                 return result;
>         }
> @@ -451,21 +465,6 @@ static void boot_process(struct load_url_result *result, void *data)
>                         _("Performing kexec load"));
>
>         rc = kexec_load(task);
> -       pb_log_fn("kexec_load returned %d\n", rc);
> -       if (rc == KEXEC_LOAD_DECRYPTION_FALURE) {
> -               update_status(task->status_fn, task->status_arg,
> -                               STATUS_ERROR, _("Decryption failed"));
> -       }
> -       else if (rc == KEXEC_LOAD_SIGNATURE_FAILURE) {
> -               update_status(task->status_fn, task->status_arg,
> -                               STATUS_ERROR,
> -                               _("Signature verification failed"));
> -       }
> -       else if (rc == KEXEC_LOAD_SIG_SETUP_INVALID) {
> -               update_status(task->status_fn, task->status_arg,
> -                               STATUS_ERROR,
> -                               _("Invalid signature configuration"));
> -       }
>
>  no_load:
>         list_for_each_entry(&task->resources, resource, list)
> --
> 2.20.1
>
> _______________________________________________
> Petitboot mailing list
> Petitboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/petitboot
Joel Stanley Jan. 23, 2020, 12:03 p.m. UTC | #2
On Thu, 23 Jan 2020 at 10:38, Joel Stanley <joel@jms.id.au> wrote:
>
> On Tue, 29 Oct 2019 at 09:26, Jeremy Kerr <jk@ozlabs.org> wrote:
> >
> > Currently, we have two sites where the result of validate_boot_files is
> > interpreted: in kexec_load, and boot_process. In the former, we generate
> > the pb_log message, and in the latter we generate the status message.
> >
> > This means we have separate places to maintain similar error messages,
> > which is prone to future errors. This change does all of the
> > interpretation directly after calling validate_boot_files().
> >
> > Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
>
> Acked-by: Joel Stanley <joel@jms.id.au>

This is what we see on a system that cannot kexec, with your patches
applied. It cannot kexec as it has
CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY=y (blocks the normal kexec
call) and CONFIG_KEXEC_FILE=n.

---
[11:48:19] boot status: [0] Performing kexec load
[11:48:19] device_handler_status: Performing kexec load
[11:48:19] Running command:
 exe:  /usr/sbin/kexec
 argv: '/usr/sbin/kexec' '-l' '--debug'
'--initrd=/var/petitboot/mnt/dev/sda2/boot/initrd.img-4.15.0-52-generic'
'--dtb=/tmp/tmp.to4ypW'
'--append=root=UUID=c5a560f9-8c01-4209-86ce-3abe4298b7cc ro'
'/var/petitboot/mnt/dev/sda2/boot/vmlinux-4.15.0-52-generic'
[11:48:19] kexec_load: kexec load (-l) failed (rc 1): free memory map:
0x01dd0000-0x30000000
0x3bd04000-0x80000000
kernel offset 0x10000 paddr 0x0 filesz 24460736 memsz 27622200
add_kexec_segment kernel      buf 0x7fff9a570010 bufsize 0x01753dc0,
dest 0x1dd0000, memsize 0x01a60000
add_kexec_segment initrd      buf 0x7fff97df0010 bufsize 0x0277f14d,
dest 0x3830000, memsize 0x02780000
add_kexec_segment device tree buf 0x7fff9bcd0010 bufsize 0x0004e71b,
dest 0x7ffb0000, memsize 0x00050000
add_kexec_segment trampoline  buf 0x31420680 bufsize 0x00000210, dest
0x7ffa0000, memsize 0x00010000
kexec syscall failed: Operation not permitted

[11:48:19] Running command:
 exe:  /usr/sbin/kexec
 argv: '/usr/sbin/kexec' '-s' '--debug'
'--initrd=/var/petitboot/mnt/dev/sda2/boot/initrd.img-4.15.0-52-generic'
'--dtb=/tmp/tmp.to4ypW'
'--append=root=UUID=c5a560f9-8c01-4209-86ce-3abe4298b7cc ro'
'/var/petitboot/mnt/dev/sda2/boot/vmlinux-4.15.0-52-generic'
[11:48:19] kexec_load: kexec load (-s) failed (rc 1): kernel_fd=7
initrd_fd=11 cmdline_len=50 flags=0
cmdline="root=UUID=c5a560f9-8c01-4209-86ce-3abe4298b7cc ro"
do_file_load: (-1) Function not implemented

[11:48:19] boot status: [1] kexec load failed: kernel_fd=7
initrd_fd=11 cmdline_len=50 flags=0
cmdline="root=UUID=c5a560f9-8c01-4209-86ce-3abe4298b7cc ro"
do_file_load: (-1) Function not implemented

[11:48:19] device_handler_status: kexec load failed: kernel_fd=7
initrd_fd=11 cmdline_len=50 flags=0
cmdline="root=UUID=c5a560f9-8c01-4209-86ce-3abe4298b7cc ro"
do_file_load: (-1) Function not implemented

[11:48:19] Failed to load all boot resources
---

The user sees "kexec load failed: kernel_fd=7 initrd_fd=11
cmdline_len=50 flags=0" in the status. Could/should/can we instead
have it display the "Function not implemented" line?

Cheers,

Joel


>
> > ---
> >  discover/boot.c | 45 ++++++++++++++++++++++-----------------------
> >  1 file changed, 22 insertions(+), 23 deletions(-)
> >
> > diff --git a/discover/boot.c b/discover/boot.c
> > index a6b88f0..9e7054b 100644
> > --- a/discover/boot.c
> > +++ b/discover/boot.c
> > @@ -75,16 +75,30 @@ static int kexec_load(struct boot_task *boot_task)
> >         boot_task->local_dtb_override = NULL;
> >         boot_task->local_image_override = NULL;
> >
> > -       if ((result = validate_boot_files(boot_task))) {
> > -               if (result == KEXEC_LOAD_DECRYPTION_FALURE) {
> > -                       pb_log("%s: Aborting kexec due to"
> > -                               " decryption failure\n", __func__);
> > -               }
> > -               if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
> > -                       pb_log("%s: Aborting kexec due to signature"
> > -                               " verification failure\n", __func__);
> > +       result = validate_boot_files(boot_task);
> > +       if (result) {
> > +               const char *msg;
> > +
> > +               switch (result) {
> > +               case KEXEC_LOAD_DECRYPTION_FALURE:
> > +                       msg = _("decryption failed");
> > +                       break;
> > +               case KEXEC_LOAD_SIGNATURE_FAILURE:
> > +                       msg = _("signature verification failed");
> > +                       break;
> > +               case KEXEC_LOAD_SIG_SETUP_INVALID:
> > +                       msg = _("invalid signature configuration");
> > +                       break;
> > +               default:
> > +                       msg = _("unknown verification failure");
> >                 }
> >
> > +               update_status(boot_task->status_fn, boot_task->status_arg,
> > +                               STATUS_ERROR,
> > +                               _("Boot verification failure: %s"), msg);
> > +               pb_log_fn("Aborting kexec due to verification failure: %s",
> > +                               msg);
> > +
> >                 validate_boot_files_cleanup(boot_task);
> >                 return result;
> >         }
> > @@ -451,21 +465,6 @@ static void boot_process(struct load_url_result *result, void *data)
> >                         _("Performing kexec load"));
> >
> >         rc = kexec_load(task);
> > -       pb_log_fn("kexec_load returned %d\n", rc);
> > -       if (rc == KEXEC_LOAD_DECRYPTION_FALURE) {
> > -               update_status(task->status_fn, task->status_arg,
> > -                               STATUS_ERROR, _("Decryption failed"));
> > -       }
> > -       else if (rc == KEXEC_LOAD_SIGNATURE_FAILURE) {
> > -               update_status(task->status_fn, task->status_arg,
> > -                               STATUS_ERROR,
> > -                               _("Signature verification failed"));
> > -       }
> > -       else if (rc == KEXEC_LOAD_SIG_SETUP_INVALID) {
> > -               update_status(task->status_fn, task->status_arg,
> > -                               STATUS_ERROR,
> > -                               _("Invalid signature configuration"));
> > -       }
> >
> >  no_load:
> >         list_for_each_entry(&task->resources, resource, list)
> > --
> > 2.20.1
> >
> > _______________________________________________
> > Petitboot mailing list
> > Petitboot@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/petitboot
diff mbox series

Patch

diff --git a/discover/boot.c b/discover/boot.c
index a6b88f0..9e7054b 100644
--- a/discover/boot.c
+++ b/discover/boot.c
@@ -75,16 +75,30 @@  static int kexec_load(struct boot_task *boot_task)
 	boot_task->local_dtb_override = NULL;
 	boot_task->local_image_override = NULL;
 
-	if ((result = validate_boot_files(boot_task))) {
-		if (result == KEXEC_LOAD_DECRYPTION_FALURE) {
-			pb_log("%s: Aborting kexec due to"
-				" decryption failure\n", __func__);
-		}
-		if (result == KEXEC_LOAD_SIGNATURE_FAILURE) {
-			pb_log("%s: Aborting kexec due to signature"
-				" verification failure\n", __func__);
+	result = validate_boot_files(boot_task);
+	if (result) {
+		const char *msg;
+
+		switch (result) {
+		case KEXEC_LOAD_DECRYPTION_FALURE:
+			msg = _("decryption failed");
+			break;
+		case KEXEC_LOAD_SIGNATURE_FAILURE:
+			msg = _("signature verification failed");
+			break;
+		case KEXEC_LOAD_SIG_SETUP_INVALID:
+			msg = _("invalid signature configuration");
+			break;
+		default:
+			msg = _("unknown verification failure");
 		}
 
+		update_status(boot_task->status_fn, boot_task->status_arg,
+				STATUS_ERROR,
+				_("Boot verification failure: %s"), msg);
+		pb_log_fn("Aborting kexec due to verification failure: %s",
+				msg);
+
 		validate_boot_files_cleanup(boot_task);
 		return result;
 	}
@@ -451,21 +465,6 @@  static void boot_process(struct load_url_result *result, void *data)
 			_("Performing kexec load"));
 
 	rc = kexec_load(task);
-	pb_log_fn("kexec_load returned %d\n", rc);
-	if (rc == KEXEC_LOAD_DECRYPTION_FALURE) {
-		update_status(task->status_fn, task->status_arg,
-				STATUS_ERROR, _("Decryption failed"));
-	}
-	else if (rc == KEXEC_LOAD_SIGNATURE_FAILURE) {
-		update_status(task->status_fn, task->status_arg,
-				STATUS_ERROR,
-				_("Signature verification failed"));
-	}
-	else if (rc == KEXEC_LOAD_SIG_SETUP_INVALID) {
-		update_status(task->status_fn, task->status_arg,
-				STATUS_ERROR,
-				_("Invalid signature configuration"));
-	}
 
 no_load:
 	list_for_each_entry(&task->resources, resource, list)