stub: Properly clean up pages on error

diff --git a/src/boot/efi/linux.c b/src/boot/efi/linux.c
index 3cbffdbbeb59489e9fbea477415527e8b6b90f76..9750006f19d0360fc3514d8c21109ab91281593b 100644 (file)
--- a/src/boot/efi/linux.c
+++ b/src/boot/efi/linux.c
@@ -92,18 +92,6 @@ static inline void cleanup_loaded_image(EFI_HANDLE *loaded_image_handle) {
         *loaded_image_handle = NULL;
 }
 
-/* struct to call cleanup_pages */
-struct pages {
-        EFI_PHYSICAL_ADDRESS addr;
-        UINTN num;
-};
-
-static inline void cleanup_pages(struct pages *p) {
-        if (p->addr == 0)
-                return;
-        (void) BS->FreePages(p->addr, p->num);
-}
-
 EFI_STATUS linux_exec(
                 EFI_HANDLE image,
                 const char *cmdline, UINTN cmdline_len,
@@ -114,7 +102,6 @@ EFI_STATUS linux_exec(
         _cleanup_(cleanup_loaded_image) EFI_HANDLE loaded_image_handle = NULL;
         uint32_t kernel_alignment, kernel_size_of_image, kernel_entry_address;
         EFI_IMAGE_ENTRY_POINT kernel_entry;
-        _cleanup_(cleanup_pages) struct pages kernel = {};
         void *new_buffer;
         EFI_STATUS err;
 
@@ -140,10 +127,11 @@ EFI_STATUS linux_exec(
            if they are not met. x86 and x86_64 kernel stubs don't do checks and fail if the BSS section is too small.
         */
         /* allocate SizeOfImage + SectionAlignment because the new_buffer can move up to Alignment-1 bytes */
-        kernel.num = EFI_SIZE_TO_PAGES(ALIGN_TO(kernel_size_of_image, kernel_alignment) + kernel_alignment);
-        err = BS->AllocatePages(AllocateAnyPages, EfiLoaderCode, kernel.num, &kernel.addr);
-        if (err != EFI_SUCCESS)
-                return EFI_OUT_OF_RESOURCES;
+        _cleanup_pages_ Pages kernel = xmalloc_pages(
+                        AllocateAnyPages,
+                        EfiLoaderCode,
+                        EFI_SIZE_TO_PAGES(ALIGN_TO(kernel_size_of_image, kernel_alignment) + kernel_alignment),
+                        0);
         new_buffer = PHYSICAL_ADDRESS_TO_POINTER(ALIGN_TO(kernel.addr, kernel_alignment));
         memcpy(new_buffer, linux_buffer, linux_length);
         /* zero out rest of memory (probably not needed, but BSS section should be 0) */

diff --git a/src/boot/efi/linux_x86.c b/src/boot/efi/linux_x86.c
index 1174aa3ece90395a588f5532c30857df7849f413..fc007c3b8f01c021ac44df0e17901d29df918e08 100644 (file)
--- a/src/boot/efi/linux_x86.c
+++ b/src/boot/efi/linux_x86.c
@@ -126,7 +126,6 @@ EFI_STATUS linux_exec(
                 const void *initrd_buffer, UINTN initrd_length) {
 
         EFI_HANDLE initrd_handle = NULL;
-        EFI_PHYSICAL_ADDRESS addr;
         EFI_STATUS err;
 
         assert(image);
@@ -162,16 +161,12 @@ EFI_STATUS linux_exec(
                 return log_error_status_stall(
                                 EFI_UNSUPPORTED, u"Initrd is above 4G, but kernel lacks support.");
 
-        addr = UINT32_MAX; /* Below the 32bit boundary */
-        err = BS->AllocatePages(
+        _cleanup_pages_ Pages boot_params_page = xmalloc_pages(
                         can_4g ? AllocateAnyPages : AllocateMaxAddress,
                         EfiLoaderData,
                         EFI_SIZE_TO_PAGES(sizeof(BootParams)),
-                        &addr);
-        if (err != EFI_SUCCESS)
-                return err;
-
-        BootParams *boot_params = PHYSICAL_ADDRESS_TO_POINTER(addr);
+                        UINT32_MAX /* Below the 4G boundary */);
+        BootParams *boot_params = PHYSICAL_ADDRESS_TO_POINTER(boot_params_page.addr);
         *boot_params = (BootParams){};
 
         /* Setup size is determined by offset 0x0202 + byte value at offset 0x0201, which is the same as
@@ -186,22 +181,19 @@ EFI_STATUS linux_exec(
         if (boot_params->hdr.setup_sects == 0)
                 boot_params->hdr.setup_sects = 4;
 
+        _cleanup_pages_ Pages cmdline_pages = {};
         if (cmdline) {
-                addr = CMDLINE_PTR_MAX;
-
-                err = BS->AllocatePages(
+                cmdline_pages = xmalloc_pages(
                                 can_4g ? AllocateAnyPages : AllocateMaxAddress,
                                 EfiLoaderData,
                                 EFI_SIZE_TO_PAGES(cmdline_len + 1),
-                                &addr);
-                if (err != EFI_SUCCESS)
-                        return err;
-
-                memcpy(PHYSICAL_ADDRESS_TO_POINTER(addr), cmdline, cmdline_len);
-                ((char *) PHYSICAL_ADDRESS_TO_POINTER(addr))[cmdline_len] = 0;
-                boot_params->hdr.cmd_line_ptr = (uint32_t) addr;
-                boot_params->ext_cmd_line_ptr = addr >> 32;
-                assert(can_4g || addr <= CMDLINE_PTR_MAX);
+                                CMDLINE_PTR_MAX);
+
+                memcpy(PHYSICAL_ADDRESS_TO_POINTER(cmdline_pages.addr), cmdline, cmdline_len);
+                ((char *) PHYSICAL_ADDRESS_TO_POINTER(cmdline_pages.addr))[cmdline_len] = 0;
+                boot_params->hdr.cmd_line_ptr = (uint32_t) cmdline_pages.addr;
+                boot_params->ext_cmd_line_ptr = cmdline_pages.addr >> 32;
+                assert(can_4g || cmdline_pages.addr <= CMDLINE_PTR_MAX);
         }
 
         /* Providing the initrd via LINUX_INITRD_MEDIA_GUID is only supported by Linux 5.8+ (5.7+ on ARM64).

diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c
index 494972fa38c551ce7e000dc0e61ae2755e27c528..2e8b4d99d2224a47403912f70b200a73df9f9247 100644 (file)
--- a/src/boot/efi/stub.c
+++ b/src/boot/efi/stub.c
@@ -21,14 +21,11 @@ _used_ _section_(".sdmagic") static const char magic[] = "#### LoaderInfo: syste
 static EFI_STATUS combine_initrd(
                 EFI_PHYSICAL_ADDRESS initrd_base, UINTN initrd_size,
                 const void * const extra_initrds[], const size_t extra_initrd_sizes[], size_t n_extra_initrds,
-                EFI_PHYSICAL_ADDRESS *ret_initrd_base, UINTN *ret_initrd_size) {
+                Pages *ret_initr_pages, UINTN *ret_initrd_size) {
 
-        EFI_PHYSICAL_ADDRESS base = UINT32_MAX; /* allocate an area below the 32bit boundary for this */
-        EFI_STATUS err;
-        uint8_t *p;
         UINTN n;
 
-        assert(ret_initrd_base);
+        assert(ret_initr_pages);
         assert(ret_initrd_size);
 
         /* Combines four initrds into one, by simple concatenation in memory */
@@ -45,15 +42,12 @@ static EFI_STATUS combine_initrd(
                 n += extra_initrd_sizes[i];
         }
 
-        err = BS->AllocatePages(
+        _cleanup_pages_ Pages pages = xmalloc_pages(
                         AllocateMaxAddress,
                         EfiLoaderData,
                         EFI_SIZE_TO_PAGES(n),
-                        &base);
-        if (err != EFI_SUCCESS)
-                return log_error_status_stall(err, L"Failed to allocate space for combined initrd: %r", err);
-
-        p = PHYSICAL_ADDRESS_TO_POINTER(base);
+                        UINT32_MAX /* Below 4G boundary. */);
+        uint8_t *p = PHYSICAL_ADDRESS_TO_POINTER(pages.addr);
         if (initrd_base != 0) {
                 UINTN pad;
 
@@ -75,10 +69,11 @@ static EFI_STATUS combine_initrd(
                 p = mempcpy(p, extra_initrds[i], extra_initrd_sizes[i]);
         }
 
-        assert((uint8_t*) PHYSICAL_ADDRESS_TO_POINTER(base) + n == p);
+        assert(PHYSICAL_ADDRESS_TO_POINTER(pages.addr + n) == p);
 
-        *ret_initrd_base = base;
+        *ret_initr_pages = pages;
         *ret_initrd_size = n;
+        pages.n_pages = 0;
 
         return EFI_SUCCESS;
 }
@@ -342,6 +337,7 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) {
         dt_size = szs[UNIFIED_SECTION_DTB];
         dt_base = dt_size != 0 ? POINTER_TO_PHYSICAL_ADDRESS(loaded_image->ImageBase) + addrs[UNIFIED_SECTION_DTB] : 0;
 
+        _cleanup_pages_ Pages initrd_pages = {};
         if (credential_initrd || global_credential_initrd || sysext_initrd) {
                 /* If we have generated initrds dynamically, let's combine them with the built-in initrd. */
                 err = combine_initrd(
@@ -361,10 +357,12 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) {
                                         pcrpkey_initrd_size,
                                 },
                                 5,
-                                &initrd_base, &initrd_size);
+                                &initrd_pages, &initrd_size);
                 if (err != EFI_SUCCESS)
                         return err;
 
+                initrd_base = initrd_pages.addr;
+
                 /* Given these might be large let's free them explicitly, quickly. */
                 credential_initrd = mfree(credential_initrd);
                 global_credential_initrd = mfree(global_credential_initrd);

diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h
index 684930d5d66d3ab283d55fb7aea21e3ba7d42092..be79b205a42264956ea88155958345660813f9ca 100644 (file)
--- a/src/boot/efi/util.h
+++ b/src/boot/efi/util.h
@@ -70,6 +70,32 @@ static inline void *xrealloc(void *p, size_t old_size, size_t new_size) {
 #define xpool_print(fmt, ...) ((char16_t *) ASSERT_SE_PTR(PoolPrint((fmt), ##__VA_ARGS__)))
 #define xnew(type, n) ((type *) xmalloc_multiply(sizeof(type), (n)))
 
+typedef struct {
+        EFI_PHYSICAL_ADDRESS addr;
+        size_t n_pages;
+} Pages;
+
+static inline void cleanup_pages(Pages *p) {
+        if (p->n_pages == 0)
+                return;
+#ifdef EFI_DEBUG
+        assert_se(BS->FreePages(p->addr, p->n_pages) == EFI_SUCCESS);
+#else
+        (void) BS->FreePages(p->addr, p->n_pages);
+#endif
+}
+
+#define _cleanup_pages_ _cleanup_(cleanup_pages)
+
+static inline Pages xmalloc_pages(
+                EFI_ALLOCATE_TYPE type, EFI_MEMORY_TYPE memory_type, size_t n_pages, EFI_PHYSICAL_ADDRESS addr) {
+        assert_se(BS->AllocatePages(type, memory_type, n_pages, &addr) == EFI_SUCCESS);
+        return (Pages) {
+                .addr = addr,
+                .n_pages = n_pages,
+        };
+}
+
 EFI_STATUS parse_boolean(const char *v, bool *b);
 
 EFI_STATUS efivar_set(const EFI_GUID *vendor, const char16_t *name, const char16_t *value, uint32_t flags);