public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: edk2-devel@lists.01.org
Cc: leif.lindholm@linaro.org, eugene@hp.com, feng.tian@intel.com,
	star.zeng@intel.com, Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH] MdeModulePkg/DxeCore: deal with allocations spanning several memmap entries
Date: Thu, 16 Mar 2017 11:35:28 +0000	[thread overview]
Message-ID: <1489664128-30700-1-git-send-email-ard.biesheuvel@linaro.org> (raw)

When attempting to perform page allocations using AllocateAddress, we
fail to check whether the entire region is free before splitting the
region. This may lead to memory being leaked further into the routine,
when it turns out that one of the memory map entries intersected by the
region is already occupied. In this case, prior conversions are not rolled
back.

For instance, starting from this situation

0x000040000000-0x00004007ffff [ConventionalMemory ]
0x000040080000-0x00004009ffff [Boot Data          ]
0x0000400a0000-0x000047ffffff [ConventionalMemory ]

a failed EfiLoaderData allocation @ 0x40000000 that covers the BootData
region will fail, but leave the first part of the allocation converted,
so we end up with

0x000040000000-0x00004007ffff [Loader Data        ]
0x000040080000-0x00004009ffff [Boot Data          ]
0x0000400a0000-0x000047ffffff [ConventionalMemory ]

even though the AllocatePages() call returned an error.

So let's check beforehand that AllocateAddress allocations are covered
by a single memory map entry, so that it either succeeds or fails
completely, rather than leaking allocations.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Core/Dxe/Mem/Page.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 260a30a214c7..92306b2f1b45 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -755,6 +755,17 @@ CoreConvertPagesEx (
     }
 
     //
+    // If we are converting the type of the range from EfiConventionalMemory to
+    // another type, we have to ensure that the entire range is covered by a
+    // single entry.
+    //
+    if (ChangingType && (NewType != EfiConventionalMemory)) {
+      if (Entry->Start > End || Entry->End < End) {
+        DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: range %lx - %lx covers multiple entries\n", Start, End));
+        return EFI_NOT_FOUND;
+      }
+    }
+    //
     // Convert range to the end, or to the end of the descriptor
     // if that's all we've got
     //
-- 
2.7.4



             reply	other threads:[~2017-03-16 11:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-16 11:35 Ard Biesheuvel [this message]
2017-03-16 13:19 ` [PATCH] MdeModulePkg/DxeCore: deal with allocations spanning several memmap entries Ard Biesheuvel
2017-03-17  3:45   ` Zeng, Star
2017-03-17 18:51     ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1489664128-30700-1-git-send-email-ard.biesheuvel@linaro.org \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox