On 07/08/17 15:38, Yao, Jiewen wrote:
> Thanks Laszlo. I think this is a special case we did not test before. And it
> is a bug we need fix.
>
> Unfortunately, I am out of office these days with limited email access. I
> just saw the email today.
>
>
> I have a quick look at the code.
Thank you for your help, and I'm sorry about disturbing you while you
are out of office.
> I believe we need add below check in
> UefiCpuPkg\PiSmmCpuDxeSmm\SmmCpuMemoryManagement.c,
> ConvertMemoryPageAttributes()
>
> ==========================
> if (BaseAddress > MAX_ADDRESS) {
> return RETURN_UNSUPPORTED;
> }
Mathematically speaking (not in C expressions), I think
BaseAddress:=(MAX_ADDRESS+1) with Length:=0 is valid as well. (In theory
anyway.)
So I would replace this check as follows:
if ((BaseAddress > 0) && ((BaseAddress - 1) > MAX_ADDRESS)) {
return RETURN_UNSUPPORTED;
}
> if (Length > MAX_ADDRESS) {
> return RETURN_UNSUPPORTED;
> }
Mathematically speaking (not in C expressions), I think BaseAddress:=0
with Length:=(MAX_ADDRESS+1) is valid too. (In theory anyway.)
So, I would replace this check as follows:
if ((Length > 0) && ((Length - 1) > MAX_ADDRESS)) {
return RETURN_UNSUPPORTED;
}
> if ((Length != 0) && (BaseAddress > MAX_ADDRESS - (Length - 1))) {
> return RETURN_UNSUPPORTED;
> }
Yes, this looks good. And, it would work correctly with the above two
modifications as well; it will accept both of the mentioned "corner cases".
(Anyway I haven't looked at the source, and the difference is purely
theoretical.)
> ==========================
> to filter invalid address in IA32.
>
>
> (Well, it is valid for OS, because OS may use PAE to match to lower. But it
> is invalid for UEFI, because UEFI uses identical address)
>
>
> Would you please file an HSD for that?
What does HSD stand for? :)
Either way, I've filed:
https://bugzilla.tianocore.org/show_bug.cgi?id=624
Thank you!
Laszlo
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:[email protected]]
>> Sent: Saturday, July 8, 2017 10:12 AM
>> To: Yao, Jiewen <[email protected]>
>> Cc: edk2-devel-01 <[email protected]>
>> Subject: memory protection crash in PiSmmCpuDxeSmm, Ia32 build with RAM
>> above 4GB
>>
>> Hi Jiewen,
>>
>> I just noticed that building OvmfIa32.dsc with -D SMM_REQUIRE, and then
>> running the 32-bit guest with 4G RAM (of which 2GB are placed in 64-bit
>> address pace), the guest crashes when PiSmmCpuDxeSmm tries to protect
>> the memory range at 4GB. Please find the log attached (it ends with the
>> crash).
>>
>> Is this expected to work?
>>
>> Thanks
>> Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
