Bug 39415

On Fri, Jun 14, 2002 at 10:32:58PM +0200, Espen Skoglund wrote:
> 
> >Description:
> 
> The AllocatePages() call in sys/boot/efi/libefi/elf_freebsd.c only
> allocate one 4KB page.  The bi_load() in sys/boot/efi/libefi/bootinfo.c,
> however, assumes that 8KB has been allocated when stashing the EFI memory
> map behind the bootinfo.  This could lead to "interesting" behaviour
> in certain (albeit probably unlikely) scenarios.

Espen,

It looks to me that rounding to a multiple of the EFI page size is
more natural for an EFI application. I would probably fix bootinfo.c
instead of elf_freebsd.c. A second reason for fixing bootinfo.c is
that elf_freebsd.c works with arbitrary large bootinfo blocks, while
bootinfo.c only works if the bootinfo block is smaller than 8KB.

A second order fix would be to allocate enough; not just for the
bootinfo blocks, but also for the memory map...

Thoughts?

-- 
 Marcel Moolenaar	  USPA: A-39004		 marcel@xcllnt.net

[Marcel Moolenaar]
> It looks to me that rounding to a multiple of the EFI page size is
> more natural for an EFI application. I would probably fix bootinfo.c
> instead of elf_freebsd.c. A second reason for fixing bootinfo.c is
> that elf_freebsd.c works with arbitrary large bootinfo blocks, while
> bootinfo.c only works if the bootinfo block is smaller than 8KB.

> A second order fix would be to allocate enough; not just for the
> bootinfo blocks, but also for the memory map...

> Thoughts?

You're right.  It makes more sense to fix it properly.  Here's a
better fix.  It gets the size of the memmap and uses it for
determining the allocation size.  The size of the allocated memory
behind the bootinfo is then passed on to bootinfo.c (the size of
memmap itself is not passed since it may increase due to an
AllocatePages() call).

	eSk


================================================================
--- elf_freebsd.c.orig	Wed Jun 19 21:14:43 2002
+++ elf_freebsd.c	Wed Jun 19 21:16:04 2002
@@ -143,15 +143,22 @@
 	struct ia64_pte		pte;
 	struct bootinfo		*bi;
 	u_int64_t		psr;
-	UINTN			mapkey;
+	UINTN			mapkey, size, dummy1;
+	UINT32			dummy2;
 	EFI_STATUS		status;
 
 	if ((md = file_findmetadata(fp, MODINFOMD_ELFHDR)) == NULL)
 		return(EFTYPE);			/* XXX actually EFUCKUP */
 	hdr = (Elf_Ehdr *)&(md->md_data);
 
+	/* Get size of EFI memory map */
+	size = 0;
+	BS->GetMemoryMap(&size, (EFI_MEMORY_DESCRIPTOR *)&dummy1,
+			 &mapkey, &dummy1, &dummy2);
+
+	size = EFI_SIZE_TO_PAGES(sizeof(struct bootinfo) + size);
 	status = BS->AllocatePages(AllocateAnyPages, EfiLoaderData,
-	    EFI_SIZE_TO_PAGES(sizeof(struct bootinfo)), (void*)&bi);
+				   size, (void*)&bi);
 	if (EFI_ERROR(status)) {
 		printf("unable to create bootinfo block (status=0x%lx)\n",
 		    (long)status);
@@ -159,7 +166,8 @@
 	}
 
 	bzero(bi, sizeof(struct bootinfo));
-	bi_load(bi, fp, &mapkey);
+	bi_load(bi, fp, &mapkey,
+		(size << EFI_PAGE_SHIFT) - sizeof(struct bootinfo));
 
 	printf("Entering %s at 0x%lx...\n", fp->f_name, hdr->e_entry);
 
--- bootinfo.c.orig	Wed Jun 19 21:14:50 2002
+++ bootinfo.c	Wed Jun 19 21:15:46 2002
@@ -242,7 +242,8 @@
  * - Module metadata are formatted and placed in kernel space.
  */
 int
-bi_load(struct bootinfo *bi, struct preloaded_file *fp, UINTN *mapkey)
+bi_load(struct bootinfo *bi, struct preloaded_file *fp, UINTN *mapkey,
+	UINTN alloced_size)
 {
     char			*rootdevname;
     struct efi_devdesc		*rootdev;
@@ -331,7 +332,7 @@
 
     /* read memory map and stash it after bootinfo */
     bi->bi_memmap = (u_int64_t)(bi + 1);
-    bi->bi_memmap_size = 8192 - sizeof(struct bootinfo);
+    bi->bi_memmap_size = alloced_size;
     status = BS->GetMemoryMap(&bi->bi_memmap_size,
 			      (EFI_MEMORY_DESCRIPTOR *)bi->bi_memmap,
 			      &key,

Comment 3 Marcel Moolenaar 2002-10-24 08:52:17 UTC

State Changed
From-To: open->closed

Fixed. Patch applied with modifications. An additional problem was 
that optaining the size of the memory map before doing an additional 
allocation could cause the previously obtained size to be too small 
to actually hold the memory map after the allocation.