[efi] Allow for fact that SNP device may be removed by executed image

The executed image may call DisconnectController() to remove our
network device.  This will leave the net device unregistered but not
yet freed (since our installed PXE base code protocol retains a
reference to the net device).

Unregistration will cause the network upper-layer driver removal
functions to be called, which will free the SNP device structure.
When the image returns from StartImage(), the snpdev pointer may
therefore no longer be valid.

The SNP device structure is not reference counted, and so we cannot
simply take out a reference to ensure that it remains valid across the
call to StartImage().  However, the code path following the call to
StartImage() doesn't actually require the SNP device pointer, only the
EFI device handle.

Store the device handle in a local variable and ensure that snpdev is
invalidated before the call to StartImage() so that future code cannot
accidentally reintroduce this issue.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/image/efi_image.c b/src/image/efi_image.c
index 9261036dac7e8678a375494f9f43606f4d62e3e5..273f8e8fde93cc7d487d9eecda3b0b1a08367b0d 100644 (file)
--- a/src/image/efi_image.c
+++ b/src/image/efi_image.c
@@ -154,6 +154,7 @@ static int efi_image_exec ( struct image *image ) {
        EFI_LOADED_IMAGE_PROTOCOL *loaded;
        struct image *shim;
        struct image *exec;
+       EFI_HANDLE device;
        EFI_HANDLE handle;
        EFI_MEMORY_TYPE type;
        wchar_t *cmdline;
@@ -169,6 +170,7 @@ static int efi_image_exec ( struct image *image ) {
                rc = -ENODEV;
                goto err_no_snpdev;
        }
+       device = snpdev->handle;
 
        /* Use shim instead of directly executing image if applicable */
        shim = ( efi_can_load ( image ) ?
@@ -186,21 +188,21 @@ static int efi_image_exec ( struct image *image ) {
                goto err_register_image;
 
        /* Install file I/O protocols */
-       if ( ( rc = efi_file_install ( snpdev->handle ) ) != 0 ) {
+       if ( ( rc = efi_file_install ( device ) ) != 0 ) {
                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 ){
+       if ( ( rc = efi_pxe_install ( device, snpdev->netdev ) ) != 0 ){
                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 ) {
+       if ( ( rc = efi_download_install ( device ) ) != 0 ) {
                DBGC ( image, "EFIIMAGE %s could not install iPXE download "
                       "protocol: %s\n", image->name, strerror ( rc ) );
                goto err_download_install;
@@ -233,8 +235,7 @@ static int efi_image_exec ( struct image *image ) {
 
        /* Install shim special handling if applicable */
        if ( shim &&
-            ( ( rc = efi_shim_install ( shim, snpdev->handle,
-                                        &cmdline ) ) != 0 ) ){
+            ( ( rc = efi_shim_install ( shim, device, &cmdline ) ) != 0 ) ) {
                DBGC ( image, "EFIIMAGE %s could not install shim handling: "
                       "%s\n", image->name, strerror ( rc ) );
                goto err_shim_install;
@@ -267,12 +268,12 @@ static int efi_image_exec ( struct image *image ) {
        if ( loaded->DeviceHandle == NULL ) {
                DBGC ( image, "EFIIMAGE %s filling in missing DeviceHandle\n",
                       image->name );
-               loaded->DeviceHandle = snpdev->handle;
+               loaded->DeviceHandle = device;
        }
 
        /* Sanity checks */
        assert ( loaded->ParentHandle == efi_image_handle );
-       assert ( loaded->DeviceHandle == snpdev->handle );
+       assert ( loaded->DeviceHandle == device );
        assert ( loaded->LoadOptionsSize == 0 );
        assert ( loaded->LoadOptions == NULL );
 
@@ -293,6 +294,9 @@ static int efi_image_exec ( struct image *image ) {
        /* Reset console since image will probably use it */
        console_reset();
 
+       /* Assume that image may cause SNP device to be removed */
+       snpdev = NULL;
+
        /* Start the image */
        if ( ( efirc = bs->StartImage ( handle, NULL, NULL ) ) != 0 ) {
                rc = -EEFI_START ( efirc );
@@ -341,11 +345,11 @@ static int efi_image_exec ( struct image *image ) {
  err_image_path:
        efi_fdt_uninstall();
  err_fdt_install:
-       efi_download_uninstall ( snpdev->handle );
+       efi_download_uninstall ( device );
  err_download_install:
-       efi_pxe_uninstall ( snpdev->handle );
+       efi_pxe_uninstall ( device );
  err_pxe_install:
-       efi_file_uninstall ( snpdev->handle );
+       efi_file_uninstall ( device );
  err_file_install:
        unregister_image ( image );
  err_register_image: