Restore loaded image of shim at Exit()

When grub2 invoked Exit() in AArch64 AAVMF, the VM crashed with the following messsages: Unloading driver at 0x000B7D7B000 Synchronous Exception at 0x00000000BF5D5E68 AllocatePool: failed to allocate 800 bytes Synchronous Exception at 0x00000000BF5D5E68 The similar error also showed when I modified MokManager to call gBS->Exit() at the end of efi_main(). However, if MokManager just returned, the error never showed. One significant difference is whether the loaded image was restored or not, and the firmware seems to need the original ImageBase pointer to do clean-up. To avoid the potential crash, this commit adds restore_loaded_image() so that we can restore the loaded image both in start_image() and do_exit(). Signed-off-by: Gary Lin <glin@suse.com>
rhboot · Mar 7, 2021 · 74d2665 · 74d2665
1 parent 6b742f2
commit 74d2665
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 17 deletions.
diff --git a/replacements.c b/replacements.c
@@ -133,6 +133,8 @@ do_exit(EFI_HANDLE ImageHandle, EFI_STATUS ExitStatus,
 
 	shim_fini();
 
+	restore_loaded_image();
+
 	efi_status = gBS->Exit(ImageHandle, ExitStatus,
 			       ExitDataSize, ExitData);
 	if (EFI_ERROR(efi_status)) {

diff --git a/shim.c b/shim.c
@@ -38,6 +38,8 @@
 
 static EFI_SYSTEM_TABLE *systab;
 static EFI_HANDLE global_image_handle;
+static EFI_LOADED_IMAGE *shim_li;
+static EFI_LOADED_IMAGE shim_li_bak;
 
 static CHAR16 *second_stage;
 void *load_options;
@@ -1069,13 +1071,24 @@ static EFI_STATUS shim_read_header(void *data, unsigned int datasize,
 	return efi_status;
 }
 
+VOID
+restore_loaded_image(VOID)
+{
+	if (shim_li->FilePath)
+		FreePool(shim_li->FilePath);
+
+	/*
+	 * Restore our original loaded image values
+	 */
+	CopyMem(shim_li, &shim_li_bak, sizeof(shim_li_bak));
+}
+
 /*
  * Load and run an EFI executable
  */
 EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 {
 	EFI_STATUS efi_status;
-	EFI_LOADED_IMAGE *li, li_bak;
 	EFI_IMAGE_ENTRY_POINT entry_point;
 	EFI_PHYSICAL_ADDRESS alloc_address;
 	UINTN alloc_pages;
@@ -1090,7 +1103,7 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 	 * binary in order to find our path
 	 */
 	efi_status = gBS->HandleProtocol(image_handle, &EFI_LOADED_IMAGE_GUID,
-					 (void **)&li);
+					 (void **)&shim_li);
 	if (EFI_ERROR(efi_status)) {
 		perror(L"Unable to init protocol\n");
 		return efi_status;
@@ -1099,14 +1112,14 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 	/*
 	 * Build a new path from the existing one plus the executable name
 	 */
-	efi_status = generate_path_from_image_path(li, ImagePath, &PathName);
+	efi_status = generate_path_from_image_path(shim_li, ImagePath, &PathName);
 	if (EFI_ERROR(efi_status)) {
 		perror(L"Unable to generate path %s: %r\n", ImagePath,
 		       efi_status);
 		goto done;
 	}
 
-	if (findNetboot(li->DeviceHandle)) {
+	if (findNetboot(shim_li->DeviceHandle)) {
 		efi_status = parseNetbootinfo(image_handle);
 		if (EFI_ERROR(efi_status)) {
 			perror(L"Netboot parsing failed: %r\n", efi_status);
@@ -1121,7 +1134,7 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 		}
 		data = sourcebuffer;
 		datasize = sourcesize;
-	} else if (find_httpboot(li->DeviceHandle)) {
+	} else if (find_httpboot(shim_li->DeviceHandle)) {
 		efi_status = httpboot_fetch_buffer (image_handle,
 						    &sourcebuffer,
 						    &sourcesize);
@@ -1136,7 +1149,7 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 		/*
 		 * Read the new executable off disk
 		 */
-		efi_status = load_image(li, &data, &datasize, PathName);
+		efi_status = load_image(shim_li, &data, &datasize, PathName);
 		if (EFI_ERROR(efi_status)) {
 			perror(L"Failed to load image %s: %r\n",
 			       PathName, efi_status);
@@ -1155,13 +1168,13 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 	 * We need to modify the loaded image protocol entry before running
 	 * the new binary, so back it up
 	 */
-	CopyMem(&li_bak, li, sizeof(li_bak));
+	CopyMem(&shim_li_bak, shim_li, sizeof(shim_li_bak));
 
 	/*
 	 * Update the loaded image with the second stage loader file path
 	 */
-	li->FilePath = FileDevicePath(NULL, PathName);
-	if (!li->FilePath) {
+	shim_li->FilePath = FileDevicePath(NULL, PathName);
+	if (!shim_li->FilePath) {
 		perror(L"Unable to update loaded image file path\n");
 		efi_status = EFI_OUT_OF_RESOURCES;
 		goto restore;
@@ -1170,7 +1183,7 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 	/*
 	 * Verify and, if appropriate, relocate and execute the executable
 	 */
-	efi_status = handle_image(data, datasize, li, &entry_point,
+	efi_status = handle_image(data, datasize, shim_li, &entry_point,
 				  &alloc_address, &alloc_pages);
 	if (EFI_ERROR(efi_status)) {
 		perror(L"Failed to load image: %r\n", efi_status);
@@ -1187,13 +1200,7 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 	efi_status = entry_point(image_handle, systab);
 
 restore:
-	if (li->FilePath)
-		FreePool(li->FilePath);
-
-	/*
-	 * Restore our original loaded image values
-	 */
-	CopyMem(li, &li_bak, sizeof(li_bak));
+	restore_loaded_image();
 done:
 	if (PathName)
 		FreePool(PathName);

diff --git a/shim.h b/shim.h
@@ -201,6 +201,7 @@ extern EFI_STATUS VLogError(const char *file, int line, const char *func, const
 extern VOID LogHexdump_(const char *file, int line, const char *func, const void *data, size_t sz);
 extern VOID PrintErrors(VOID);
 extern VOID ClearErrors(VOID);
+extern VOID restore_loaded_image(VOID);
 extern EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath);
 extern EFI_STATUS import_mok_state(EFI_HANDLE image_handle);