From 204d39222a0ff9f91fdffc2809de0b7f5aaabbae Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 23 Jan 2023 19:07:35 +0000 Subject: [efi] Add efi_path_terminate() utility function Signed-off-by: Michael Brown --- src/image/efi_image.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'src/image') diff --git a/src/image/efi_image.c b/src/image/efi_image.c index 3c98decbf..66a19524b 100644 --- a/src/image/efi_image.c +++ b/src/image/efi_image.c @@ -96,9 +96,7 @@ efi_image_path ( struct image *image, EFI_DEVICE_PATH_PROTOCOL *parent ) { efi_snprintf ( filepath->PathName, ( name_len + 1 /* NUL */ ), "%s", image->name ); end = ( ( ( void * ) filepath ) + filepath_len ); - end->Type = END_DEVICE_PATH_TYPE; - end->SubType = END_ENTIRE_DEVICE_PATH_SUBTYPE; - end->Length[0] = sizeof ( *end ); + efi_path_terminate ( end ); return path; } -- cgit v1.2.3-55-g7522 From 9e1f7a3659071004f4b8c76f2593da6287f0d575 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 6 Mar 2023 16:28:48 +0000 Subject: [image] Always unregister currently executing image We unregister script images during their execution, to prevent a "boot" command from re-executing the containing script. This also has the side effect of preventing executing scripts from showing up within the Linux magic initrd image (or the Multiboot module list). Additional logic in bzimage.c and efi_file.c prevents a currently executing kernel from showing up within the magic initrd image. Similar logic in multiboot.c prevents the Multiboot kernel from showing up as a Multiboot module. This still leaves some corner cases that are not covered correctly. For example: when using a gzip-compressed kernel image, nothing will currently hide the original compressed image from the magic initrd. Fix by moving the logic that temporarily unregisters the current image from script_exec() to image_exec(), so that it applies to all image types, and simplify the magic initrd and Multiboot module list construction logic on the basis that no further filtering of the registered image list is necessary. This change has the side effect of hiding currently executing EFI images from the virtual filesystem exposed by iPXE. For example, when using iPXE to boot wimboot, the wimboot binary itself will no longer be visible within the virtual filesystem. Signed-off-by: Michael Brown --- src/arch/x86/image/bzimage.c | 8 -------- src/arch/x86/image/multiboot.c | 4 ---- src/core/image.c | 12 +++++++++--- src/image/script.c | 9 --------- src/interface/efi/efi_file.c | 6 +----- 5 files changed, 10 insertions(+), 29 deletions(-) (limited to 'src/image') diff --git a/src/arch/x86/image/bzimage.c b/src/arch/x86/image/bzimage.c index b40bb2d07..b15bd5563 100644 --- a/src/arch/x86/image/bzimage.c +++ b/src/arch/x86/image/bzimage.c @@ -355,10 +355,6 @@ static size_t bzimage_load_initrd ( struct image *image, size_t offset; size_t pad_len; - /* Do not include kernel image itself as an initrd */ - if ( initrd == image ) - return 0; - /* Create cpio header for non-prebuilt images */ offset = cpio_header ( initrd, &cpio ); @@ -406,10 +402,6 @@ static int bzimage_check_initrds ( struct image *image, /* Calculate total loaded length of initrds */ for_each_image ( initrd ) { - /* Skip kernel */ - if ( initrd == image ) - continue; - /* Calculate length */ len += bzimage_load_initrd ( image, initrd, UNULL ); len = bzimage_align ( len ); diff --git a/src/arch/x86/image/multiboot.c b/src/arch/x86/image/multiboot.c index 0c85df708..c1c63bc97 100644 --- a/src/arch/x86/image/multiboot.c +++ b/src/arch/x86/image/multiboot.c @@ -204,10 +204,6 @@ static int multiboot_add_modules ( struct image *image, physaddr_t start, break; } - /* Do not include kernel image itself as a module */ - if ( module_image == image ) - continue; - /* Page-align the module */ start = ( ( start + 0xfff ) & ~0xfff ); diff --git a/src/core/image.c b/src/core/image.c index 5a9aebc74..b280eb4d3 100644 --- a/src/core/image.c +++ b/src/core/image.c @@ -349,9 +349,8 @@ int image_exec ( struct image *image ) { /* Preserve record of any currently-running image */ saved_current_image = current_image; - /* Take out a temporary reference to the image. This allows - * the image to unregister itself if necessary, without - * automatically freeing itself. + /* Take out a temporary reference to the image, so that it + * does not get freed when temporarily unregistered. */ current_image = image_get ( image ); @@ -371,6 +370,9 @@ int image_exec ( struct image *image ) { /* Record boot attempt */ syslog ( LOG_NOTICE, "Executing \"%s\"\n", image->name ); + /* Temporarily unregister the image during its execution */ + unregister_image ( image ); + /* Try executing the image */ if ( ( rc = image->type->exec ( image ) ) != 0 ) { DBGC ( image, "IMAGE %s could not execute: %s\n", @@ -387,6 +389,10 @@ int image_exec ( struct image *image ) { image->name, strerror ( rc ) ); } + /* Re-register image (unless due to be replaced) */ + if ( ! image->replacement ) + register_image ( image ); + /* Pick up replacement image before we drop the original * image's temporary reference. The replacement image must * already be registered, so we don't need to hold a temporary diff --git a/src/image/script.c b/src/image/script.c index 28050868a..b34df1e21 100644 --- a/src/image/script.c +++ b/src/image/script.c @@ -197,11 +197,6 @@ static int script_exec ( struct image *image ) { size_t saved_offset; int rc; - /* Temporarily de-register image, so that a "boot" command - * doesn't throw us into an execution loop. - */ - unregister_image ( image ); - /* Preserve state of any currently-running script */ saved_offset = script_offset; @@ -212,10 +207,6 @@ static int script_exec ( struct image *image ) { /* Restore saved state */ script_offset = saved_offset; - /* Re-register image (unless we have been replaced) */ - if ( ! image->replacement ) - register_image ( image ); - return rc; } diff --git a/src/interface/efi/efi_file.c b/src/interface/efi/efi_file.c index f2b44fa73..a0bba1606 100644 --- a/src/interface/efi/efi_file.c +++ b/src/interface/efi/efi_file.c @@ -240,10 +240,6 @@ static size_t efi_file_read_initrd ( struct efi_file_reader *reader ) { len = 0; for_each_image ( image ) { - /* Ignore currently executing image */ - if ( image == current_image ) - continue; - /* Pad to alignment boundary */ pad_len = ( ( -reader->pos ) & ( INITRD_ALIGN - 1 ) ); if ( pad_len ) { @@ -1091,7 +1087,7 @@ int efi_file_install ( EFI_HANDLE handle ) { * instance only if the initrd is non-empty, since Linux does * not gracefully handle a zero-length initrd. */ - load = ( list_is_singular ( &images ) ? NULL : &efi_file_initrd.load ); + load = ( have_images() ? &efi_file_initrd.load : NULL ); if ( ( rc = efi_file_path_install ( &efi_file_initrd_path.vendor.Header, load ) ) != 0 ) { goto err_initrd; -- cgit v1.2.3-55-g7522 From 54fcb7c29ce2fabe109286e3c4dfeb297e0f3916 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Tue, 7 Mar 2023 14:18:00 +0000 Subject: [efi] Use image name instead of pointer value in debug messages Signed-off-by: Michael Brown --- src/image/efi_image.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) (limited to 'src/image') diff --git a/src/image/efi_image.c b/src/image/efi_image.c index 66a19524b..467fb05a4 100644 --- a/src/image/efi_image.c +++ b/src/image/efi_image.c @@ -147,38 +147,38 @@ static int efi_image_exec ( struct image *image ) { /* Find an appropriate device handle to use */ snpdev = last_opened_snpdev(); if ( ! snpdev ) { - DBGC ( image, "EFIIMAGE %p could not identify SNP device\n", - image ); + DBGC ( image, "EFIIMAGE %s could not identify SNP device\n", + image->name ); rc = -ENODEV; goto err_no_snpdev; } /* Install file I/O protocols */ if ( ( rc = efi_file_install ( snpdev->handle ) ) != 0 ) { - DBGC ( image, "EFIIMAGE %p could not install file protocol: " - "%s\n", image, strerror ( rc ) ); + DBGC ( image, "EFIIMAGE %s could not install file protocol: " + "%s\n", image->name, strerror ( rc ) ); goto err_file_install; } /* Install PXE base code protocol */ if ( ( rc = efi_pxe_install ( snpdev->handle, snpdev->netdev ) ) != 0 ){ - DBGC ( image, "EFIIMAGE %p could not install PXE protocol: " - "%s\n", image, strerror ( rc ) ); + DBGC ( image, "EFIIMAGE %s could not install PXE protocol: " + "%s\n", image->name, strerror ( rc ) ); goto err_pxe_install; } /* Install iPXE download protocol */ if ( ( rc = efi_download_install ( snpdev->handle ) ) != 0 ) { - DBGC ( image, "EFIIMAGE %p could not install iPXE download " - "protocol: %s\n", image, strerror ( rc ) ); + DBGC ( image, "EFIIMAGE %s could not install iPXE download " + "protocol: %s\n", image->name, strerror ( rc ) ); goto err_download_install; } /* Create device path for image */ path = efi_image_path ( image, snpdev->path ); if ( ! path ) { - DBGC ( image, "EFIIMAGE %p could not create device path\n", - image ); + DBGC ( image, "EFIIMAGE %s could not create device path\n", + image->name ); rc = -ENOMEM; goto err_image_path; } @@ -186,8 +186,8 @@ static int efi_image_exec ( struct image *image ) { /* Create command line for image */ cmdline = efi_image_cmdline ( image ); if ( ! cmdline ) { - DBGC ( image, "EFIIMAGE %p could not create command line\n", - image ); + DBGC ( image, "EFIIMAGE %s could not create command line\n", + image->name ); rc = -ENOMEM; goto err_cmdline; } @@ -199,8 +199,8 @@ static int efi_image_exec ( struct image *image ) { image->len, &handle ) ) != 0 ) { /* Not an EFI image */ rc = -EEFI_LOAD ( efirc ); - DBGC ( image, "EFIIMAGE %p could not load: %s\n", - image, strerror ( rc ) ); + DBGC ( image, "EFIIMAGE %s could not load: %s\n", + image->name, strerror ( rc ) ); if ( efirc == EFI_SECURITY_VIOLATION ) { goto err_load_image_security_violation; } else { @@ -220,8 +220,8 @@ static int efi_image_exec ( struct image *image ) { /* Some EFI 1.10 implementations seem not to fill in DeviceHandle */ if ( loaded.image->DeviceHandle == NULL ) { - DBGC ( image, "EFIIMAGE %p filling in missing DeviceHandle\n", - image ); + DBGC ( image, "EFIIMAGE %s filling in missing DeviceHandle\n", + image->name ); loaded.image->DeviceHandle = snpdev->handle; } @@ -251,14 +251,14 @@ static int efi_image_exec ( struct image *image ) { /* Start the image */ if ( ( efirc = bs->StartImage ( handle, NULL, NULL ) ) != 0 ) { rc = -EEFI_START ( efirc ); - DBGC ( image, "EFIIMAGE %p could not start (or returned with " - "error): %s\n", image, strerror ( rc ) ); + DBGC ( image, "EFIIMAGE %s could not start (or returned with " + "error): %s\n", image->name, strerror ( rc ) ); goto err_start_image; } /* If image was a driver, connect it up to anything available */ if ( type == EfiBootServicesCode ) { - DBGC ( image, "EFIIMAGE %p connecting drivers\n", image ); + DBGC ( image, "EFIIMAGE %s connecting drivers\n", image->name ); efi_driver_reconnect_all(); } @@ -324,8 +324,8 @@ static int efi_image_probe ( struct image *image ) { image->len, &handle ) ) != 0 ) { /* Not an EFI image */ rc = -EEFI_LOAD ( efirc ); - DBGC ( image, "EFIIMAGE %p could not load: %s\n", - image, strerror ( rc ) ); + DBGC ( image, "EFIIMAGE %s could not load: %s\n", + image->name, strerror ( rc ) ); if ( efirc == EFI_SECURITY_VIOLATION ) { goto err_load_image_security_violation; } else { -- cgit v1.2.3-55-g7522