| Message ID | 5de2d80398986b81b6cfcdd35436bba8bf62c0e7.1662459668.git.baskov@ispras.ru (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | x86_64: Improvements at compressed kernel stage | expand |
On Tue, 6 Sept 2022 at 12:42, Evgeniy Baskov <baskov@ispras.ru> wrote: > > Add EFI_MEMORY_ATTRIBUTE_PROTOCOL as preferred alternative to DXE > services for changing memory attributes in the EFISTUB. > > Use DXE services only as a fallback in case aforementioned protocol > is not supported by UEFI implementation. > > Move DXE services initialization code closer to the place they are used > to match EFI_MEMORY_ATTRIBUTE_PROTOCOL initialization code. > > Signed-off-by: Evgeniy Baskov <baskov@ispras.ru> > --- > drivers/firmware/efi/libstub/mem.c | 166 ++++++++++++++++++------ > drivers/firmware/efi/libstub/x86-stub.c | 17 --- > 2 files changed, 127 insertions(+), 56 deletions(-) > > diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c > index 89ebc8ad2c22..8c8782993b30 100644 > --- a/drivers/firmware/efi/libstub/mem.c > +++ b/drivers/firmware/efi/libstub/mem.c > @@ -5,6 +5,9 @@ > > #include "efistub.h" > > +const efi_dxe_services_table_t *efi_dxe_table; > +efi_memory_attribute_protocol_t *efi_mem_attrib_proto; > + > static inline bool mmap_has_headroom(unsigned long buff_size, > unsigned long map_size, > unsigned long desc_size) > @@ -131,50 +134,32 @@ void efi_free(unsigned long size, unsigned long addr) > efi_bs_call(free_pages, addr, nr_pages); > } > > -/** > - * efi_adjust_memory_range_protection() - change memory range protection attributes > - * @start: memory range start address > - * @size: memory range size > - * > - * Actual memory range for which memory attributes are modified is > - * the smallest ranged with start address and size aligned to EFI_PAGE_SIZE > - * that includes [start, start + size]. > - * > - * @return: status code > - */ > -efi_status_t efi_adjust_memory_range_protection(unsigned long start, > - unsigned long size, > - unsigned long attributes) > +static void retrive_dxe_table(void) retrieve > +{ > + efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); > + if (efi_dxe_table && > + efi_dxe_table->hdr.signature != EFI_DXE_SERVICES_TABLE_SIGNATURE) { > + efi_warn("Ignoring DXE services table: invalid signature\n"); > + efi_dxe_table = NULL; > + } > +} > + > +static efi_status_t adjust_mem_attrib_dxe(efi_physical_addr_t rounded_start, > + efi_physical_addr_t rounded_end, > + unsigned long attributes) > { > efi_status_t status; > efi_gcd_memory_space_desc_t desc; > - efi_physical_addr_t end, next; > - efi_physical_addr_t rounded_start, rounded_end; > + efi_physical_addr_t end, next, start; > efi_physical_addr_t unprotect_start, unprotect_size; > int has_system_memory = 0; > > - if (efi_dxe_table == NULL) > - return EFI_UNSUPPORTED; > + if (!efi_dxe_table) { > + retrive_dxe_table(); Same here > > - /* > - * This function should not be used to modify attributes > - * other than writable/executable. > - */ > - > - if ((attributes & ~(EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) > - return EFI_INVALID_PARAMETER; > - > - /* > - * Disallow simultaniously executable and writable memory > - * to inforce W^X policy if direct extraction code is enabled. > - */ > - > - if ((attributes & (EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0 && > - IS_ENABLED(CONFIG_EFI_STUB_EXTRACT_DIRECT)) > - return EFI_INVALID_PARAMETER; > - > - rounded_start = rounddown(start, EFI_PAGE_SIZE); > - rounded_end = roundup(start + size, EFI_PAGE_SIZE); > + if (!efi_dxe_table) > + return EFI_UNSUPPORTED; > + } > > /* > * Don't modify memory region attributes, they are > @@ -182,14 +167,15 @@ efi_status_t efi_adjust_memory_range_protection(unsigned long start, > * encounter firmware bugs. > */ > > - for (end = start + size; start < end; start = next) { > + > + for (start = rounded_start, end = rounded_end; start < end; start = next) { > > status = efi_dxe_call(get_memory_space_descriptor, > start, &desc); > > if (status != EFI_SUCCESS) { > efi_warn("Unable to get memory descriptor at %lx\n", > - start); > + (unsigned long)start); > return status; > } > > @@ -231,3 +217,105 @@ efi_status_t efi_adjust_memory_range_protection(unsigned long start, > > return EFI_SUCCESS; > } > + > +static void retrive_memory_attributes_proto(void) and here > +{ > + efi_status_t status; > + efi_guid_t guid = EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID; > + > + status = efi_bs_call(locate_protocol, &guid, NULL, > + (void **)&efi_mem_attrib_proto); > + if (status != EFI_SUCCESS) > + efi_mem_attrib_proto = NULL; > +} > + > +/** > + * efi_adjust_memory_range_protection() - change memory range protection attributes > + * @start: memory range start address > + * @size: memory range size > + * > + * Actual memory range for which memory attributes are modified is > + * the smallest ranged with start address and size aligned to EFI_PAGE_SIZE > + * that includes [start, start + size]. > + * > + * This function first attempts to use EFI_MEMORY_ATTRIBUTE_PROTOCOL, > + * that is a part of UEFI Specification since version 2.10. > + * If the protocol is unavailable it falls back to DXE services functions. > + * > + * @return: status code > + */ > +efi_status_t efi_adjust_memory_range_protection(unsigned long start, > + unsigned long size, > + unsigned long attributes) > +{ > + efi_status_t status; > + efi_physical_addr_t rounded_start, rounded_end; > + unsigned long attr_clear; > + > + /* > + * This function should not be used to modify attributes > + * other than writable/executable. > + */ > + > + if ((attributes & ~(EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) > + return EFI_INVALID_PARAMETER; > + > + /* > + * Disallow simultaniously executable and writable memory simultaneously > + * to inforce W^X policy if direct extraction code is enabled. enforce > + */ > + > + if ((attributes & (EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0 && > + IS_ENABLED(CONFIG_EFI_STUB_EXTRACT_DIRECT)) efi_adjust_memory_range_protection() is a generic routine, but here it depends on a x86-specific Kconfig symbol. Is that really necessary? > + return EFI_INVALID_PARAMETER; > + > + rounded_start = rounddown(start, EFI_PAGE_SIZE); > + rounded_end = roundup(start + size, EFI_PAGE_SIZE); > + > + if (!efi_mem_attrib_proto) { > + retrive_memory_attributes_proto(); retrieve > + > + /* Fall back to DXE services if unsupported */ > + if (!efi_mem_attrib_proto) { > + return adjust_mem_attrib_dxe(rounded_start, > + rounded_end, > + attributes); > + } > + } > + > + /* > + * Unlike DXE services functions, EFI_MEMORY_ATTRIBUTE_PROTOCOL > + * does not clear unset protection bit, so it needs to be cleared > + * explcitly > + */ > + > + attr_clear = ~attributes & > + (EFI_MEMORY_RO | EFI_MEMORY_XP | EFI_MEMORY_RP); > + > + status = efi_call_proto(efi_mem_attrib_proto, > + clear_memory_attributes, > + rounded_start, > + rounded_end - rounded_start, > + attr_clear); > + if (status != EFI_SUCCESS) { > + efi_warn("Failed to clear memory attributes at [%08lx,%08lx]: %lx", Need \n at the end here > + (unsigned long)rounded_start, > + (unsigned long)rounded_end, > + status); > + return status; > + } > + > + status = efi_call_proto(efi_mem_attrib_proto, > + set_memory_attributes, > + rounded_start, > + rounded_end - rounded_start, > + attributes); > + if (status != EFI_SUCCESS) { > + efi_warn("Failed to set memory attributes at [%08lx,%08lx]: %lx", and here > + (unsigned long)rounded_start, > + (unsigned long)rounded_end, > + status); > + } > + > + return status; > +} > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c > index 914106d547a6..dd1e1e663072 100644 > --- a/drivers/firmware/efi/libstub/x86-stub.c > +++ b/drivers/firmware/efi/libstub/x86-stub.c > @@ -22,7 +22,6 @@ > #define MAXMEM_X86_64_4LEVEL (1ull << 46) > > const efi_system_table_t *efi_system_table; > -const efi_dxe_services_table_t *efi_dxe_table; > extern u32 image_offset; > static efi_loaded_image_t *image = NULL; > > @@ -401,15 +400,6 @@ static void setup_sections_memory_protection(void *image_base, > unsigned long init_size) > { > #ifdef CONFIG_EFI_DXE_MEM_ATTRIBUTES > - efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); > - > - if (!efi_dxe_table || > - efi_dxe_table->hdr.signature != EFI_DXE_SERVICES_TABLE_SIGNATURE) { > - efi_warn("Unable to locate EFI DXE services table\n"); > - efi_dxe_table = NULL; > - return; > - } > - > extern char _head[], _ehead[]; > extern char _compressed[], _ecompressed[]; > extern char _text[], _etext[]; > @@ -791,13 +781,6 @@ unsigned long efi_main(efi_handle_t handle, > if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) > efi_exit(handle, EFI_INVALID_PARAMETER); > > - efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); > - if (efi_dxe_table && > - efi_dxe_table->hdr.signature != EFI_DXE_SERVICES_TABLE_SIGNATURE) { > - efi_warn("Ignoring DXE services table: invalid signature\n"); > - efi_dxe_table = NULL; > - } > - > #ifndef CONFIG_EFI_STUB_EXTRACT_DIRECT > /* > * If the kernel isn't already loaded at a suitable address, > -- > 2.35.1 >
On 2022-10-19 10:42, Ard Biesheuvel wrote: > On Tue, 6 Sept 2022 at 12:42, Evgeniy Baskov <baskov@ispras.ru> wrote: >> >> Add EFI_MEMORY_ATTRIBUTE_PROTOCOL as preferred alternative to DXE >> services for changing memory attributes in the EFISTUB. >> >> Use DXE services only as a fallback in case aforementioned protocol >> is not supported by UEFI implementation. >> >> Move DXE services initialization code closer to the place they are >> used >> to match EFI_MEMORY_ATTRIBUTE_PROTOCOL initialization code. >> >> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru> >> --- >> drivers/firmware/efi/libstub/mem.c | 166 >> ++++++++++++++++++------ >> drivers/firmware/efi/libstub/x86-stub.c | 17 --- >> 2 files changed, 127 insertions(+), 56 deletions(-) >> >> diff --git a/drivers/firmware/efi/libstub/mem.c >> b/drivers/firmware/efi/libstub/mem.c >> index 89ebc8ad2c22..8c8782993b30 100644 >> --- a/drivers/firmware/efi/libstub/mem.c >> +++ b/drivers/firmware/efi/libstub/mem.c >> @@ -5,6 +5,9 @@ >> >> #include "efistub.h" >> >> +const efi_dxe_services_table_t *efi_dxe_table; >> +efi_memory_attribute_protocol_t *efi_mem_attrib_proto; >> + >> static inline bool mmap_has_headroom(unsigned long buff_size, >> unsigned long map_size, >> unsigned long desc_size) >> @@ -131,50 +134,32 @@ void efi_free(unsigned long size, unsigned long >> addr) >> efi_bs_call(free_pages, addr, nr_pages); >> } >> >> -/** >> - * efi_adjust_memory_range_protection() - change memory range >> protection attributes >> - * @start: memory range start address >> - * @size: memory range size >> - * >> - * Actual memory range for which memory attributes are modified is >> - * the smallest ranged with start address and size aligned to >> EFI_PAGE_SIZE >> - * that includes [start, start + size]. >> - * >> - * @return: status code >> - */ >> -efi_status_t efi_adjust_memory_range_protection(unsigned long start, >> - unsigned long size, >> - unsigned long >> attributes) >> +static void retrive_dxe_table(void) > > retrieve > >> +{ >> + efi_dxe_table = >> get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); >> + if (efi_dxe_table && >> + efi_dxe_table->hdr.signature != >> EFI_DXE_SERVICES_TABLE_SIGNATURE) { >> + efi_warn("Ignoring DXE services table: invalid >> signature\n"); >> + efi_dxe_table = NULL; >> + } >> +} >> + >> +static efi_status_t adjust_mem_attrib_dxe(efi_physical_addr_t >> rounded_start, >> + efi_physical_addr_t >> rounded_end, >> + unsigned long attributes) >> { >> efi_status_t status; >> efi_gcd_memory_space_desc_t desc; >> - efi_physical_addr_t end, next; >> - efi_physical_addr_t rounded_start, rounded_end; >> + efi_physical_addr_t end, next, start; >> efi_physical_addr_t unprotect_start, unprotect_size; >> int has_system_memory = 0; >> >> - if (efi_dxe_table == NULL) >> - return EFI_UNSUPPORTED; >> + if (!efi_dxe_table) { >> + retrive_dxe_table(); > > Same here > >> >> - /* >> - * This function should not be used to modify attributes >> - * other than writable/executable. >> - */ >> - >> - if ((attributes & ~(EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) >> - return EFI_INVALID_PARAMETER; >> - >> - /* >> - * Disallow simultaniously executable and writable memory >> - * to inforce W^X policy if direct extraction code is enabled. >> - */ >> - >> - if ((attributes & (EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0 && >> - IS_ENABLED(CONFIG_EFI_STUB_EXTRACT_DIRECT)) >> - return EFI_INVALID_PARAMETER; >> - >> - rounded_start = rounddown(start, EFI_PAGE_SIZE); >> - rounded_end = roundup(start + size, EFI_PAGE_SIZE); >> + if (!efi_dxe_table) >> + return EFI_UNSUPPORTED; >> + } >> >> /* >> * Don't modify memory region attributes, they are >> @@ -182,14 +167,15 @@ efi_status_t >> efi_adjust_memory_range_protection(unsigned long start, >> * encounter firmware bugs. >> */ >> >> - for (end = start + size; start < end; start = next) { >> + >> + for (start = rounded_start, end = rounded_end; start < end; >> start = next) { >> >> status = efi_dxe_call(get_memory_space_descriptor, >> start, &desc); >> >> if (status != EFI_SUCCESS) { >> efi_warn("Unable to get memory descriptor at >> %lx\n", >> - start); >> + (unsigned long)start); >> return status; >> } >> >> @@ -231,3 +217,105 @@ efi_status_t >> efi_adjust_memory_range_protection(unsigned long start, >> >> return EFI_SUCCESS; >> } >> + >> +static void retrive_memory_attributes_proto(void) > > and here > >> +{ >> + efi_status_t status; >> + efi_guid_t guid = EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID; >> + >> + status = efi_bs_call(locate_protocol, &guid, NULL, >> + (void **)&efi_mem_attrib_proto); >> + if (status != EFI_SUCCESS) >> + efi_mem_attrib_proto = NULL; >> +} >> + >> +/** >> + * efi_adjust_memory_range_protection() - change memory range >> protection attributes >> + * @start: memory range start address >> + * @size: memory range size >> + * >> + * Actual memory range for which memory attributes are modified is >> + * the smallest ranged with start address and size aligned to >> EFI_PAGE_SIZE >> + * that includes [start, start + size]. >> + * >> + * This function first attempts to use EFI_MEMORY_ATTRIBUTE_PROTOCOL, >> + * that is a part of UEFI Specification since version 2.10. >> + * If the protocol is unavailable it falls back to DXE services >> functions. >> + * >> + * @return: status code >> + */ >> +efi_status_t efi_adjust_memory_range_protection(unsigned long start, >> + unsigned long size, >> + unsigned long >> attributes) >> +{ >> + efi_status_t status; >> + efi_physical_addr_t rounded_start, rounded_end; >> + unsigned long attr_clear; >> + >> + /* >> + * This function should not be used to modify attributes >> + * other than writable/executable. >> + */ >> + >> + if ((attributes & ~(EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) >> + return EFI_INVALID_PARAMETER; >> + >> + /* >> + * Disallow simultaniously executable and writable memory > > simultaneously > >> + * to inforce W^X policy if direct extraction code is enabled. > > enforce > >> + */ >> + >> + if ((attributes & (EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0 && >> + IS_ENABLED(CONFIG_EFI_STUB_EXTRACT_DIRECT)) > > efi_adjust_memory_range_protection() is a generic routine, but here it > depends on a x86-specific Kconfig symbol. Is that really necessary? Since, I will make direct extraction unconditional, I will remove this as well, since it will be always true. > >> + return EFI_INVALID_PARAMETER; >> + >> + rounded_start = rounddown(start, EFI_PAGE_SIZE); >> + rounded_end = roundup(start + size, EFI_PAGE_SIZE); >> + >> + if (!efi_mem_attrib_proto) { >> + retrive_memory_attributes_proto(); > > retrieve > >> + >> + /* Fall back to DXE services if unsupported */ >> + if (!efi_mem_attrib_proto) { >> + return adjust_mem_attrib_dxe(rounded_start, >> + rounded_end, >> + attributes); >> + } >> + } >> + >> + /* >> + * Unlike DXE services functions, >> EFI_MEMORY_ATTRIBUTE_PROTOCOL >> + * does not clear unset protection bit, so it needs to be >> cleared >> + * explcitly >> + */ >> + >> + attr_clear = ~attributes & >> + (EFI_MEMORY_RO | EFI_MEMORY_XP | EFI_MEMORY_RP); >> + >> + status = efi_call_proto(efi_mem_attrib_proto, >> + clear_memory_attributes, >> + rounded_start, >> + rounded_end - rounded_start, >> + attr_clear); >> + if (status != EFI_SUCCESS) { >> + efi_warn("Failed to clear memory attributes at >> [%08lx,%08lx]: %lx", > > Need \n at the end here > >> + (unsigned long)rounded_start, >> + (unsigned long)rounded_end, >> + status); >> + return status; >> + } >> + >> + status = efi_call_proto(efi_mem_attrib_proto, >> + set_memory_attributes, >> + rounded_start, >> + rounded_end - rounded_start, >> + attributes); >> + if (status != EFI_SUCCESS) { >> + efi_warn("Failed to set memory attributes at >> [%08lx,%08lx]: %lx", > > and here > >> + (unsigned long)rounded_start, >> + (unsigned long)rounded_end, >> + status); >> + } >> + >> + return status; >> +} >> diff --git a/drivers/firmware/efi/libstub/x86-stub.c >> b/drivers/firmware/efi/libstub/x86-stub.c >> index 914106d547a6..dd1e1e663072 100644 >> --- a/drivers/firmware/efi/libstub/x86-stub.c >> +++ b/drivers/firmware/efi/libstub/x86-stub.c >> @@ -22,7 +22,6 @@ >> #define MAXMEM_X86_64_4LEVEL (1ull << 46) >> >> const efi_system_table_t *efi_system_table; >> -const efi_dxe_services_table_t *efi_dxe_table; >> extern u32 image_offset; >> static efi_loaded_image_t *image = NULL; >> >> @@ -401,15 +400,6 @@ static void setup_sections_memory_protection(void >> *image_base, >> unsigned long init_size) >> { >> #ifdef CONFIG_EFI_DXE_MEM_ATTRIBUTES >> - efi_dxe_table = >> get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); >> - >> - if (!efi_dxe_table || >> - efi_dxe_table->hdr.signature != >> EFI_DXE_SERVICES_TABLE_SIGNATURE) { >> - efi_warn("Unable to locate EFI DXE services table\n"); >> - efi_dxe_table = NULL; >> - return; >> - } >> - >> extern char _head[], _ehead[]; >> extern char _compressed[], _ecompressed[]; >> extern char _text[], _etext[]; >> @@ -791,13 +781,6 @@ unsigned long efi_main(efi_handle_t handle, >> if (efi_system_table->hdr.signature != >> EFI_SYSTEM_TABLE_SIGNATURE) >> efi_exit(handle, EFI_INVALID_PARAMETER); >> >> - efi_dxe_table = >> get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); >> - if (efi_dxe_table && >> - efi_dxe_table->hdr.signature != >> EFI_DXE_SERVICES_TABLE_SIGNATURE) { >> - efi_warn("Ignoring DXE services table: invalid >> signature\n"); >> - efi_dxe_table = NULL; >> - } >> - >> #ifndef CONFIG_EFI_STUB_EXTRACT_DIRECT >> /* >> * If the kernel isn't already loaded at a suitable address, >> -- >> 2.35.1 >> Thanks, for nitpicks, I will fix all the typos. \n's are fixed by Peter's patch, so I will rely on it.
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c index 89ebc8ad2c22..8c8782993b30 100644 --- a/drivers/firmware/efi/libstub/mem.c +++ b/drivers/firmware/efi/libstub/mem.c @@ -5,6 +5,9 @@ #include "efistub.h" +const efi_dxe_services_table_t *efi_dxe_table; +efi_memory_attribute_protocol_t *efi_mem_attrib_proto; + static inline bool mmap_has_headroom(unsigned long buff_size, unsigned long map_size, unsigned long desc_size) @@ -131,50 +134,32 @@ void efi_free(unsigned long size, unsigned long addr) efi_bs_call(free_pages, addr, nr_pages); } -/** - * efi_adjust_memory_range_protection() - change memory range protection attributes - * @start: memory range start address - * @size: memory range size - * - * Actual memory range for which memory attributes are modified is - * the smallest ranged with start address and size aligned to EFI_PAGE_SIZE - * that includes [start, start + size]. - * - * @return: status code - */ -efi_status_t efi_adjust_memory_range_protection(unsigned long start, - unsigned long size, - unsigned long attributes) +static void retrive_dxe_table(void) +{ + efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); + if (efi_dxe_table && + efi_dxe_table->hdr.signature != EFI_DXE_SERVICES_TABLE_SIGNATURE) { + efi_warn("Ignoring DXE services table: invalid signature\n"); + efi_dxe_table = NULL; + } +} + +static efi_status_t adjust_mem_attrib_dxe(efi_physical_addr_t rounded_start, + efi_physical_addr_t rounded_end, + unsigned long attributes) { efi_status_t status; efi_gcd_memory_space_desc_t desc; - efi_physical_addr_t end, next; - efi_physical_addr_t rounded_start, rounded_end; + efi_physical_addr_t end, next, start; efi_physical_addr_t unprotect_start, unprotect_size; int has_system_memory = 0; - if (efi_dxe_table == NULL) - return EFI_UNSUPPORTED; + if (!efi_dxe_table) { + retrive_dxe_table(); - /* - * This function should not be used to modify attributes - * other than writable/executable. - */ - - if ((attributes & ~(EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) - return EFI_INVALID_PARAMETER; - - /* - * Disallow simultaniously executable and writable memory - * to inforce W^X policy if direct extraction code is enabled. - */ - - if ((attributes & (EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0 && - IS_ENABLED(CONFIG_EFI_STUB_EXTRACT_DIRECT)) - return EFI_INVALID_PARAMETER; - - rounded_start = rounddown(start, EFI_PAGE_SIZE); - rounded_end = roundup(start + size, EFI_PAGE_SIZE); + if (!efi_dxe_table) + return EFI_UNSUPPORTED; + } /* * Don't modify memory region attributes, they are @@ -182,14 +167,15 @@ efi_status_t efi_adjust_memory_range_protection(unsigned long start, * encounter firmware bugs. */ - for (end = start + size; start < end; start = next) { + + for (start = rounded_start, end = rounded_end; start < end; start = next) { status = efi_dxe_call(get_memory_space_descriptor, start, &desc); if (status != EFI_SUCCESS) { efi_warn("Unable to get memory descriptor at %lx\n", - start); + (unsigned long)start); return status; } @@ -231,3 +217,105 @@ efi_status_t efi_adjust_memory_range_protection(unsigned long start, return EFI_SUCCESS; } + +static void retrive_memory_attributes_proto(void) +{ + efi_status_t status; + efi_guid_t guid = EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID; + + status = efi_bs_call(locate_protocol, &guid, NULL, + (void **)&efi_mem_attrib_proto); + if (status != EFI_SUCCESS) + efi_mem_attrib_proto = NULL; +} + +/** + * efi_adjust_memory_range_protection() - change memory range protection attributes + * @start: memory range start address + * @size: memory range size + * + * Actual memory range for which memory attributes are modified is + * the smallest ranged with start address and size aligned to EFI_PAGE_SIZE + * that includes [start, start + size]. + * + * This function first attempts to use EFI_MEMORY_ATTRIBUTE_PROTOCOL, + * that is a part of UEFI Specification since version 2.10. + * If the protocol is unavailable it falls back to DXE services functions. + * + * @return: status code + */ +efi_status_t efi_adjust_memory_range_protection(unsigned long start, + unsigned long size, + unsigned long attributes) +{ + efi_status_t status; + efi_physical_addr_t rounded_start, rounded_end; + unsigned long attr_clear; + + /* + * This function should not be used to modify attributes + * other than writable/executable. + */ + + if ((attributes & ~(EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) + return EFI_INVALID_PARAMETER; + + /* + * Disallow simultaniously executable and writable memory + * to inforce W^X policy if direct extraction code is enabled. + */ + + if ((attributes & (EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0 && + IS_ENABLED(CONFIG_EFI_STUB_EXTRACT_DIRECT)) + return EFI_INVALID_PARAMETER; + + rounded_start = rounddown(start, EFI_PAGE_SIZE); + rounded_end = roundup(start + size, EFI_PAGE_SIZE); + + if (!efi_mem_attrib_proto) { + retrive_memory_attributes_proto(); + + /* Fall back to DXE services if unsupported */ + if (!efi_mem_attrib_proto) { + return adjust_mem_attrib_dxe(rounded_start, + rounded_end, + attributes); + } + } + + /* + * Unlike DXE services functions, EFI_MEMORY_ATTRIBUTE_PROTOCOL + * does not clear unset protection bit, so it needs to be cleared + * explcitly + */ + + attr_clear = ~attributes & + (EFI_MEMORY_RO | EFI_MEMORY_XP | EFI_MEMORY_RP); + + status = efi_call_proto(efi_mem_attrib_proto, + clear_memory_attributes, + rounded_start, + rounded_end - rounded_start, + attr_clear); + if (status != EFI_SUCCESS) { + efi_warn("Failed to clear memory attributes at [%08lx,%08lx]: %lx", + (unsigned long)rounded_start, + (unsigned long)rounded_end, + status); + return status; + } + + status = efi_call_proto(efi_mem_attrib_proto, + set_memory_attributes, + rounded_start, + rounded_end - rounded_start, + attributes); + if (status != EFI_SUCCESS) { + efi_warn("Failed to set memory attributes at [%08lx,%08lx]: %lx", + (unsigned long)rounded_start, + (unsigned long)rounded_end, + status); + } + + return status; +} diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index 914106d547a6..dd1e1e663072 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -22,7 +22,6 @@ #define MAXMEM_X86_64_4LEVEL (1ull << 46) const efi_system_table_t *efi_system_table; -const efi_dxe_services_table_t *efi_dxe_table; extern u32 image_offset; static efi_loaded_image_t *image = NULL; @@ -401,15 +400,6 @@ static void setup_sections_memory_protection(void *image_base, unsigned long init_size) { #ifdef CONFIG_EFI_DXE_MEM_ATTRIBUTES - efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); - - if (!efi_dxe_table || - efi_dxe_table->hdr.signature != EFI_DXE_SERVICES_TABLE_SIGNATURE) { - efi_warn("Unable to locate EFI DXE services table\n"); - efi_dxe_table = NULL; - return; - } - extern char _head[], _ehead[]; extern char _compressed[], _ecompressed[]; extern char _text[], _etext[]; @@ -791,13 +781,6 @@ unsigned long efi_main(efi_handle_t handle, if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) efi_exit(handle, EFI_INVALID_PARAMETER); - efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); - if (efi_dxe_table && - efi_dxe_table->hdr.signature != EFI_DXE_SERVICES_TABLE_SIGNATURE) { - efi_warn("Ignoring DXE services table: invalid signature\n"); - efi_dxe_table = NULL; - } - #ifndef CONFIG_EFI_STUB_EXTRACT_DIRECT /* * If the kernel isn't already loaded at a suitable address,
Add EFI_MEMORY_ATTRIBUTE_PROTOCOL as preferred alternative to DXE services for changing memory attributes in the EFISTUB. Use DXE services only as a fallback in case aforementioned protocol is not supported by UEFI implementation. Move DXE services initialization code closer to the place they are used to match EFI_MEMORY_ATTRIBUTE_PROTOCOL initialization code. Signed-off-by: Evgeniy Baskov <baskov@ispras.ru> --- drivers/firmware/efi/libstub/mem.c | 166 ++++++++++++++++++------ drivers/firmware/efi/libstub/x86-stub.c | 17 --- 2 files changed, 127 insertions(+), 56 deletions(-)