public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* WSMT bits
@ 2020-03-10  9:36 Laszlo Ersek
  2020-03-10 13:48 ` Laszlo Ersek
  0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2020-03-10  9:36 UTC (permalink / raw)
  To: Jiewen Yao; +Cc: edk2-devel-groups-io, Ray Ni

Hi Jiewen,

reading the following chapter:

  https://edk2-docs.gitbooks.io/a-tour-beyond-bios-memory-protection-in-uefi-bios/content/memory-protection-in-SMM.html

I'm having trouble associating the protection features implemented in
edk2 with the various bits in the WSMT (per
"MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h").

For example, it seems like the bits a platform sets in the WSMT *might*
depend on "PcdCpuSmmRestrictedMemoryAccess".

Can someone clarify these please?


FWIW, in the edk2-platforms tree, the
"Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c" source
file sets EFI_WSMT_PROTECTION_FLAGS_FIXED_COMM_BUFFERS and
EFI_WSMT_PROTECTION_FLAGS_COMM_BUFFER_NESTED_PTR_PROTECTION. It does not
set EFI_WSMT_PROTECTION_FLAGS_SYSTEM_RESOURCE_PROTECTION.

Is this bitmask (from Vlv2TbltDevicePkg) the general pattern that other
edk2 platforms with SMM support should expose too, as a starting point?

Does Vlv2TbltDevicePkg perform some specific actions in order to claim
these feature bits, or do they simply report guarantees that the core
edk2 SMM infrastructure provides out of the box?

This code was originally added to Vlv2TbltDevicePkg in edk2 (not
edk2-platforms) commit 2c855d3aaf36d (preceding the movement of
Vlv2TbltDevicePkg to edk2-platforms):

commit 2c855d3aaf36da80f8c4f0ae12d31900a628b0a9
Author: Lu, ShifeiX A <shifeix.a.lu@intel.com>
Date:   Thu Jul 28 16:21:28 2016 +0800

    Vlv2DeviceRefCodePkg&Vlv2DevicePkg:Add sample WSMT table.

    This is an sample WSMT table, which we only
    update BIT0 and BIT1 of Protections flags fields.

    Contributed-under: TianoCore Contribution Agreement 1.0
    Signed-off-by: lushifex <shifeix.a.lu@intel.com>
    Reviewed-by: David Wei <david.wei@intel.com>

 Vlv2DeviceRefCodePkg/AcpiTablesPCAT/AcpiTables.inf |  3 ++-
 Vlv2DeviceRefCodePkg/AcpiTablesPCAT/Wsmt/Wsmt.aslc | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c      | 13 +++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)

And that's not a lot of explanation, unfortunately.

(Note: I have not read the WSMT spec.)

Thanks,
Laszlo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: WSMT bits
  2020-03-10  9:36 WSMT bits Laszlo Ersek
@ 2020-03-10 13:48 ` Laszlo Ersek
  2020-03-11  2:01   ` Yao, Jiewen
  0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2020-03-10 13:48 UTC (permalink / raw)
  To: Jiewen Yao; +Cc: edk2-devel-groups-io, Ray Ni

Hi again Jiewen,

On 03/10/20 10:36, Laszlo Ersek wrote:
> Hi Jiewen,
> 
> reading the following chapter:
> 
>   https://edk2-docs.gitbooks.io/a-tour-beyond-bios-memory-protection-in-uefi-bios/content/memory-protection-in-SMM.html
> 
> I'm having trouble associating the protection features implemented in
> edk2 with the various bits in the WSMT (per
> "MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h").
> 
> For example, it seems like the bits a platform sets in the WSMT *might*
> depend on "PcdCpuSmmRestrictedMemoryAccess".
> 
> Can someone clarify these please?
> 
> 
> FWIW, in the edk2-platforms tree, the
> "Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c" source
> file sets EFI_WSMT_PROTECTION_FLAGS_FIXED_COMM_BUFFERS and
> EFI_WSMT_PROTECTION_FLAGS_COMM_BUFFER_NESTED_PTR_PROTECTION. It does not
> set EFI_WSMT_PROTECTION_FLAGS_SYSTEM_RESOURCE_PROTECTION.
> 
> Is this bitmask (from Vlv2TbltDevicePkg) the general pattern that other
> edk2 platforms with SMM support should expose too, as a starting point?

I have now read another whitepaper from you:

  A Tour Beyond BIOS
  Secure SMM Communication in the EFI Developer Kit II

It's a great whitepaper, it answers all my questions. Based on it, I
think that OVMF should enable all three bits in the WSMT:

(1) BIT0 -- FIXED_COMM_BUFFERS:

(1.1) OVMF uses the standard edk2 SMM infrastructure. Drivers in that
infrastructure verify that the communication buffers that they receive
are in permitted regions (no overlap with SMRAM, and resident in
permitted memory types), via SmmIsBufferOutsideSmmValid().

(1.2) OVMF contains only one custom SMI handler (which is for CPU
hotplug). This handler does use normal RAM for a bit of communication
with hot-added CPUs, but the RAM used for this purpose is allocated as
reserved memory, and before End-of-Dxe.

Furthermore, TOC-TOU is actively considered and mitigated, as the
"communication area" is a single byte (a boolean flag), and the design
considers the OS actively attacking that byte.

(2) BIT1 -- COMM_BUFFER_NESTED_PTR_PROTECTION:

OVMF uses the standard edk2 SMM infra, which validates the targets of
nested pointers.

The custom SMI handler (for CPU hotplug) does not process pointers that
arrive from an untrusted context.

(3) BIT2 -- SYSTEM_RESOURCE_PROTECTION:

This bit is defined somewhat unclearly, but OVMF does lock both TSEG,
and the QEMU-specific "SMRAM at default SMBASE", at SmmReadyToLock. See
e.g. commit 9108fc17b09c ("OvmfPkg/SmmAccess: close and lock SMRAM at
default SMBASE", 2020-02-05).

So I think OVMF should install the WSMT, and set all three bits.


However, there's a catch, for (1) FIXED_COMM_BUFFERS:

(1.3) While OVMF produces a Memory Type Information HOB, it is not
*adaptive*.

I'm not sure what the whitepaper suggests:

(1.3.1) that the HOB exist (because then released / unused BINs will
never become usable memory in the final memory map),

(1.3.2) or that BINs should actually accommodate all the allocations (so
that no runtime memory is reallocated *ever*).

Because, in the non-adaptive approach (1.3.1), what happens if there is
a RuntimeData reallocation after End-of-Dxe, but even the *previous*
allocation (that's now getting released) used to live outside of a BIN?

Unfortunately, enabling the adaptive Memory Type Information (1.3.2) for
OVMF is challenging, due to
<https://bugzilla.tianocore.org/show_bug.cgi?id=1086>. OVMF can be
booted with 128 MiB RAM and 1 TiB RAM just the same, and so a platform
PEIM cannot tell, in advance, what is a valid Memory Type Information
variable that the DXE core will accept, vs. an invalid Memory Type
Information variable that the DXE core will reject (and hang upon, due
to BIN priming failure).

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: WSMT bits
  2020-03-10 13:48 ` Laszlo Ersek
@ 2020-03-11  2:01   ` Yao, Jiewen
  2020-03-11 10:23     ` Laszlo Ersek
  0 siblings, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2020-03-11  2:01 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-groups-io, Ni, Ray

Thanks Laszlo. I am glad that you like the whitepaper.

Right. The platform should assert the WSMT bit, because the platform need make sure that all SMI handlers do right thing for SMM communication buffer.
Using PcdCpuSmmRestrictedMemoryAccess is a good way to ensure any violation can be caught. It is highly recommend, unless it conflicts with some other features such as heap guard debug, or server memory hotplug.

I recommend we keep MemoryTypeInfo as a separate topic for https://bugzilla.tianocore.org/show_bug.cgi?id=1086 
I still think memory type information should be used - 1.3.2.

I just do not understand well enough on your comment - " OVMF can be booted with 128 MiB RAM and 1 TiB RAM just the same, and so a platform PEIM cannot tell, in advance, what is a valid Memory Type Information variable that the DXE core will accept ..."
You are right that OVMF can boot with 128MB or 1TB. But that is all DRAM, right?
BIN size should be same, because BIN only includes Runtime/ACPI/Reserved memory. That is no reason to make them different between 128MB and 1TB.
Or do I misunderstand something? Would you please educate me why they are different?

Thank you
Yao Jiewen


> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, March 10, 2020 9:48 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: WSMT bits
> 
> Hi again Jiewen,
> 
> On 03/10/20 10:36, Laszlo Ersek wrote:
> > Hi Jiewen,
> >
> > reading the following chapter:
> >
> >   https://edk2-docs.gitbooks.io/a-tour-beyond-bios-memory-protection-in-
> uefi-bios/content/memory-protection-in-SMM.html
> >
> > I'm having trouble associating the protection features implemented in
> > edk2 with the various bits in the WSMT (per
> > "MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h").
> >
> > For example, it seems like the bits a platform sets in the WSMT *might*
> > depend on "PcdCpuSmmRestrictedMemoryAccess".
> >
> > Can someone clarify these please?
> >
> >
> > FWIW, in the edk2-platforms tree, the
> > "Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c" source
> > file sets EFI_WSMT_PROTECTION_FLAGS_FIXED_COMM_BUFFERS and
> >
> EFI_WSMT_PROTECTION_FLAGS_COMM_BUFFER_NESTED_PTR_PROTECTION.
> It does not
> > set EFI_WSMT_PROTECTION_FLAGS_SYSTEM_RESOURCE_PROTECTION.
> >
> > Is this bitmask (from Vlv2TbltDevicePkg) the general pattern that other
> > edk2 platforms with SMM support should expose too, as a starting point?
> 
> I have now read another whitepaper from you:
> 
>   A Tour Beyond BIOS
>   Secure SMM Communication in the EFI Developer Kit II
> 
> It's a great whitepaper, it answers all my questions. Based on it, I
> think that OVMF should enable all three bits in the WSMT:
> 
> (1) BIT0 -- FIXED_COMM_BUFFERS:
> 
> (1.1) OVMF uses the standard edk2 SMM infrastructure. Drivers in that
> infrastructure verify that the communication buffers that they receive
> are in permitted regions (no overlap with SMRAM, and resident in
> permitted memory types), via SmmIsBufferOutsideSmmValid().
> 
> (1.2) OVMF contains only one custom SMI handler (which is for CPU
> hotplug). This handler does use normal RAM for a bit of communication
> with hot-added CPUs, but the RAM used for this purpose is allocated as
> reserved memory, and before End-of-Dxe.
> 
> Furthermore, TOC-TOU is actively considered and mitigated, as the
> "communication area" is a single byte (a boolean flag), and the design
> considers the OS actively attacking that byte.
> 
> (2) BIT1 -- COMM_BUFFER_NESTED_PTR_PROTECTION:
> 
> OVMF uses the standard edk2 SMM infra, which validates the targets of
> nested pointers.
> 
> The custom SMI handler (for CPU hotplug) does not process pointers that
> arrive from an untrusted context.
> 
> (3) BIT2 -- SYSTEM_RESOURCE_PROTECTION:
> 
> This bit is defined somewhat unclearly, but OVMF does lock both TSEG,
> and the QEMU-specific "SMRAM at default SMBASE", at SmmReadyToLock. See
> e.g. commit 9108fc17b09c ("OvmfPkg/SmmAccess: close and lock SMRAM at
> default SMBASE", 2020-02-05).
> 
> So I think OVMF should install the WSMT, and set all three bits.
> 
> 
> However, there's a catch, for (1) FIXED_COMM_BUFFERS:
> 
> (1.3) While OVMF produces a Memory Type Information HOB, it is not
> *adaptive*.
> 
> I'm not sure what the whitepaper suggests:
> 
> (1.3.1) that the HOB exist (because then released / unused BINs will
> never become usable memory in the final memory map),
> 
> (1.3.2) or that BINs should actually accommodate all the allocations (so
> that no runtime memory is reallocated *ever*).
> 
> Because, in the non-adaptive approach (1.3.1), what happens if there is
> a RuntimeData reallocation after End-of-Dxe, but even the *previous*
> allocation (that's now getting released) used to live outside of a BIN?
> 
> Unfortunately, enabling the adaptive Memory Type Information (1.3.2) for
> OVMF is challenging, due to
> <https://bugzilla.tianocore.org/show_bug.cgi?id=1086>. OVMF can be
> booted with 128 MiB RAM and 1 TiB RAM just the same, and so a platform
> PEIM cannot tell, in advance, what is a valid Memory Type Information
> variable that the DXE core will accept, vs. an invalid Memory Type
> Information variable that the DXE core will reject (and hang upon, due
> to BIN priming failure).
> 
> Thanks
> Laszlo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: WSMT bits
  2020-03-11  2:01   ` Yao, Jiewen
@ 2020-03-11 10:23     ` Laszlo Ersek
  2020-03-11 12:00       ` Yao, Jiewen
  0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2020-03-11 10:23 UTC (permalink / raw)
  To: Yao, Jiewen; +Cc: edk2-devel-groups-io, Ni, Ray

On 03/11/20 03:01, Yao, Jiewen wrote:
> Thanks Laszlo. I am glad that you like the whitepaper.
> 
> Right. The platform should assert the WSMT bit, because the platform need make sure that all SMI handlers do right thing for SMM communication buffer.
> Using PcdCpuSmmRestrictedMemoryAccess is a good way to ensure any violation can be caught. It is highly recommend, unless it conflicts with some other features such as heap guard debug, or server memory hotplug.

Thanks for your answer! And yes, PcdCpuSmmRestrictedMemoryAccess is TRUE
in OVMF. (Inheriting the DEC default.)

> 
> I recommend we keep MemoryTypeInfo as a separate topic for https://bugzilla.tianocore.org/show_bug.cgi?id=1086 
> I still think memory type information should be used - 1.3.2.

Right. In the end, I posted an OvmfPkg patch series yesterday, for
covering point (1.3.2) -- adaptive MemoryTypeInfo:

[edk2-devel] [PATCH 0/5]
OvmfPkg: improve SMM comms security with adaptive MemoryTypeInformation

http://mid.mail-archive.com/20200310222739.26717-1-lersek@redhat.com
https://edk2.groups.io/g/devel/message/55726

I decided that TianoCore#1086 should not be considered a blocker for
this feature. My reasoning is captured here:

https://bugzilla.tianocore.org/show_bug.cgi?id=386#c15

> I just do not understand well enough on your comment - " OVMF can be booted with 128 MiB RAM and 1 TiB RAM just the same, and so a platform PEIM cannot tell, in advance, what is a valid Memory Type Information variable that the DXE core will accept ..."
> You are right that OVMF can boot with 128MB or 1TB. But that is all DRAM, right?
> BIN size should be same, because BIN only includes Runtime/ACPI/Reserved memory. That is no reason to make them different between 128MB and 1TB.
> Or do I misunderstand something? Would you please educate me why they are different?

This is a great point, and I remember that both of your white papers
("Memory Map and Practices in UEFI BIOS", and "Secure SMM
Communication") recommend that only Runtime/ACPI/Reserved memory be
tracked by MemoryTypeInfo. That will indeed make the guest RAM size
irrelevant.

For now, the series that I posted, sticks with the *pre-existent* OVMF
practice that boot services data/code too are tracked by MemoryTypeInfo:

  { EfiACPIMemoryNVS,       0x004 },
  { EfiACPIReclaimMemory,   0x008 },
  { EfiReservedMemoryType,  0x004 },
  { EfiRuntimeServicesData, 0x024 },
  { EfiRuntimeServicesCode, 0x030 },
  { EfiBootServicesCode,    0x180 },
  { EfiBootServicesData,    0xF00 },
  { EfiMaxMemoryType,       0x000 }

I didn't want to diverge from that practice at once, in the current
patch series.

I'd like to adopt the recommendation from the white papers -- i.e., stop
tracking EfiACPIReclaimMemory, EfiBootServicesCode, and
EfiBootServicesData -- as a separate work item.

Actually, can you please clarify: the "Secure SMM Communication" does
not recommend tracking EfiACPIReclaimMemory, but "Memory Map and
Practices in UEFI BIOS" does. So what is the right thing to do, should
we continue tracking EfiACPIReclaimMemory?

Anyway, let me thank you very much, as I didn't make the connection
myself that Runtime/ACPI/Reserved are not expected to *scale* with the
guest RAM size, even if the BS and Loader memory footprint of various
UEFI applications / drivers could.

Cheers!
Laszlo

> 
> Thank you
> Yao Jiewen
> 
> 
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, March 10, 2020 9:48 PM
>> To: Yao, Jiewen <jiewen.yao@intel.com>
>> Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
>> Subject: Re: WSMT bits
>>
>> Hi again Jiewen,
>>
>> On 03/10/20 10:36, Laszlo Ersek wrote:
>>> Hi Jiewen,
>>>
>>> reading the following chapter:
>>>
>>>   https://edk2-docs.gitbooks.io/a-tour-beyond-bios-memory-protection-in-
>> uefi-bios/content/memory-protection-in-SMM.html
>>>
>>> I'm having trouble associating the protection features implemented in
>>> edk2 with the various bits in the WSMT (per
>>> "MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h").
>>>
>>> For example, it seems like the bits a platform sets in the WSMT *might*
>>> depend on "PcdCpuSmmRestrictedMemoryAccess".
>>>
>>> Can someone clarify these please?
>>>
>>>
>>> FWIW, in the edk2-platforms tree, the
>>> "Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c" source
>>> file sets EFI_WSMT_PROTECTION_FLAGS_FIXED_COMM_BUFFERS and
>>>
>> EFI_WSMT_PROTECTION_FLAGS_COMM_BUFFER_NESTED_PTR_PROTECTION.
>> It does not
>>> set EFI_WSMT_PROTECTION_FLAGS_SYSTEM_RESOURCE_PROTECTION.
>>>
>>> Is this bitmask (from Vlv2TbltDevicePkg) the general pattern that other
>>> edk2 platforms with SMM support should expose too, as a starting point?
>>
>> I have now read another whitepaper from you:
>>
>>   A Tour Beyond BIOS
>>   Secure SMM Communication in the EFI Developer Kit II
>>
>> It's a great whitepaper, it answers all my questions. Based on it, I
>> think that OVMF should enable all three bits in the WSMT:
>>
>> (1) BIT0 -- FIXED_COMM_BUFFERS:
>>
>> (1.1) OVMF uses the standard edk2 SMM infrastructure. Drivers in that
>> infrastructure verify that the communication buffers that they receive
>> are in permitted regions (no overlap with SMRAM, and resident in
>> permitted memory types), via SmmIsBufferOutsideSmmValid().
>>
>> (1.2) OVMF contains only one custom SMI handler (which is for CPU
>> hotplug). This handler does use normal RAM for a bit of communication
>> with hot-added CPUs, but the RAM used for this purpose is allocated as
>> reserved memory, and before End-of-Dxe.
>>
>> Furthermore, TOC-TOU is actively considered and mitigated, as the
>> "communication area" is a single byte (a boolean flag), and the design
>> considers the OS actively attacking that byte.
>>
>> (2) BIT1 -- COMM_BUFFER_NESTED_PTR_PROTECTION:
>>
>> OVMF uses the standard edk2 SMM infra, which validates the targets of
>> nested pointers.
>>
>> The custom SMI handler (for CPU hotplug) does not process pointers that
>> arrive from an untrusted context.
>>
>> (3) BIT2 -- SYSTEM_RESOURCE_PROTECTION:
>>
>> This bit is defined somewhat unclearly, but OVMF does lock both TSEG,
>> and the QEMU-specific "SMRAM at default SMBASE", at SmmReadyToLock. See
>> e.g. commit 9108fc17b09c ("OvmfPkg/SmmAccess: close and lock SMRAM at
>> default SMBASE", 2020-02-05).
>>
>> So I think OVMF should install the WSMT, and set all three bits.
>>
>>
>> However, there's a catch, for (1) FIXED_COMM_BUFFERS:
>>
>> (1.3) While OVMF produces a Memory Type Information HOB, it is not
>> *adaptive*.
>>
>> I'm not sure what the whitepaper suggests:
>>
>> (1.3.1) that the HOB exist (because then released / unused BINs will
>> never become usable memory in the final memory map),
>>
>> (1.3.2) or that BINs should actually accommodate all the allocations (so
>> that no runtime memory is reallocated *ever*).
>>
>> Because, in the non-adaptive approach (1.3.1), what happens if there is
>> a RuntimeData reallocation after End-of-Dxe, but even the *previous*
>> allocation (that's now getting released) used to live outside of a BIN?
>>
>> Unfortunately, enabling the adaptive Memory Type Information (1.3.2) for
>> OVMF is challenging, due to
>> <https://bugzilla.tianocore.org/show_bug.cgi?id=1086>. OVMF can be
>> booted with 128 MiB RAM and 1 TiB RAM just the same, and so a platform
>> PEIM cannot tell, in advance, what is a valid Memory Type Information
>> variable that the DXE core will accept, vs. an invalid Memory Type
>> Information variable that the DXE core will reject (and hang upon, due
>> to BIN priming failure).
>>
>> Thanks
>> Laszlo
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: WSMT bits
  2020-03-11 10:23     ` Laszlo Ersek
@ 2020-03-11 12:00       ` Yao, Jiewen
  2020-03-11 13:02         ` Laszlo Ersek
  0 siblings, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2020-03-11 12:00 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-groups-io, Ni, Ray

Great question on ACPIReclaim!
Yes, it is the confusing part.

1) Per ACPI specification, ACPI reclaim is the memory to hold the ACPI table.
It *may* be reclaimed by OS as OS memory, after OS copied the ACPI table to its own local space.
As such, we need make sure it is treated as *OS usable* memory. So we do not allow SMM driver to touch it.

2) Per our experience, ACPI reclaim *may* be treated as reserved memory by some OS.
OS just does not use it.
As such, we need group them in the BIN to not leave too many holes for OS. So we need track it in MemoryTypeInfo.


You may refer to https://github.com/tianocore/edk2-platforms/blob/master/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c
Below is our platform template.
GLOBAL_REMOVE_IF_UNREFERENCED EFI_MEMORY_TYPE_INFORMATION mDefaultMemoryTypeInformation[] = {
  { EfiACPIReclaimMemory,   FixedPcdGet32 (PcdPlatformEfiAcpiReclaimMemorySize) },  // ASL
  { EfiACPIMemoryNVS,       FixedPcdGet32 (PcdPlatformEfiAcpiNvsMemorySize) },      // ACPI NVS (including S3 related)
  { EfiReservedMemoryType,  FixedPcdGet32 (PcdPlatformEfiReservedMemorySize) },     // BIOS Reserved (including S3 related)
  { EfiRuntimeServicesData, FixedPcdGet32 (PcdPlatformEfiRtDataMemorySize) },       // Runtime Service Data
  { EfiRuntimeServicesCode, FixedPcdGet32 (PcdPlatformEfiRtCodeMemorySize) },       // Runtime Service Code
  { EfiMaxMemoryType, 0 }
};

Thank you
Yao Jiewen

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, March 11, 2020 6:23 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: WSMT bits
> 
> On 03/11/20 03:01, Yao, Jiewen wrote:
> > Thanks Laszlo. I am glad that you like the whitepaper.
> >
> > Right. The platform should assert the WSMT bit, because the platform need
> make sure that all SMI handlers do right thing for SMM communication buffer.
> > Using PcdCpuSmmRestrictedMemoryAccess is a good way to ensure any
> violation can be caught. It is highly recommend, unless it conflicts with some
> other features such as heap guard debug, or server memory hotplug.
> 
> Thanks for your answer! And yes, PcdCpuSmmRestrictedMemoryAccess is TRUE
> in OVMF. (Inheriting the DEC default.)
> 
> >
> > I recommend we keep MemoryTypeInfo as a separate topic for
> https://bugzilla.tianocore.org/show_bug.cgi?id=1086
> > I still think memory type information should be used - 1.3.2.
> 
> Right. In the end, I posted an OvmfPkg patch series yesterday, for
> covering point (1.3.2) -- adaptive MemoryTypeInfo:
> 
> [edk2-devel] [PATCH 0/5]
> OvmfPkg: improve SMM comms security with adaptive MemoryTypeInformation
> 
> http://mid.mail-archive.com/20200310222739.26717-1-lersek@redhat.com
> https://edk2.groups.io/g/devel/message/55726
> 
> I decided that TianoCore#1086 should not be considered a blocker for
> this feature. My reasoning is captured here:
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=386#c15
> 
> > I just do not understand well enough on your comment - " OVMF can be
> booted with 128 MiB RAM and 1 TiB RAM just the same, and so a platform PEIM
> cannot tell, in advance, what is a valid Memory Type Information variable that
> the DXE core will accept ..."
> > You are right that OVMF can boot with 128MB or 1TB. But that is all DRAM,
> right?
> > BIN size should be same, because BIN only includes Runtime/ACPI/Reserved
> memory. That is no reason to make them different between 128MB and 1TB.
> > Or do I misunderstand something? Would you please educate me why they are
> different?
> 
> This is a great point, and I remember that both of your white papers
> ("Memory Map and Practices in UEFI BIOS", and "Secure SMM
> Communication") recommend that only Runtime/ACPI/Reserved memory be
> tracked by MemoryTypeInfo. That will indeed make the guest RAM size
> irrelevant.
> 
> For now, the series that I posted, sticks with the *pre-existent* OVMF
> practice that boot services data/code too are tracked by MemoryTypeInfo:
> 
>   { EfiACPIMemoryNVS,       0x004 },
>   { EfiACPIReclaimMemory,   0x008 },
>   { EfiReservedMemoryType,  0x004 },
>   { EfiRuntimeServicesData, 0x024 },
>   { EfiRuntimeServicesCode, 0x030 },
>   { EfiBootServicesCode,    0x180 },
>   { EfiBootServicesData,    0xF00 },
>   { EfiMaxMemoryType,       0x000 }
> 
> I didn't want to diverge from that practice at once, in the current
> patch series.
> 
> I'd like to adopt the recommendation from the white papers -- i.e., stop
> tracking EfiACPIReclaimMemory, EfiBootServicesCode, and
> EfiBootServicesData -- as a separate work item.
> 
> Actually, can you please clarify: the "Secure SMM Communication" does
> not recommend tracking EfiACPIReclaimMemory, but "Memory Map and
> Practices in UEFI BIOS" does. So what is the right thing to do, should
> we continue tracking EfiACPIReclaimMemory?
> 
> Anyway, let me thank you very much, as I didn't make the connection
> myself that Runtime/ACPI/Reserved are not expected to *scale* with the
> guest RAM size, even if the BS and Loader memory footprint of various
> UEFI applications / drivers could.
> 
> Cheers!
> Laszlo
> 
> >
> > Thank you
> > Yao Jiewen
> >
> >
> >> -----Original Message-----
> >> From: Laszlo Ersek <lersek@redhat.com>
> >> Sent: Tuesday, March 10, 2020 9:48 PM
> >> To: Yao, Jiewen <jiewen.yao@intel.com>
> >> Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Ni, Ray
> <ray.ni@intel.com>
> >> Subject: Re: WSMT bits
> >>
> >> Hi again Jiewen,
> >>
> >> On 03/10/20 10:36, Laszlo Ersek wrote:
> >>> Hi Jiewen,
> >>>
> >>> reading the following chapter:
> >>>
> >>>   https://edk2-docs.gitbooks.io/a-tour-beyond-bios-memory-protection-in-
> >> uefi-bios/content/memory-protection-in-SMM.html
> >>>
> >>> I'm having trouble associating the protection features implemented in
> >>> edk2 with the various bits in the WSMT (per
> >>>
> "MdePkg/Include/IndustryStandard/WindowsSmmSecurityMitigationTable.h").
> >>>
> >>> For example, it seems like the bits a platform sets in the WSMT *might*
> >>> depend on "PcdCpuSmmRestrictedMemoryAccess".
> >>>
> >>> Can someone clarify these please?
> >>>
> >>>
> >>> FWIW, in the edk2-platforms tree, the
> >>> "Platform/Intel/Vlv2TbltDevicePkg/AcpiPlatform/AcpiPlatform.c" source
> >>> file sets EFI_WSMT_PROTECTION_FLAGS_FIXED_COMM_BUFFERS and
> >>>
> >>
> EFI_WSMT_PROTECTION_FLAGS_COMM_BUFFER_NESTED_PTR_PROTECTION.
> >> It does not
> >>> set EFI_WSMT_PROTECTION_FLAGS_SYSTEM_RESOURCE_PROTECTION.
> >>>
> >>> Is this bitmask (from Vlv2TbltDevicePkg) the general pattern that other
> >>> edk2 platforms with SMM support should expose too, as a starting point?
> >>
> >> I have now read another whitepaper from you:
> >>
> >>   A Tour Beyond BIOS
> >>   Secure SMM Communication in the EFI Developer Kit II
> >>
> >> It's a great whitepaper, it answers all my questions. Based on it, I
> >> think that OVMF should enable all three bits in the WSMT:
> >>
> >> (1) BIT0 -- FIXED_COMM_BUFFERS:
> >>
> >> (1.1) OVMF uses the standard edk2 SMM infrastructure. Drivers in that
> >> infrastructure verify that the communication buffers that they receive
> >> are in permitted regions (no overlap with SMRAM, and resident in
> >> permitted memory types), via SmmIsBufferOutsideSmmValid().
> >>
> >> (1.2) OVMF contains only one custom SMI handler (which is for CPU
> >> hotplug). This handler does use normal RAM for a bit of communication
> >> with hot-added CPUs, but the RAM used for this purpose is allocated as
> >> reserved memory, and before End-of-Dxe.
> >>
> >> Furthermore, TOC-TOU is actively considered and mitigated, as the
> >> "communication area" is a single byte (a boolean flag), and the design
> >> considers the OS actively attacking that byte.
> >>
> >> (2) BIT1 -- COMM_BUFFER_NESTED_PTR_PROTECTION:
> >>
> >> OVMF uses the standard edk2 SMM infra, which validates the targets of
> >> nested pointers.
> >>
> >> The custom SMI handler (for CPU hotplug) does not process pointers that
> >> arrive from an untrusted context.
> >>
> >> (3) BIT2 -- SYSTEM_RESOURCE_PROTECTION:
> >>
> >> This bit is defined somewhat unclearly, but OVMF does lock both TSEG,
> >> and the QEMU-specific "SMRAM at default SMBASE", at SmmReadyToLock.
> See
> >> e.g. commit 9108fc17b09c ("OvmfPkg/SmmAccess: close and lock SMRAM at
> >> default SMBASE", 2020-02-05).
> >>
> >> So I think OVMF should install the WSMT, and set all three bits.
> >>
> >>
> >> However, there's a catch, for (1) FIXED_COMM_BUFFERS:
> >>
> >> (1.3) While OVMF produces a Memory Type Information HOB, it is not
> >> *adaptive*.
> >>
> >> I'm not sure what the whitepaper suggests:
> >>
> >> (1.3.1) that the HOB exist (because then released / unused BINs will
> >> never become usable memory in the final memory map),
> >>
> >> (1.3.2) or that BINs should actually accommodate all the allocations (so
> >> that no runtime memory is reallocated *ever*).
> >>
> >> Because, in the non-adaptive approach (1.3.1), what happens if there is
> >> a RuntimeData reallocation after End-of-Dxe, but even the *previous*
> >> allocation (that's now getting released) used to live outside of a BIN?
> >>
> >> Unfortunately, enabling the adaptive Memory Type Information (1.3.2) for
> >> OVMF is challenging, due to
> >> <https://bugzilla.tianocore.org/show_bug.cgi?id=1086>. OVMF can be
> >> booted with 128 MiB RAM and 1 TiB RAM just the same, and so a platform
> >> PEIM cannot tell, in advance, what is a valid Memory Type Information
> >> variable that the DXE core will accept, vs. an invalid Memory Type
> >> Information variable that the DXE core will reject (and hang upon, due
> >> to BIN priming failure).
> >>
> >> Thanks
> >> Laszlo
> >


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: WSMT bits
  2020-03-11 12:00       ` Yao, Jiewen
@ 2020-03-11 13:02         ` Laszlo Ersek
  0 siblings, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2020-03-11 13:02 UTC (permalink / raw)
  To: Yao, Jiewen; +Cc: edk2-devel-groups-io, Ni, Ray

On 03/11/20 13:00, Yao, Jiewen wrote:
> Great question on ACPIReclaim!
> Yes, it is the confusing part.
> 
> 1) Per ACPI specification, ACPI reclaim is the memory to hold the ACPI table.
> It *may* be reclaimed by OS as OS memory, after OS copied the ACPI table to its own local space.
> As such, we need make sure it is treated as *OS usable* memory. So we do not allow SMM driver to touch it.
> 
> 2) Per our experience, ACPI reclaim *may* be treated as reserved memory by some OS.
> OS just does not use it.
> As such, we need group them in the BIN to not leave too many holes for OS. So we need track it in MemoryTypeInfo.
> 
> 
> You may refer to https://github.com/tianocore/edk2-platforms/blob/master/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c
> Below is our platform template.
> GLOBAL_REMOVE_IF_UNREFERENCED EFI_MEMORY_TYPE_INFORMATION mDefaultMemoryTypeInformation[] = {
>   { EfiACPIReclaimMemory,   FixedPcdGet32 (PcdPlatformEfiAcpiReclaimMemorySize) },  // ASL
>   { EfiACPIMemoryNVS,       FixedPcdGet32 (PcdPlatformEfiAcpiNvsMemorySize) },      // ACPI NVS (including S3 related)
>   { EfiReservedMemoryType,  FixedPcdGet32 (PcdPlatformEfiReservedMemorySize) },     // BIOS Reserved (including S3 related)
>   { EfiRuntimeServicesData, FixedPcdGet32 (PcdPlatformEfiRtDataMemorySize) },       // Runtime Service Data
>   { EfiRuntimeServicesCode, FixedPcdGet32 (PcdPlatformEfiRtCodeMemorySize) },       // Runtime Service Code
>   { EfiMaxMemoryType, 0 }
> };

Very tricky indeed. Thanks for the explanation! Now I know what to do.

Cheers!
Laszlo


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-11 13:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-10  9:36 WSMT bits Laszlo Ersek
2020-03-10 13:48 ` Laszlo Ersek
2020-03-11  2:01   ` Yao, Jiewen
2020-03-11 10:23     ` Laszlo Ersek
2020-03-11 12:00       ` Yao, Jiewen
2020-03-11 13:02         ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox