[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 <mcb30@ipxe.org>

diff --git a/src/arch/x86/image/bzimage.c b/src/arch/x86/image/bzimage.c
index b40bb2d070ebb9a3dc3318aa2cdef8e2a4d30af5..b15bd55637cceac7a7459779969b9edfe09cb15b 100644 (file)
--- 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 0c85df70864e995edbbf7defb8c8a6b2a08e477a..c1c63bc975ee9ee151b29ad37df0441c594784e4 100644 (file)
--- 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 5a9aebc7472442e56a62c4aa24631fb353f748f8..b280eb4d3bebc730cee2219e763050b3e2edb4c5 100644 (file)
--- 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 28050868af2387bac80e480d8d6c9d2867d7e4f4..b34df1e21a52ee78525d12dfd2e9d824e59dd75b 100644 (file)
--- 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 f2b44fa7379b40d6acddea6d417d63727a8c0dcb..a0bba1606277dcf80368a9edb062b66bef09f0ea 100644 (file)
--- 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;