Laszlo:
So, your proposal is to change line 1158 if ((Entry->BaseAddress +
Length) > MaxAddress) { ==> if ((Entry->BaseAddress + Length - 1) >
MaxAddress) {?
Thanks
Liming
-----Original Message-----
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Thursday, February 26, 2015 2:06 AM
To: edk2-devel list
Subject: Re: [edk2] possible off-by-one in CoreAllocateSpace()?
On 02/13/15 13:42, Laszlo Ersek wrote:
> Hi,
>
> there's this code in CoreAllocateSpace(), in
> "MdeModulePkg/Core/Dxe/Gcd/Gcd.c":
>
> 1156 if (GcdAllocateType ==
> EfiGcdAllocateMaxAddressSearchTopDown ||
> 1157 GcdAllocateType == EfiGcdAllocateAnySearchTopDown) {
> 1158 if ((Entry->BaseAddress + Length) > MaxAddress) {
> 1159 continue;
> 1160 }
> 1161 if (Length > (Entry->EndAddress + 1)) {
> 1162 Status = EFI_NOT_FOUND;
> 1163 goto Done;
> 1164 }
> 1165 if (Entry->EndAddress > MaxAddress) {
> 1166 *BaseAddress = MaxAddress;
> 1167 } else {
> 1168 *BaseAddress = Entry->EndAddress;
> 1169 }
> 1170 *BaseAddress = (*BaseAddress + 1 - Length) &
> (~AlignmentMask);
> 1171 } else {
> 1172 *BaseAddress = (Entry->BaseAddress + AlignmentMask) &
> (~AlignmentMask);
> 1173 if ((*BaseAddress + Length - 1) > MaxAddress) {
> 1174 Status = EFI_NOT_FOUND;
> 1175 goto Done;
> 1176 }
> 1177 }
>
> When serching bottom-up (which is the more usual case), the check on line
> 1173 considers the fact that MaxAddress is an *inclusive* limit, while Length
> is an *exclusive* size. That is, the following situation is valid:
>
> BaseAddress + Length == MaxAddress + 1
>
> which is equivalent to
>
> BaseAddress + Length - 1 == MaxAddress
>
> Hence the check on line 1173
>
> BaseAddress + Length - 1 > MaxAddress
>
> is a correct check for invalidity. Okay.
>
> However, the invalidity check on line 1158 seems wrong, for the top-down
> search:
>
> Entry->BaseAddress + Length > MaxAddress
>
> is too strict (off-by-one). Namely,
>
> BaseAddress + Length == MaxAddress + 1
>
> is a valid condition (see above), but it triggers:
>
> BaseAddress + Length == MaxAddress + 1 > MaxAddress
>
> I noticed this error by printing GCD maps (DEBUG_GCD) and using
> EfiGcdAllocateMaxAddressSearchTopDown.
Ping.
This issue is not a security problem, it just pessimizes allocations (it wastes
address space).
Thanks,
Laszlo
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
