From 41fec869fb3b25fd5bb5b454ab1bf39660ce314d Mon Sep 17 00:00:00 2001 From: Patrick Rudolph Date: Wed, 6 May 2020 10:55:12 +0200 Subject: cpu/x86/smm: Add helper functions to verify SMM access * Add a function to check if a region overlaps with SMM. * Add a function to check if a pointer points to SMM. * Document functions in Documentation/security/smm To be used to verify data accesses in SMM. Change-Id: Ia525d2bc685377f50ecf3bdcf337a4c885488213 Signed-off-by: Patrick Rudolph Signed-off-by: Christian Walter Reviewed-on: https://review.coreboot.org/c/coreboot/+/41084 Reviewed-by: Angel Pons Reviewed-by: Patrick Georgi Tested-by: build bot (Jenkins) --- Documentation/security/index.md | 4 ++++ Documentation/security/smm.md | 29 +++++++++++++++++++++++++++++ src/cpu/x86/smm/smihandler.c | 10 +++++++++- src/cpu/x86/smm/smm_module_handler.c | 9 +++++++++ src/include/cpu/x86/smm.h | 12 ++++++++++++ 5 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 Documentation/security/smm.md diff --git a/Documentation/security/index.md b/Documentation/security/index.md index d5d4e2b93e..c9cb4a77de 100644 --- a/Documentation/security/index.md +++ b/Documentation/security/index.md @@ -13,3 +13,7 @@ This section describes documentation about the security architecture of coreboot - [Intel TXT in general](intel/txt.md) - [Intel TXT Initial Boot Block](intel/txt_ibb.md) - [Intel Authenticated Code Modules](intel/acm.md) + +## SMM + +- [System Management Mode](smm.md) diff --git a/Documentation/security/smm.md b/Documentation/security/smm.md new file mode 100644 index 0000000000..4e95427b86 --- /dev/null +++ b/Documentation/security/smm.md @@ -0,0 +1,29 @@ +# x86 System Managment Mode + +## Introduction + +The code running in System Management Mode (SMM) provides runtime services +to applications running in [ring0]. It has a higher privilege level than +[ring0] and resides in the SMRAM region which cannot be accessed from [ring0]. + +SMM can be entered by issuing System Managment Interrupts (SMIs). + +## Secure data exchange + +In order to not leak SMM internals or accidentally overwrite parts of SMM, +[ring0] provided data (pointers, offsets, sizes, ...) must be checked before +using them in SMM. + +There exist two methods to verify data: + +```C +/* Returns true if the region overlaps with the SMM */ +bool smm_region_overlaps_handler(struct region *r); +``` + +```C +/* Returns true if the memory pointed to overlaps with SMM reserved memory. */ +static inline bool smm_points_to_smram(const void *ptr, const size_t len); +``` + +[ring0]: https://en.wikipedia.org/wiki/Protection_ring diff --git a/src/cpu/x86/smm/smihandler.c b/src/cpu/x86/smm/smihandler.c index bb4689e24e..8fd95bb563 100644 --- a/src/cpu/x86/smm/smihandler.c +++ b/src/cpu/x86/smm/smihandler.c @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -119,6 +120,13 @@ static inline void *smm_save_state(uintptr_t base, int arch_offset, int node) return (void *)base; } +bool smm_region_overlaps_handler(const struct region *r) +{ + const struct region r_smm = {SMM_BASE, SMM_DEFAULT_SIZE}; + + return region_overlap(&r_smm, r); +} + /** * @brief Interrupt handler for SMI# * @@ -129,7 +137,7 @@ void smi_handler(u32 smm_revision) { unsigned int node; smm_state_save_area_t state_save; - u32 smm_base = 0xa0000; /* ASEG */ + u32 smm_base = SMM_BASE; /* ASEG */ /* Are we ok to execute the handler? */ if (!smi_obtain_lock()) { diff --git a/src/cpu/x86/smm/smm_module_handler.c b/src/cpu/x86/smm/smm_module_handler.c index 404b0ccdaa..37af199080 100644 --- a/src/cpu/x86/smm/smm_module_handler.c +++ b/src/cpu/x86/smm/smm_module_handler.c @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -103,6 +104,14 @@ void *smm_get_save_state(int cpu) return base; } +bool smm_region_overlaps_handler(const struct region *r) +{ + const struct region r_smm = {smm_runtime->smbase, smm_runtime->smm_size}; + const struct region r_aseg = {SMM_BASE, SMM_DEFAULT_SIZE}; + + return region_overlap(&r_smm, r) || region_overlap(&r_aseg, r); +} + asmlinkage void smm_handler_start(void *arg) { const struct smm_module_params *p; diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 75c19a5cd5..ca834743b5 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -4,6 +4,7 @@ #define CPU_X86_SMM_H #include +#include #include #define SMM_DEFAULT_BASE 0x30000 @@ -93,6 +94,17 @@ asmlinkage void smm_handler_start(void *params); * account CPUs which are configured to not save their state to RAM. */ void *smm_get_save_state(int cpu); +/* Returns true if the region overlaps with the SMM */ +bool smm_region_overlaps_handler(const struct region *r); + +/* Returns true if the memory pointed to overlaps with SMM reserved memory. */ +static inline bool smm_points_to_smram(const void *ptr, const size_t len) +{ + const struct region r = {(uintptr_t)ptr, len}; + + return smm_region_overlaps_handler(&r); +} + /* SMM Module Loading API */ /* The smm_loader_params structure provides direction to the SMM loader: -- cgit v1.2.3