Skip to content

Commit e2fbcd2

Browse files
committed
pe: Fix PF in GRUB after memattrs call
The call to update_mem_attrs() takes an aligned pointer within an allocated region but passes the entire size of the allocated region. The result is that Shim may remove execute permission from some pages belonging to GRUB causing a page fault upon returning from the LoadImage call. There are two cases: * When loading the image, set the memory attributes for exactly what we intend to load. * When freeing the image, be cautious and apply the edk2 workaround for the entire allocated region. Fixes: 226fee2 ("PE Loader: support and require NX") Fixes: 2f64bb9 ("loader-protocol: add workaround for EDK2 2025.02 page fault on FreePages") Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
1 parent afc4955 commit e2fbcd2

2 files changed

Lines changed: 3 additions & 6 deletions

File tree

loader-proto.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,6 @@ try_load_from_lf2(EFI_DEVICE_PATH *dp, buffer_properties_t *bprop)
186186
static void
187187
free_pages_alloc_image(SHIM_LOADED_IMAGE *image)
188188
{
189-
char *buffer;
190-
191189
if (!image || !image->alloc_address || !image->alloc_pages)
192190
return;
193191

@@ -196,9 +194,8 @@ free_pages_alloc_image(SHIM_LOADED_IMAGE *image)
196194
* are set on a loaded image, this will cause a page fault when it
197195
* is freed. Ensure W+ X- are set instead before freeing. */
198196

199-
buffer = (void *)ALIGN_VALUE((unsigned long)image->alloc_address, image->alloc_alignment);
200-
update_mem_attrs((uintptr_t)buffer, image->alloc_pages * PAGE_SIZE, MEM_ATTR_R|MEM_ATTR_W,
201-
MEM_ATTR_X);
197+
update_mem_attrs((uintptr_t)image->alloc_address,
198+
image->alloc_pages * PAGE_SIZE, MEM_ATTR_R|MEM_ATTR_W, MEM_ATTR_X);
202199

203200
BS->FreePages(image->alloc_address, image->alloc_pages);
204201
image->alloc_address = 0;

pe.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ handle_image (void *data, unsigned int datasize,
717717
dprint(L"Loading 0x%llx bytes at 0x%llx\n",
718718
(unsigned long long)context.ImageSize,
719719
(unsigned long long)(uintptr_t)buffer);
720-
update_mem_attrs((uintptr_t)buffer, alloc_size, MEM_ATTR_R|MEM_ATTR_W,
720+
update_mem_attrs((uintptr_t)buffer, context.ImageSize, MEM_ATTR_R|MEM_ATTR_W,
721721
MEM_ATTR_X);
722722

723723
CopyMem(buffer, data, context.SizeOfHeaders);

0 commit comments

Comments
 (0)