* [PATCH 01/10] efi: use typed function pointers for runtime services table
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
@ 2017-01-02 10:24 ` Lukas Wunner
2017-01-02 10:24 ` [PATCH 02/10] efi: Constify GetVariable() / SetVariable() signatures Lukas Wunner
` (10 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Instead of using void pointers, and casting them to correctly typed
function pointers upon use, declare the runtime services pointers
as function pointers using their respective prototypes, for which
typedefs are already available.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
include/linux/efi.h | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 712a3aa..889d40d 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -509,24 +509,6 @@ struct efi_boot_memmap {
u64 query_variable_info;
} efi_runtime_services_64_t;
-typedef struct {
- efi_table_hdr_t hdr;
- void *get_time;
- void *set_time;
- void *get_wakeup_time;
- void *set_wakeup_time;
- void *set_virtual_address_map;
- void *convert_pointer;
- void *get_variable;
- void *get_next_variable;
- void *set_variable;
- void *get_next_high_mono_count;
- void *reset_system;
- void *update_capsule;
- void *query_capsule_caps;
- void *query_variable_info;
-} efi_runtime_services_t;
-
typedef efi_status_t efi_get_time_t (efi_time_t *tm, efi_time_cap_t *tc);
typedef efi_status_t efi_set_time_t (efi_time_t *tm);
typedef efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, efi_bool_t *pending,
@@ -561,6 +543,24 @@ typedef efi_status_t efi_query_variable_store_t(u32 attributes,
unsigned long size,
bool nonblocking);
+typedef struct {
+ efi_table_hdr_t hdr;
+ efi_get_time_t *get_time;
+ efi_set_time_t *set_time;
+ efi_get_wakeup_time_t *get_wakeup_time;
+ efi_set_wakeup_time_t *set_wakeup_time;
+ efi_set_virtual_address_map_t *set_virtual_address_map;
+ void *convert_pointer;
+ efi_get_variable_t *get_variable;
+ efi_get_next_variable_t *get_next_variable;
+ efi_set_variable_t *set_variable;
+ efi_get_next_high_mono_count_t *get_next_high_mono_count;
+ efi_reset_system_t *reset_system;
+ efi_update_capsule_t *update_capsule;
+ efi_query_capsule_caps_t *query_capsule_caps;
+ efi_query_variable_info_t *query_variable_info;
+} efi_runtime_services_t;
+
void efi_native_runtime_setup(void);
/*
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 02/10] efi: Constify GetVariable() / SetVariable() signatures
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2017-01-02 10:24 ` [PATCH 01/10] efi: use typed function pointers for runtime services table Lukas Wunner
@ 2017-01-02 10:24 ` Lukas Wunner
[not found] ` <bd030e11cd114fb1d568b21eddfc1fa75cd42e5e.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2017-01-02 10:24 ` [PATCH 05/10] efi: Constify InstallConfigurationTable() Lukas Wunner
` (9 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
Cc: Tony Luck, Fenghua Yu, Boris Ostrovsky, David Vrabel,
Juergen Gross
GetVariable() / SetVariable() is typically called with immutable
VendorGuid and VariableName arguments. SetVariable() is also often
called with an immutable Data argument. However declaring these const
in the caller results in a compiler warning since they're not marked
const in the function signatures. Rectify that, in keeping with the EFI
spec which marks these arguments "IN".
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Fenghua Yu <fenghua.yu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Boris Ostrovsky <boris.ostrovsky-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Cc: David Vrabel <david.vrabel-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
Cc: Juergen Gross <jgross-IBi9RG/b67k@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
arch/ia64/kernel/efi.c | 9 ++++-----
arch/x86/include/asm/xen/interface.h | 1 +
arch/x86/platform/efi/efi_64.c | 18 +++++++++---------
drivers/firmware/efi/runtime-wrappers.c | 15 ++++++++-------
drivers/firmware/google/gsmi.c | 10 +++++-----
drivers/xen/efi.c | 12 ++++++------
include/linux/efi.h | 10 ++++++----
include/xen/arm/interface.h | 1 +
include/xen/interface/platform.h | 11 +++++++++--
include/xen/xen-ops.h | 12 ++++++------
10 files changed, 55 insertions(+), 44 deletions(-)
diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index 1212956..f57d089 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -125,8 +125,8 @@
#define STUB_GET_VARIABLE(prefix, adjust_arg) \
static efi_status_t \
-prefix##_get_variable (efi_char16_t *name, efi_guid_t *vendor, u32 *attr, \
- unsigned long *data_size, void *data) \
+prefix##_get_variable (const efi_char16_t *name, const efi_guid_t *vendor, \
+ u32 *attr, unsigned long *data_size, void *data) \
{ \
struct ia64_fpreg fr[6]; \
u32 *aattr = NULL; \
@@ -161,9 +161,8 @@
#define STUB_SET_VARIABLE(prefix, adjust_arg) \
static efi_status_t \
-prefix##_set_variable (efi_char16_t *name, efi_guid_t *vendor, \
- u32 attr, unsigned long data_size, \
- void *data) \
+prefix##_set_variable (const efi_char16_t *name, const efi_guid_t *vendor, \
+ u32 attr, unsigned long data_size, const void *data) \
{ \
struct ia64_fpreg fr[6]; \
efi_status_t ret; \
diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
index 62ca03e..543716c 100644
--- a/arch/x86/include/asm/xen/interface.h
+++ b/arch/x86/include/asm/xen/interface.h
@@ -86,6 +86,7 @@
/* Guest handles for primitive C types. */
__DEFINE_GUEST_HANDLE(uchar, unsigned char);
__DEFINE_GUEST_HANDLE(uint, unsigned int);
+__DEFINE_GUEST_HANDLE(cvoid, const void);
DEFINE_GUEST_HANDLE(char);
DEFINE_GUEST_HANDLE(int);
DEFINE_GUEST_HANDLE(void);
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index ee966aa..2a7d423 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -632,13 +632,13 @@ static efi_status_t efi_thunk_set_time(efi_time_t *tm)
return status;
}
-static unsigned long efi_name_size(efi_char16_t *name)
+static unsigned long efi_name_size(const efi_char16_t *name)
{
return ucs2_strsize(name, EFI_VAR_NAME_LEN) + 1;
}
static efi_status_t
-efi_thunk_get_variable(efi_char16_t *name, efi_guid_t *vendor,
+efi_thunk_get_variable(const efi_char16_t *name, const efi_guid_t *vendor,
u32 *attr, unsigned long *data_size, void *data)
{
efi_status_t status;
@@ -646,8 +646,8 @@ static unsigned long efi_name_size(efi_char16_t *name)
u32 phys_data_size, phys_data;
phys_data_size = virt_to_phys_or_null(data_size);
- phys_vendor = virt_to_phys_or_null(vendor);
- phys_name = virt_to_phys_or_null_size(name, efi_name_size(name));
+ phys_vendor = virt_to_phys_or_null((void *)vendor);
+ phys_name = virt_to_phys_or_null_size((void *)name, efi_name_size(name));
phys_attr = virt_to_phys_or_null(attr);
phys_data = virt_to_phys_or_null_size(data, *data_size);
@@ -658,15 +658,15 @@ static unsigned long efi_name_size(efi_char16_t *name)
}
static efi_status_t
-efi_thunk_set_variable(efi_char16_t *name, efi_guid_t *vendor,
- u32 attr, unsigned long data_size, void *data)
+efi_thunk_set_variable(const efi_char16_t *name, const efi_guid_t *vendor,
+ u32 attr, unsigned long data_size, const void *data)
{
u32 phys_name, phys_vendor, phys_data;
efi_status_t status;
- phys_name = virt_to_phys_or_null_size(name, efi_name_size(name));
- phys_vendor = virt_to_phys_or_null(vendor);
- phys_data = virt_to_phys_or_null_size(data, data_size);
+ phys_name = virt_to_phys_or_null_size((void *)name, efi_name_size(name));
+ phys_vendor = virt_to_phys_or_null((void *)vendor);
+ phys_data = virt_to_phys_or_null_size((void *)data, data_size);
/* If data_size is > sizeof(u32) we've got problems */
status = efi_thunk(set_variable, phys_name, phys_vendor,
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index ae54870b..a98de14 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -136,8 +136,8 @@ static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
return status;
}
-static efi_status_t virt_efi_get_variable(efi_char16_t *name,
- efi_guid_t *vendor,
+static efi_status_t virt_efi_get_variable(const efi_char16_t *name,
+ const efi_guid_t *vendor,
u32 *attr,
unsigned long *data_size,
void *data)
@@ -165,11 +165,11 @@ static efi_status_t virt_efi_get_next_variable(unsigned long *name_size,
return status;
}
-static efi_status_t virt_efi_set_variable(efi_char16_t *name,
- efi_guid_t *vendor,
+static efi_status_t virt_efi_set_variable(const efi_char16_t *name,
+ const efi_guid_t *vendor,
u32 attr,
unsigned long data_size,
- void *data)
+ const void *data)
{
efi_status_t status;
@@ -182,9 +182,10 @@ static efi_status_t virt_efi_set_variable(efi_char16_t *name,
}
static efi_status_t
-virt_efi_set_variable_nonblocking(efi_char16_t *name, efi_guid_t *vendor,
+virt_efi_set_variable_nonblocking(const efi_char16_t *name,
+ const efi_guid_t *vendor,
u32 attr, unsigned long data_size,
- void *data)
+ const void *data)
{
efi_status_t status;
diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
index c463871..a1b3cd9 100644
--- a/drivers/firmware/google/gsmi.c
+++ b/drivers/firmware/google/gsmi.c
@@ -289,8 +289,8 @@ static int gsmi_exec(u8 func, u8 sub)
return rc;
}
-static efi_status_t gsmi_get_variable(efi_char16_t *name,
- efi_guid_t *vendor, u32 *attr,
+static efi_status_t gsmi_get_variable(const efi_char16_t *name,
+ const efi_guid_t *vendor, u32 *attr,
unsigned long *data_size,
void *data)
{
@@ -410,11 +410,11 @@ static efi_status_t gsmi_get_next_variable(unsigned long *name_size,
return ret;
}
-static efi_status_t gsmi_set_variable(efi_char16_t *name,
- efi_guid_t *vendor,
+static efi_status_t gsmi_set_variable(const efi_char16_t *name,
+ const efi_guid_t *vendor,
u32 attr,
unsigned long data_size,
- void *data)
+ const void *data)
{
struct gsmi_nvram_var_param param = {
.name_ptr = gsmi_dev.name_buf->address,
diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
index 22f71ff..248740a 100644
--- a/drivers/xen/efi.c
+++ b/drivers/xen/efi.c
@@ -117,9 +117,9 @@ efi_status_t xen_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
}
EXPORT_SYMBOL_GPL(xen_efi_set_wakeup_time);
-efi_status_t xen_efi_get_variable(efi_char16_t *name, efi_guid_t *vendor,
- u32 *attr, unsigned long *data_size,
- void *data)
+efi_status_t xen_efi_get_variable(const efi_char16_t *name,
+ const efi_guid_t *vendor, u32 *attr,
+ unsigned long *data_size, void *data)
{
struct xen_platform_op op = INIT_EFI_OP(get_variable);
@@ -165,9 +165,9 @@ efi_status_t xen_efi_get_next_variable(unsigned long *name_size,
}
EXPORT_SYMBOL_GPL(xen_efi_get_next_variable);
-efi_status_t xen_efi_set_variable(efi_char16_t *name, efi_guid_t *vendor,
- u32 attr, unsigned long data_size,
- void *data)
+efi_status_t xen_efi_set_variable(const efi_char16_t *name,
+ const efi_guid_t *vendor, u32 attr,
+ unsigned long data_size, const void *data)
{
struct xen_platform_op op = INIT_EFI_OP(set_variable);
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 889d40d..6ade102 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -514,13 +514,15 @@ struct efi_boot_memmap {
typedef efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, efi_bool_t *pending,
efi_time_t *tm);
typedef efi_status_t efi_set_wakeup_time_t (efi_bool_t enabled, efi_time_t *tm);
-typedef efi_status_t efi_get_variable_t (efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
+typedef efi_status_t efi_get_variable_t (const efi_char16_t *name,
+ const efi_guid_t *vendor, u32 *attr,
unsigned long *data_size, void *data);
typedef efi_status_t efi_get_next_variable_t (unsigned long *name_size, efi_char16_t *name,
efi_guid_t *vendor);
-typedef efi_status_t efi_set_variable_t (efi_char16_t *name, efi_guid_t *vendor,
- u32 attr, unsigned long data_size,
- void *data);
+typedef efi_status_t efi_set_variable_t (const efi_char16_t *name,
+ const efi_guid_t *vendor, u32 attr,
+ unsigned long data_size,
+ const void *data);
typedef efi_status_t efi_get_next_high_mono_count_t (u32 *count);
typedef void efi_reset_system_t (int reset_type, efi_status_t status,
unsigned long data_size, efi_char16_t *data);
diff --git a/include/xen/arm/interface.h b/include/xen/arm/interface.h
index 75d5968..daa2956 100644
--- a/include/xen/arm/interface.h
+++ b/include/xen/arm/interface.h
@@ -47,6 +47,7 @@
/* Guest handles for primitive C types. */
__DEFINE_GUEST_HANDLE(uchar, unsigned char);
__DEFINE_GUEST_HANDLE(uint, unsigned int);
+__DEFINE_GUEST_HANDLE(cvoid, const void);
DEFINE_GUEST_HANDLE(char);
DEFINE_GUEST_HANDLE(int);
DEFINE_GUEST_HANDLE(void);
diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h
index 732efb0..b7e249c 100644
--- a/include/xen/interface/platform.h
+++ b/include/xen/interface/platform.h
@@ -171,7 +171,7 @@ struct xenpf_efi_runtime_call {
#define XEN_EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002
#define XEN_EFI_VARIABLE_RUNTIME_ACCESS 0x00000004
struct {
- GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */
+ GUEST_HANDLE(cvoid) name; /* UCS-2/UTF-16 string */
xen_ulong_t size;
GUEST_HANDLE(void) data;
struct xenpf_efi_guid {
@@ -180,7 +180,14 @@ struct xenpf_efi_runtime_call {
uint16_t data3;
uint8_t data4[8];
} vendor_guid;
- } get_variable, set_variable;
+ } get_variable;
+
+ struct {
+ GUEST_HANDLE(cvoid) name; /* UCS-2/UTF-16 string */
+ xen_ulong_t size;
+ GUEST_HANDLE(cvoid) data;
+ struct xenpf_efi_guid vendor_guid;
+ } set_variable;
struct {
xen_ulong_t size;
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index b5486e6..b118f56 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -103,14 +103,14 @@ int xen_xlate_map_ballooned_pages(xen_pfn_t **pfns, void **vaddr,
efi_status_t xen_efi_get_wakeup_time(efi_bool_t *enabled, efi_bool_t *pending,
efi_time_t *tm);
efi_status_t xen_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm);
-efi_status_t xen_efi_get_variable(efi_char16_t *name, efi_guid_t *vendor,
- u32 *attr, unsigned long *data_size,
- void *data);
+efi_status_t xen_efi_get_variable(const efi_char16_t *name,
+ const efi_guid_t *vendor, u32 *attr,
+ unsigned long *data_size, void *data);
efi_status_t xen_efi_get_next_variable(unsigned long *name_size,
efi_char16_t *name, efi_guid_t *vendor);
-efi_status_t xen_efi_set_variable(efi_char16_t *name, efi_guid_t *vendor,
- u32 attr, unsigned long data_size,
- void *data);
+efi_status_t xen_efi_set_variable(const efi_char16_t *name,
+ const efi_guid_t *vendor, u32 attr,
+ unsigned long data_size, const void *data);
efi_status_t xen_efi_query_variable_info(u32 attr, u64 *storage_space,
u64 *remaining_space,
u64 *max_variable_size);
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 05/10] efi: Constify InstallConfigurationTable()
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2017-01-02 10:24 ` [PATCH 01/10] efi: use typed function pointers for runtime services table Lukas Wunner
2017-01-02 10:24 ` [PATCH 02/10] efi: Constify GetVariable() / SetVariable() signatures Lukas Wunner
@ 2017-01-02 10:24 ` Lukas Wunner
2017-01-02 10:24 ` [PATCH 07/10] efi: Constify EFI_RNG_PROTOCOL.GetRNG() Lukas Wunner
` (8 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
GUIDs passed to InstallConfigurationTable() are not modified, so declare
them const. This allows the compiler to place them in the rodata
section instead of generating them on the stack at runtime. Pass GUIDs
as literals rather than assigning them to temporary variables to save a
bit on code. Constification of the GUIDs is in compliance with the EFI
spec which marks them "IN" for this boot service.
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
drivers/firmware/efi/libstub/arm32-stub.c | 2 +-
drivers/firmware/efi/libstub/random.c | 5 ++---
include/linux/efi.h | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
index e1f0b28..1be40b6 100644
--- a/drivers/firmware/efi/libstub/arm32-stub.c
+++ b/drivers/firmware/efi/libstub/arm32-stub.c
@@ -26,7 +26,7 @@ efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)
return EFI_SUCCESS;
}
-static efi_guid_t screen_info_guid = LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID;
+static const efi_guid_t screen_info_guid = LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID;
struct screen_info *alloc_screen_info(efi_system_table_t *sys_table_arg)
{
diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index ffb1130..c29a722 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -149,7 +149,6 @@ efi_status_t efi_random_alloc(efi_system_table_t *sys_table_arg,
efi_status_t efi_random_get_seed(efi_system_table_t *sys_table_arg)
{
efi_guid_t rng_algo_raw = EFI_RNG_ALGORITHM_RAW;
- efi_guid_t rng_table_guid = LINUX_EFI_RANDOM_SEED_TABLE_GUID;
struct efi_rng_protocol *rng;
struct linux_efi_random_seed *seed;
efi_status_t status;
@@ -179,8 +178,8 @@ efi_status_t efi_random_get_seed(efi_system_table_t *sys_table_arg)
goto err_freepool;
seed->size = RANDOM_SEED_SIZE;
- status = efi_call_early(install_configuration_table, &rng_table_guid,
- seed);
+ status = efi_call_early(install_configuration_table,
+ &LINUX_EFI_RANDOM_SEED_TABLE_GUID, seed);
if (status != EFI_SUCCESS)
goto err_freepool;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index c4923c1f..0c260f0 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -300,7 +300,7 @@ struct efi_boot_memmap {
efi_status_t (*locate_handle)(int, const efi_guid_t *, void *,
unsigned long *, efi_handle_t *);
void *locate_device_path;
- efi_status_t (*install_configuration_table)(efi_guid_t *, void *);
+ efi_status_t (*install_configuration_table)(const efi_guid_t *, void *);
void *load_image;
void *start_image;
void *exit;
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 07/10] efi: Constify EFI_RNG_PROTOCOL.GetRNG()
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (2 preceding siblings ...)
2017-01-02 10:24 ` [PATCH 05/10] efi: Constify InstallConfigurationTable() Lukas Wunner
@ 2017-01-02 10:24 ` Lukas Wunner
2017-01-02 10:24 ` [PATCH 03/10] efi: Constify GetVariable() / SetVariable() arguments Lukas Wunner
` (7 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
GUIDs passed to EFI_RNG_PROTOCOL.GetRNG() are not modified, so declare
them const. This allows the compiler to place them in the rodata
section instead of generating them on the stack at runtime. Pass GUIDs
as literals rather than assigning them to temporary variables to save a
bit on code. Constification of the GUIDs is in compliance with the EFI
spec which marks them "IN" for this protocol function.
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
drivers/firmware/efi/libstub/random.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index c29a722..844bdc9 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -17,7 +17,7 @@ struct efi_rng_protocol {
efi_status_t (*get_info)(struct efi_rng_protocol *,
unsigned long *, efi_guid_t *);
efi_status_t (*get_rng)(struct efi_rng_protocol *,
- efi_guid_t *, unsigned long, u8 *out);
+ const efi_guid_t *, unsigned long, u8 *out);
};
efi_status_t efi_get_random_bytes(efi_system_table_t *sys_table_arg,
@@ -148,7 +148,6 @@ efi_status_t efi_random_alloc(efi_system_table_t *sys_table_arg,
efi_status_t efi_random_get_seed(efi_system_table_t *sys_table_arg)
{
- efi_guid_t rng_algo_raw = EFI_RNG_ALGORITHM_RAW;
struct efi_rng_protocol *rng;
struct linux_efi_random_seed *seed;
efi_status_t status;
@@ -164,7 +163,7 @@ efi_status_t efi_random_get_seed(efi_system_table_t *sys_table_arg)
if (status != EFI_SUCCESS)
return status;
- status = rng->get_rng(rng, &rng_algo_raw, RANDOM_SEED_SIZE,
+ status = rng->get_rng(rng, &EFI_RNG_ALGORITHM_RAW, RANDOM_SEED_SIZE,
seed->bits);
if (status == EFI_UNSUPPORTED)
/*
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 03/10] efi: Constify GetVariable() / SetVariable() arguments
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (3 preceding siblings ...)
2017-01-02 10:24 ` [PATCH 07/10] efi: Constify EFI_RNG_PROTOCOL.GetRNG() Lukas Wunner
@ 2017-01-02 10:24 ` Lukas Wunner
[not found] ` <2be92e87ff7855bcd88c86a7436f46d3488483f0.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2017-01-02 10:24 ` [PATCH 08/10] efi: Constify EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString() Lukas Wunner
` (6 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
Cc: Tony Luck, Fenghua Yu, Anton Vorontsov, Colin Cross, Kees Cook,
Artur Paszkiewicz, Mike Marciniszyn, Dennis Dalessandro
GUIDs and variable names passed to GetVariable() / SetVariable() are not
modified, so declare them const. This allows the compiler to place them
in the rodata section instead of generating them on the stack at
runtime. Pass GUIDs as literals rather than assigning them to temporary
variables to save a bit on code.
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Fenghua Yu <fenghua.yu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Anton Vorontsov <anton-9xeibp6oKSgdnm+yROfE0A@public.gmane.org>
Cc: Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Artur Paszkiewicz <artur.paszkiewicz-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
arch/ia64/kernel/efi.c | 15 ++++-----------
arch/x86/platform/efi/quirks.c | 2 +-
drivers/firmware/efi/efi-pstore.c | 4 ++--
drivers/firmware/efi/efibc.c | 3 +--
drivers/firmware/efi/libstub/arm-stub.c | 5 ++---
drivers/infiniband/hw/hfi1/efivar.c | 6 +-----
drivers/scsi/isci/probe_roms.c | 2 +-
7 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index f57d089..f1264ab 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -919,22 +919,15 @@ static void __init handle_palo(unsigned long phys_addr)
efi_uart_console_only(void)
{
efi_status_t status;
- char *s, name[] = "ConOut";
- efi_guid_t guid = EFI_GLOBAL_VARIABLE_GUID;
- efi_char16_t *utf16, name_utf16[32];
+ const char name[] = "ConOut";
+ const efi_char16_t name_utf16[] = { 'C', 'o', 'n', 'O', 'u', 't', 0 };
unsigned char data[1024];
unsigned long size = sizeof(data);
struct efi_generic_dev_path *hdr, *end_addr;
int uart = 0;
- /* Convert to UTF-16 */
- utf16 = name_utf16;
- s = name;
- while (*s)
- *utf16++ = *s++ & 0x7f;
- *utf16 = 0;
-
- status = efi.get_variable(name_utf16, &guid, NULL, &size, data);
+ status = efi.get_variable(name_utf16, &EFI_GLOBAL_VARIABLE_GUID,
+ NULL, &size, data);
if (status != EFI_SUCCESS) {
printk(KERN_ERR "No EFI %s variable?\n", name);
return 0;
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 10aca63..aad3701 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -19,7 +19,7 @@
#define EFI_DUMMY_GUID \
EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92, 0xa9)
-static efi_char16_t efi_dummy_name[6] = { 'D', 'U', 'M', 'M', 'Y', 0 };
+static const efi_char16_t efi_dummy_name[6] = { 'D', 'U', 'M', 'M', 'Y', 0 };
static bool efi_no_storage_paranoia;
diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index f402ba2..0911449 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -265,7 +265,6 @@ static int efi_pstore_write(enum pstore_type_id type,
{
char name[DUMP_NAME_LEN];
efi_char16_t efi_name[DUMP_NAME_LEN];
- efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
int i, ret = 0;
sprintf(name, "dump-type%u-%u-%d-%lu-%c", type, part, count,
@@ -274,7 +273,8 @@ static int efi_pstore_write(enum pstore_type_id type,
for (i = 0; i < DUMP_NAME_LEN; i++)
efi_name[i] = name[i];
- efivar_entry_set_safe(efi_name, vendor, PSTORE_EFI_ATTRIBUTES,
+ efivar_entry_set_safe(efi_name, LINUX_EFI_CRASH_GUID,
+ PSTORE_EFI_ATTRIBUTES,
!pstore_cannot_block_path(reason),
size, psi->buf);
diff --git a/drivers/firmware/efi/efibc.c b/drivers/firmware/efi/efibc.c
index 503bbe2..57ddf8c 100644
--- a/drivers/firmware/efi/efibc.c
+++ b/drivers/firmware/efi/efibc.c
@@ -32,7 +32,6 @@ static void efibc_str_to_str16(const char *str, efi_char16_t *str16)
static int efibc_set_variable(const char *name, const char *value)
{
int ret;
- efi_guid_t guid = LINUX_EFI_LOADER_ENTRY_GUID;
struct efivar_entry *entry;
size_t size = (strlen(value) + 1) * sizeof(efi_char16_t);
@@ -49,7 +48,7 @@ static int efibc_set_variable(const char *name, const char *value)
efibc_str_to_str16(name, entry->var.VariableName);
efibc_str_to_str16(value, (efi_char16_t *)entry->var.Data);
- memcpy(&entry->var.VendorGuid, &guid, sizeof(guid));
+ entry->var.VendorGuid = LINUX_EFI_LOADER_ENTRY_GUID;
ret = efivar_entry_set(entry,
EFI_VARIABLE_NON_VOLATILE
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 6fca48c..da72bfb 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -27,13 +27,12 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)
static efi_char16_t const sm_var_name[] = {
'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0 };
- efi_guid_t var_guid = EFI_GLOBAL_VARIABLE_GUID;
efi_get_variable_t *f_getvar = sys_table_arg->runtime->get_variable;
u8 val;
unsigned long size = sizeof(val);
efi_status_t status;
- status = f_getvar((efi_char16_t *)sb_var_name, (efi_guid_t *)&var_guid,
+ status = f_getvar(sb_var_name, &EFI_GLOBAL_VARIABLE_GUID,
NULL, &size, &val);
if (status != EFI_SUCCESS)
@@ -42,7 +41,7 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)
if (val == 0)
return 0;
- status = f_getvar((efi_char16_t *)sm_var_name, (efi_guid_t *)&var_guid,
+ status = f_getvar(sm_var_name, &EFI_GLOBAL_VARIABLE_GUID,
NULL, &size, &val);
if (status != EFI_SUCCESS)
diff --git a/drivers/infiniband/hw/hfi1/efivar.c b/drivers/infiniband/hw/hfi1/efivar.c
index 106349f..f769cc2 100644
--- a/drivers/infiniband/hw/hfi1/efivar.c
+++ b/drivers/infiniband/hw/hfi1/efivar.c
@@ -66,7 +66,6 @@ static int read_efi_var(const char *name, unsigned long *size,
{
efi_status_t status;
efi_char16_t *uni_name;
- efi_guid_t guid;
unsigned long temp_size;
void *temp_buffer;
void *data;
@@ -95,13 +94,10 @@ static int read_efi_var(const char *name, unsigned long *size,
for (i = 0; name[i]; i++)
uni_name[i] = name[i];
- /* need a variable for our GUID */
- guid = HFI1_EFIVAR_GUID;
-
/* call into EFI runtime services */
status = efi.get_variable(
uni_name,
- &guid,
+ &HFI1_EFIVAR_GUID,
NULL,
&temp_size,
temp_buffer);
diff --git a/drivers/scsi/isci/probe_roms.c b/drivers/scsi/isci/probe_roms.c
index 8ac646e..d2c17c1 100644
--- a/drivers/scsi/isci/probe_roms.c
+++ b/drivers/scsi/isci/probe_roms.c
@@ -34,7 +34,7 @@
#include "task.h"
#include "probe_roms.h"
-static efi_char16_t isci_efivar_name[] = {
+static const efi_char16_t isci_efivar_name[] = {
'R', 's', 't', 'S', 'c', 'u', 'O'
};
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 08/10] efi: Constify EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString()
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (4 preceding siblings ...)
2017-01-02 10:24 ` [PATCH 03/10] efi: Constify GetVariable() / SetVariable() arguments Lukas Wunner
@ 2017-01-02 10:24 ` Lukas Wunner
2017-01-02 10:24 ` [PATCH 09/10] efi: Constify efi_guidcmp() arguments Lukas Wunner
` (5 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
UCS-2 strings passed to EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString()
are not modified, so can be declared const. This allows the compiler to
place them in the rodata section instead of generating them on the stack
at runtime, such as with the '\r' string passed from efi_printk() to
efi_char16_printk(). Constification of such strings is in compliance
with the EFI spec which marks them "IN" for this protocol function.
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
arch/x86/boot/compressed/eboot.c | 2 +-
drivers/firmware/efi/libstub/arm-stub.c | 2 +-
drivers/firmware/efi/libstub/efi-stub-helper.c | 2 +-
drivers/firmware/efi/libstub/efistub.h | 2 +-
include/linux/efi.h | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 61014f5..fee5693 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -97,7 +97,7 @@ static inline efi_status_t __open_volume64(void *__image, void **__fh)
return __open_volume32(__image, __fh);
}
-void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
+void efi_char16_printk(efi_system_table_t *table, const efi_char16_t *str)
{
efi_call_proto(efi_simple_text_output_protocol, output_string,
efi_early->text_output, str);
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 70d6722..1053ec2 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -90,7 +90,7 @@ efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
}
void efi_char16_printk(efi_system_table_t *sys_table_arg,
- efi_char16_t *str)
+ const efi_char16_t *str)
{
struct efi_simple_text_output_protocol *out;
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 8e06307..a9b6e4d 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -48,7 +48,7 @@ void efi_printk(efi_system_table_t *sys_table_arg, char *str)
ch[0] = *s8;
if (*s8 == '\n') {
- efi_char16_t nl[2] = { '\r', 0 };
+ const efi_char16_t nl[2] = { '\r', 0 };
efi_char16_printk(sys_table_arg, nl);
}
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 2fe7746..12dee6b 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -24,7 +24,7 @@
#define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
#endif
-void efi_char16_printk(efi_system_table_t *, efi_char16_t *);
+void efi_char16_printk(efi_system_table_t *, const efi_char16_t *);
efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg, void *__image,
void **__fh);
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 6ed28e1..24a3e55 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1257,7 +1257,7 @@ struct efivar_entry {
struct efi_simple_text_output_protocol {
void *reset;
- efi_status_t (*output_string)(void *, void *);
+ efi_status_t (*output_string)(void *, const void *);
void *test_string;
};
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 09/10] efi: Constify efi_guidcmp() arguments
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (5 preceding siblings ...)
2017-01-02 10:24 ` [PATCH 08/10] efi: Constify EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString() Lukas Wunner
@ 2017-01-02 10:24 ` Lukas Wunner
2017-01-02 10:24 ` [PATCH 06/10] efi: Constify EFI_FILE_PROTOCOL.GetInfo() Lukas Wunner
` (4 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
Cc: Anton Vorontsov, Colin Cross, Kees Cook, Tony Luck
GUIDs passed to efi_guidcmp() are not modified, so declare them const.
This allows the compiler to place them in the rodata section instead of
generating them on the stack at runtime. Pass GUIDs as literals rather
than assigning them to temporary variables to save a bit on code.
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Anton Vorontsov <anton-9xeibp6oKSgdnm+yROfE0A@public.gmane.org>
Cc: Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
drivers/firmware/efi/efi-pstore.c | 6 ++----
drivers/firmware/efi/libstub/fdt.c | 3 +--
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 0911449..e7c9fd1 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -46,7 +46,6 @@ static inline u64 generic_id(unsigned long timestamp,
static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
{
- efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
struct pstore_read_data *cb_data = data;
char name[DUMP_NAME_LEN], data_type;
int i;
@@ -54,7 +53,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
unsigned int part;
unsigned long time, size;
- if (efi_guidcmp(entry->var.VendorGuid, vendor))
+ if (efi_guidcmp(entry->var.VendorGuid, LINUX_EFI_CRASH_GUID))
return 0;
for (i = 0; i < DUMP_NAME_LEN; i++)
@@ -299,14 +298,13 @@ struct pstore_erase_data {
static int efi_pstore_erase_func(struct efivar_entry *entry, void *data)
{
struct pstore_erase_data *ed = data;
- efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
efi_char16_t efi_name_old[DUMP_NAME_LEN];
efi_char16_t *efi_name = ed->name;
unsigned long ucs2_len = ucs2_strlen(ed->name);
char name_old[DUMP_NAME_LEN];
int i;
- if (efi_guidcmp(entry->var.VendorGuid, vendor))
+ if (efi_guidcmp(entry->var.VendorGuid, LINUX_EFI_CRASH_GUID))
return 0;
if (ucs2_strncmp(entry->var.VariableName,
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index a6a9311..6ebc3bb 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -332,7 +332,6 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
void *get_fdt(efi_system_table_t *sys_table, unsigned long *fdt_size)
{
- efi_guid_t fdt_guid = DEVICE_TREE_GUID;
efi_config_table_t *tables;
void *fdt;
int i;
@@ -341,7 +340,7 @@ void *get_fdt(efi_system_table_t *sys_table, unsigned long *fdt_size)
fdt = NULL;
for (i = 0; i < sys_table->nr_tables; i++)
- if (efi_guidcmp(tables[i].guid, fdt_guid) == 0) {
+ if (efi_guidcmp(tables[i].guid, DEVICE_TREE_GUID) == 0) {
fdt = (void *) tables[i].table;
if (fdt_check_header(fdt) != 0) {
pr_efi_err(sys_table, "Invalid header detected on UEFI supplied FDT, ignoring ...\n");
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 06/10] efi: Constify EFI_FILE_PROTOCOL.GetInfo()
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (6 preceding siblings ...)
2017-01-02 10:24 ` [PATCH 09/10] efi: Constify efi_guidcmp() arguments Lukas Wunner
@ 2017-01-02 10:24 ` Lukas Wunner
2017-01-02 10:24 ` [PATCH 10/10] uuid: Constify UUID compound literals Lukas Wunner
` (3 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
GUIDs passed to EFI_FILE_PROTOCOL.GetInfo() are not modified, so declare
them const. This allows the compiler to place them in the rodata
section instead of generating them on the stack at runtime. Pass GUIDs
as literals rather than assigning them to temporary variables to save a
bit on code. Constification of the GUIDs is in compliance with the EFI
spec which marks them "IN" for this protocol function.
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
drivers/firmware/efi/libstub/efi-stub-helper.c | 5 ++---
include/linux/efi.h | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 6ee9164..8e06307 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -345,7 +345,6 @@ static efi_status_t efi_file_size(efi_system_table_t *sys_table_arg, void *__fh,
efi_file_handle_t *h, *fh = __fh;
efi_file_info_t *info;
efi_status_t status;
- efi_guid_t info_guid = EFI_FILE_INFO_ID;
unsigned long info_sz;
status = efi_call_proto(efi_file_handle, open, fh, &h, filename_16,
@@ -360,7 +359,7 @@ static efi_status_t efi_file_size(efi_system_table_t *sys_table_arg, void *__fh,
*handle = h;
info_sz = 0;
- status = efi_call_proto(efi_file_handle, get_info, h, &info_guid,
+ status = efi_call_proto(efi_file_handle, get_info, h, &EFI_FILE_INFO_ID,
&info_sz, NULL);
if (status != EFI_BUFFER_TOO_SMALL) {
efi_printk(sys_table_arg, "Failed to get file info size\n");
@@ -375,7 +374,7 @@ static efi_status_t efi_file_size(efi_system_table_t *sys_table_arg, void *__fh,
return status;
}
- status = efi_call_proto(efi_file_handle, get_info, h, &info_guid,
+ status = efi_call_proto(efi_file_handle, get_info, h, &EFI_FILE_INFO_ID,
&info_sz, info);
if (status == EFI_BUFFER_TOO_SMALL) {
efi_call_early(free_pool, info);
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 0c260f0..6ed28e1 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -838,7 +838,7 @@ struct efi_fdt_params {
void *write;
void *get_position;
void *set_position;
- efi_status_t (*get_info)(struct _efi_file_handle *, efi_guid_t *,
+ efi_status_t (*get_info)(struct _efi_file_handle *, const efi_guid_t *,
unsigned long *, void *);
void *set_info;
void *flush;
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 10/10] uuid: Constify UUID compound literals
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (7 preceding siblings ...)
2017-01-02 10:24 ` [PATCH 06/10] efi: Constify EFI_FILE_PROTOCOL.GetInfo() Lukas Wunner
@ 2017-01-02 10:24 ` Lukas Wunner
[not found] ` <c61bbe4733f78c830f6073e86f32159751bbda8c.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2017-01-02 10:24 ` [PATCH 04/10] efi: Constify HandleProtocol() / LocateHandle() / LocateProtocol() Lukas Wunner
` (2 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
Cc: Huang Ying, Rasmus Villemoes
The UUID_BE() and UUID_LE() macros (as well as the derived EFI_GUID())
generate a compound literal for a 128-bit ID. By default, these are not
stored in the rodata section but generated on the stack at runtime in a
very time- and space-inefficient manner.
E.g. EFI_FILE_SYSTEM_GUID (964E5B22-6459-11D2-8E39-00A0C969723B) results
in x86_64 code which requires 80 bytes text instead of 16 bytes rodata:
c6 44 24 20 22 movb $0x22,0x20(%rsp)
c6 44 24 21 5b movb $0x5b,0x21(%rsp)
c6 44 24 22 4e movb $0x4e,0x22(%rsp)
c6 44 24 23 96 movb $0x96,0x23(%rsp)
c6 44 24 24 59 movb $0x59,0x24(%rsp)
c6 44 24 25 64 movb $0x64,0x25(%rsp)
c6 44 24 26 d2 movb $0xd2,0x26(%rsp)
c6 44 24 27 11 movb $0x11,0x27(%rsp)
c6 44 24 28 8e movb $0x8e,0x28(%rsp)
c6 44 24 29 39 movb $0x39,0x29(%rsp)
c6 44 24 2a 00 movb $0x0,0x2a(%rsp)
c6 44 24 2b a0 movb $0xa0,0x2b(%rsp)
c6 44 24 2c c9 movb $0xc9,0x2c(%rsp)
c6 44 24 2d 69 movb $0x69,0x2d(%rsp)
c6 44 24 2e 72 movb $0x72,0x2e(%rsp)
c6 44 24 2f 3b movb $0x3b,0x2f(%rsp)
gcc 7 improves on this by using 2x movabs + 2x mov instructions,
amounting to 30 bytes text instead of 16 bytes rodata. That's more
space-efficient than 80 bytes but still undesirable from a performance
and security perspective:
48 b8 22 5b 4e 96 59 64 d2 11 movabs $0x11d26459964e5b,%rax
48 89 44 24 20 mov %rax,0x20(%rsp)
48 b8 8e 39 00 a0 c9 69 72 3b movabs $0x3b7269c9a00039,%rax
48 89 44 24 28 mov %rax,0x28(%rsp)
The reason for generating compound literals on the stack is that they're
allowed to be modified. E.g. a pointer to a UUID might be used to
change some of its 16 bytes. In the context of UUIDs this seems silly
though: The uuid_be, uuid_le and efi_guid_t data types hide the
distinct bytes making up the ID, the bytes are not meant to be modified.
Moreover, the UUIDs are usually pre-defined in a spec such as UEFI and
there's no reason at all to modify them.
Thus, change the UUID_BE and UUID_LE macros to declare the generated
compound literal const, allowing the compiler to put it in rodata.
When using the compound literal as an initializer for a variable, that
variable has to be declared const as well for the UUID to become rodata.
Is that all? Unfortunately not. Under certain conditions gcc refuses
to store compound literals in rodata despite them being declared const.
Rasmus Villemoes opened a bugzilla entry for this in December 2015:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725
The upshot is that if a UUID is passed by value to a function (either
directly or as a variable), it is stored in rodata. This is the case
for efi_guidcmp(). But if the UUID is passed by reference, it is
generated on the stack, in spite of it being declared const and the
function parameter being declared const. This is the case for the
various EFI boot and runtime services calls such as GetVariable.
A workaround is to declare a *static* const variable, assign the UUID to
it and pass the variable by reference to the function.
This issue also affects efi_char16_t strings which are only declared
const (and not static const). They're likewise generated on the stack
at runtime.
clang 3.9 is not better than gcc: It does the exact opposite by putting
UUIDs in rodata if they're passed by reference, and generating them on
the stack if they're passed by value.
Cc: Huang Ying <ying.huang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
include/uapi/linux/uuid.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 3738e5f..9184f01 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -29,14 +29,14 @@
} uuid_be;
#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
-((uuid_le) \
+((const uuid_le) \
{{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
(b) & 0xff, ((b) >> 8) & 0xff, \
(c) & 0xff, ((c) >> 8) & 0xff, \
(d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
#define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
-((uuid_be) \
+((const uuid_be) \
{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
((b) >> 8) & 0xff, (b) & 0xff, \
((c) >> 8) & 0xff, (c) & 0xff, \
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 04/10] efi: Constify HandleProtocol() / LocateHandle() / LocateProtocol()
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (8 preceding siblings ...)
2017-01-02 10:24 ` [PATCH 10/10] uuid: Constify UUID compound literals Lukas Wunner
@ 2017-01-02 10:24 ` Lukas Wunner
2017-01-03 21:19 ` [PATCH 00/10] efi: Constify all the things Kees Cook
2017-01-04 17:26 ` Ard Biesheuvel
11 siblings, 0 replies; 22+ messages in thread
From: Lukas Wunner @ 2017-01-02 10:24 UTC (permalink / raw)
To: Matt Fleming, Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA
GUIDs passed to HandleProtocol() / LocateHandle() / LocateProtocol() are
not modified, so declare them const. This allows the compiler to place
them in the rodata section instead of generating them on the stack at
runtime. Pass GUIDs as literals rather than assigning them to temporary
variables to save a bit on code. (Except for UGA and GOP GUIDs as they
are passed around between functions.) Constification of the GUIDs is
in compliance with the EFI spec which marks them "IN" for said boot
services.
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
arch/x86/boot/compressed/eboot.c | 48 ++++++++++++++++-----------------
drivers/firmware/efi/libstub/arm-stub.c | 10 +++----
drivers/firmware/efi/libstub/gop.c | 12 ++++-----
drivers/firmware/efi/libstub/random.c | 6 ++---
include/linux/efi.h | 9 ++++---
5 files changed, 39 insertions(+), 46 deletions(-)
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 6d3aeab..61014f5 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -43,13 +43,12 @@ static inline efi_status_t __open_volume32(void *__image, void **__fh)
efi_file_io_interface_t *io;
efi_loaded_image_32_t *image = __image;
efi_file_handle_32_t *fh;
- efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
efi_status_t status;
void *handle = (void *)(unsigned long)image->device_handle;
unsigned long func;
status = efi_call_early(handle_protocol, handle,
- &fs_proto, (void **)&io);
+ &EFI_FILE_SYSTEM_GUID, (void **)&io);
if (status != EFI_SUCCESS) {
efi_printk(sys_table, "Failed to handle fs_proto\n");
return status;
@@ -69,13 +68,12 @@ static inline efi_status_t __open_volume64(void *__image, void **__fh)
efi_file_io_interface_t *io;
efi_loaded_image_64_t *image = __image;
efi_file_handle_64_t *fh;
- efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
efi_status_t status;
void *handle = (void *)(unsigned long)image->device_handle;
unsigned long func;
status = efi_call_early(handle_protocol, handle,
- &fs_proto, (void **)&io);
+ &EFI_FILE_SYSTEM_GUID, (void **)&io);
if (status != EFI_SUCCESS) {
efi_printk(sys_table, "Failed to handle fs_proto\n");
return status;
@@ -173,7 +171,6 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
unsigned long size)
{
efi_pci_io_protocol_32 *pci = NULL;
- efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
u32 *handles = (u32 *)(unsigned long)pci_handle;
efi_status_t status;
unsigned long nr_pci;
@@ -191,7 +188,8 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
u32 h = handles[i];
status = efi_call_early(handle_protocol, h,
- &pci_proto, (void **)&pci);
+ &EFI_PCI_IO_PROTOCOL_GUID,
+ (void **)&pci);
if (status != EFI_SUCCESS)
continue;
@@ -280,7 +278,6 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
unsigned long size)
{
efi_pci_io_protocol_64 *pci = NULL;
- efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
u64 *handles = (u64 *)(unsigned long)pci_handle;
efi_status_t status;
unsigned long nr_pci;
@@ -298,7 +295,8 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
u64 h = handles[i];
status = efi_call_early(handle_protocol, h,
- &pci_proto, (void **)&pci);
+ &EFI_PCI_IO_PROTOCOL_GUID,
+ (void **)&pci);
if (status != EFI_SUCCESS)
continue;
@@ -333,12 +331,12 @@ static void setup_efi_pci(struct boot_params *params)
{
efi_status_t status;
void **pci_handle = NULL;
- efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
unsigned long size = 0;
status = efi_call_early(locate_handle,
EFI_LOCATE_BY_PROTOCOL,
- &pci_proto, NULL, &size, pci_handle);
+ &EFI_PCI_IO_PROTOCOL_GUID,
+ NULL, &size, pci_handle);
if (status == EFI_BUFFER_TOO_SMALL) {
status = efi_call_early(allocate_pool,
@@ -351,7 +349,8 @@ static void setup_efi_pci(struct boot_params *params)
}
status = efi_call_early(locate_handle,
- EFI_LOCATE_BY_PROTOCOL, &pci_proto,
+ EFI_LOCATE_BY_PROTOCOL,
+ &EFI_PCI_IO_PROTOCOL_GUID,
NULL, &size, pci_handle);
}
@@ -369,13 +368,13 @@ static void setup_efi_pci(struct boot_params *params)
static void retrieve_apple_device_properties(struct boot_params *boot_params)
{
- efi_guid_t guid = APPLE_PROPERTIES_PROTOCOL_GUID;
struct setup_data *data, *new;
efi_status_t status;
u32 size = 0;
void *p;
- status = efi_call_early(locate_protocol, &guid, NULL, &p);
+ status = efi_call_early(locate_protocol,
+ &APPLE_PROPERTIES_PROTOCOL_GUID, NULL, &p);
if (status != EFI_SUCCESS)
return;
@@ -434,7 +433,7 @@ static void setup_quirks(struct boot_params *boot_params)
setup_uga32(void **uga_handle, unsigned long size, u32 *width, u32 *height)
{
struct efi_uga_draw_protocol *uga = NULL, *first_uga;
- efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID;
+ const efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID;
unsigned long nr_ugas;
u32 *handles = (u32 *)uga_handle;;
efi_status_t status = EFI_INVALID_PARAMETER;
@@ -443,7 +442,6 @@ static void setup_quirks(struct boot_params *boot_params)
first_uga = NULL;
nr_ugas = size / sizeof(u32);
for (i = 0; i < nr_ugas; i++) {
- efi_guid_t pciio_proto = EFI_PCI_IO_PROTOCOL_GUID;
u32 w, h, depth, refresh;
void *pciio;
u32 handle = handles[i];
@@ -453,7 +451,8 @@ static void setup_quirks(struct boot_params *boot_params)
if (status != EFI_SUCCESS)
continue;
- efi_call_early(handle_protocol, handle, &pciio_proto, &pciio);
+ efi_call_early(handle_protocol, handle,
+ &EFI_PCI_IO_PROTOCOL_GUID, &pciio);
status = efi_early->call((unsigned long)uga->get_mode, uga,
&w, &h, &depth, &refresh);
@@ -479,7 +478,7 @@ static void setup_quirks(struct boot_params *boot_params)
setup_uga64(void **uga_handle, unsigned long size, u32 *width, u32 *height)
{
struct efi_uga_draw_protocol *uga = NULL, *first_uga;
- efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID;
+ const efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID;
unsigned long nr_ugas;
u64 *handles = (u64 *)uga_handle;;
efi_status_t status = EFI_INVALID_PARAMETER;
@@ -488,7 +487,6 @@ static void setup_quirks(struct boot_params *boot_params)
first_uga = NULL;
nr_ugas = size / sizeof(u64);
for (i = 0; i < nr_ugas; i++) {
- efi_guid_t pciio_proto = EFI_PCI_IO_PROTOCOL_GUID;
u32 w, h, depth, refresh;
void *pciio;
u64 handle = handles[i];
@@ -498,7 +496,8 @@ static void setup_quirks(struct boot_params *boot_params)
if (status != EFI_SUCCESS)
continue;
- efi_call_early(handle_protocol, handle, &pciio_proto, &pciio);
+ efi_call_early(handle_protocol, handle,
+ &EFI_PCI_IO_PROTOCOL_GUID, &pciio);
status = efi_early->call((unsigned long)uga->get_mode, uga,
&w, &h, &depth, &refresh);
@@ -523,8 +522,8 @@ static void setup_quirks(struct boot_params *boot_params)
/*
* See if we have Universal Graphics Adapter (UGA) protocol
*/
-static efi_status_t setup_uga(struct screen_info *si, efi_guid_t *uga_proto,
- unsigned long size)
+static efi_status_t setup_uga(struct screen_info *si,
+ const efi_guid_t *uga_proto, unsigned long size)
{
efi_status_t status;
u32 width, height;
@@ -575,9 +574,9 @@ static efi_status_t setup_uga(struct screen_info *si, efi_guid_t *uga_proto,
void setup_graphics(struct boot_params *boot_params)
{
- efi_guid_t graphics_proto = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
+ const efi_guid_t graphics_proto = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
struct screen_info *si;
- efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID;
+ const efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID;
efi_status_t status;
unsigned long size;
void **gop_handle = NULL;
@@ -618,7 +617,6 @@ struct boot_params *make_boot_params(struct efi_config *c)
struct setup_header *hdr;
efi_loaded_image_t *image;
void *options, *handle;
- efi_guid_t proto = LOADED_IMAGE_PROTOCOL_GUID;
int options_size = 0;
efi_status_t status;
char *cmdline_ptr;
@@ -642,7 +640,7 @@ struct boot_params *make_boot_params(struct efi_config *c)
setup_boot_services32(efi_early);
status = efi_call_early(handle_protocol, handle,
- &proto, (void *)&image);
+ &LOADED_IMAGE_PROTOCOL_GUID, (void *)&image);
if (status != EFI_SUCCESS) {
efi_printk(sys_table, "Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
return NULL;
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index da72bfb..70d6722 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -71,12 +71,11 @@ efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
efi_file_io_interface_t *io;
efi_loaded_image_t *image = __image;
efi_file_handle_t *fh;
- efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
efi_status_t status;
void *handle = (void *)(unsigned long)image->device_handle;
- status = sys_table_arg->boottime->handle_protocol(handle,
- &fs_proto, (void **)&io);
+ status = efi_call_early(handle_protocol, handle,
+ &EFI_FILE_SYSTEM_GUID, (void **)&io);
if (status != EFI_SUCCESS) {
efi_printk(sys_table_arg, "Failed to handle fs_proto\n");
return status;
@@ -101,7 +100,7 @@ void efi_char16_printk(efi_system_table_t *sys_table_arg,
static struct screen_info *setup_graphics(efi_system_table_t *sys_table_arg)
{
- efi_guid_t gop_proto = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
+ const efi_guid_t gop_proto = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
efi_status_t status;
unsigned long size;
void **gop_handle = NULL;
@@ -153,7 +152,6 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
char *cmdline_ptr = NULL;
int cmdline_size = 0;
unsigned long new_fdt_addr;
- efi_guid_t loaded_image_proto = LOADED_IMAGE_PROTOCOL_GUID;
unsigned long reserve_addr = 0;
unsigned long reserve_size = 0;
int secure_boot = 0;
@@ -175,7 +173,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
* line.
*/
status = sys_table->boottime->handle_protocol(handle,
- &loaded_image_proto, (void *)&image);
+ &LOADED_IMAGE_PROTOCOL_GUID, (void *)&image);
if (status != EFI_SUCCESS) {
pr_efi_err(sys_table, "Failed to get loaded image protocol\n");
goto fail;
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 932742e..9513a98 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -111,7 +111,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)
static efi_status_t
setup_gop32(efi_system_table_t *sys_table_arg, struct screen_info *si,
- efi_guid_t *proto, unsigned long size, void **gop_handle)
+ const efi_guid_t *proto, unsigned long size, void **gop_handle)
{
struct efi_graphics_output_protocol_32 *gop32, *first_gop;
unsigned long nr_gops;
@@ -131,7 +131,6 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)
nr_gops = size / sizeof(u32);
for (i = 0; i < nr_gops; i++) {
struct efi_graphics_output_mode_info *info = NULL;
- efi_guid_t conout_proto = EFI_CONSOLE_OUT_DEVICE_GUID;
bool conout_found = false;
void *dummy = NULL;
efi_handle_t h = (efi_handle_t)(unsigned long)handles[i];
@@ -143,7 +142,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)
continue;
status = efi_call_early(handle_protocol, h,
- &conout_proto, &dummy);
+ &EFI_CONSOLE_OUT_DEVICE_GUID, &dummy);
if (status == EFI_SUCCESS)
conout_found = true;
@@ -228,7 +227,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)
static efi_status_t
setup_gop64(efi_system_table_t *sys_table_arg, struct screen_info *si,
- efi_guid_t *proto, unsigned long size, void **gop_handle)
+ const efi_guid_t *proto, unsigned long size, void **gop_handle)
{
struct efi_graphics_output_protocol_64 *gop64, *first_gop;
unsigned long nr_gops;
@@ -248,7 +247,6 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)
nr_gops = size / sizeof(u64);
for (i = 0; i < nr_gops; i++) {
struct efi_graphics_output_mode_info *info = NULL;
- efi_guid_t conout_proto = EFI_CONSOLE_OUT_DEVICE_GUID;
bool conout_found = false;
void *dummy = NULL;
efi_handle_t h = (efi_handle_t)(unsigned long)handles[i];
@@ -260,7 +258,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)
continue;
status = efi_call_early(handle_protocol, h,
- &conout_proto, &dummy);
+ &EFI_CONSOLE_OUT_DEVICE_GUID, &dummy);
if (status == EFI_SUCCESS)
conout_found = true;
@@ -323,7 +321,7 @@ static void find_bits(unsigned long mask, u8 *pos, u8 *size)
* See if we have Graphics Output Protocol
*/
efi_status_t efi_setup_gop(efi_system_table_t *sys_table_arg,
- struct screen_info *si, efi_guid_t *proto,
+ struct screen_info *si, const efi_guid_t *proto,
unsigned long size)
{
efi_status_t status;
diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index 7e72954..ffb1130 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -23,11 +23,10 @@ struct efi_rng_protocol {
efi_status_t efi_get_random_bytes(efi_system_table_t *sys_table_arg,
unsigned long size, u8 *out)
{
- efi_guid_t rng_proto = EFI_RNG_PROTOCOL_GUID;
efi_status_t status;
struct efi_rng_protocol *rng;
- status = efi_call_early(locate_protocol, &rng_proto, NULL,
+ status = efi_call_early(locate_protocol, &EFI_RNG_PROTOCOL_GUID, NULL,
(void **)&rng);
if (status != EFI_SUCCESS)
return status;
@@ -149,14 +148,13 @@ efi_status_t efi_random_alloc(efi_system_table_t *sys_table_arg,
efi_status_t efi_random_get_seed(efi_system_table_t *sys_table_arg)
{
- efi_guid_t rng_proto = EFI_RNG_PROTOCOL_GUID;
efi_guid_t rng_algo_raw = EFI_RNG_ALGORITHM_RAW;
efi_guid_t rng_table_guid = LINUX_EFI_RANDOM_SEED_TABLE_GUID;
struct efi_rng_protocol *rng;
struct linux_efi_random_seed *seed;
efi_status_t status;
- status = efi_call_early(locate_protocol, &rng_proto, NULL,
+ status = efi_call_early(locate_protocol, &EFI_RNG_PROTOCOL_GUID, NULL,
(void **)&rng);
if (status != EFI_SUCCESS)
return status;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 6ade102..c4923c1f 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -293,10 +293,11 @@ struct efi_boot_memmap {
void *install_protocol_interface;
void *reinstall_protocol_interface;
void *uninstall_protocol_interface;
- efi_status_t (*handle_protocol)(efi_handle_t, efi_guid_t *, void **);
+ efi_status_t (*handle_protocol)(efi_handle_t, const efi_guid_t *,
+ void **);
void *__reserved;
void *register_protocol_notify;
- efi_status_t (*locate_handle)(int, efi_guid_t *, void *,
+ efi_status_t (*locate_handle)(int, const efi_guid_t *, void *,
unsigned long *, efi_handle_t *);
void *locate_device_path;
efi_status_t (*install_configuration_table)(efi_guid_t *, void *);
@@ -315,7 +316,7 @@ struct efi_boot_memmap {
void *open_protocol_information;
void *protocols_per_handle;
void *locate_handle_buffer;
- efi_status_t (*locate_protocol)(efi_guid_t *, void *, void **);
+ efi_status_t (*locate_protocol)(const efi_guid_t *, void *, void **);
void *install_multiple_protocol_interfaces;
void *uninstall_multiple_protocol_interfaces;
void *calculate_crc32;
@@ -1472,7 +1473,7 @@ efi_status_t handle_cmdline_files(efi_system_table_t *sys_table_arg,
efi_status_t efi_parse_options(char *cmdline);
efi_status_t efi_setup_gop(efi_system_table_t *sys_table_arg,
- struct screen_info *si, efi_guid_t *proto,
+ struct screen_info *si, const efi_guid_t *proto,
unsigned long size);
bool efi_runtime_disabled(void);
--
2.11.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 00/10] efi: Constify all the things
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (9 preceding siblings ...)
2017-01-02 10:24 ` [PATCH 04/10] efi: Constify HandleProtocol() / LocateHandle() / LocateProtocol() Lukas Wunner
@ 2017-01-03 21:19 ` Kees Cook
2017-01-04 17:26 ` Ard Biesheuvel
11 siblings, 0 replies; 22+ messages in thread
From: Kees Cook @ 2017-01-03 21:19 UTC (permalink / raw)
To: Lukas Wunner
Cc: Matt Fleming, Ard Biesheuvel,
linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Tony Luck,
Fenghua Yu, Boris Ostrovsky, David Vrabel, Juergen Gross,
Anton Vorontsov, Colin Cross, Artur Paszkiewicz, Mike Marciniszyn,
Dennis Dalessandro, Huang Ying, Rasmus Villemoes
On Mon, Jan 2, 2017 at 2:24 AM, Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org> wrote:
> When disassembling the EFI stub I noticed that GUIDs and efi_char16_t
> strings are not placed in rodata but generated on the stack at runtime.
> GUIDs occupy up to 80 bytes of text instead of just 16 bytes rodata
> which annoyed me enough to go down the rabbit hole of fixing it.
> This series constifies all GUIDs and EFI-related strings I could find.
>
> The most important patch is [10/10] to constify the EFI_GUID() macro.
> This has to be at the end of the series to avoid compiler warnings.
> Reviewers may want to read its commit message first as it contains
> background information. In particular it explains why this series
> only fixes the kernel side of things. Additional work is necessary
> on the compiler side to actually have all the GUIDs and strings in
> rodata. At the moment both gcc and clang achieve that only partially.
>
> Ard Biesheuvel expressed an interest in these patches in November but
> stressed the importance that arguments to EFI boot/runtime/protocol
> services may only be declared const if the spec marks them "IN":
> https://lkml.org/lkml/2016/11/21/459
>
> Hence I've split the series by boot/runtime/protocol services to allow
> for easy verification of this fact. Constification of GetVariable() /
> SetVariable() is further split in one patch for the function signatures
> and another for the actual arguments because changing the function
> signatures is quite involved in this case.
>
> efi.git/next needs to be rebased on v4.10-rc1 before this series can be
> applied because it requires commits f6697df36bdf ("x86/efi: Prevent mixed
> mode boot corruption with CONFIG_VMAP_STACK=y") and 2fbadc3002c5
> ("arm/arm64: xen: Move shared architecture headers to include/xen/arm").
>
> Unfortunately I was not able to compile-test on ia64, there's no cross-
> compiler package available on Debian. :-( I did not receive complaints
> from 0-day, so I'm cautiously optimistic that I didn't cause breakage
> there.
>
> I've pushed the patches to GitHub to ease reviewing/fetching:
> https://github.com/l1k/linux/commits/efi_const_v1
>
> Thanks,
>
> Lukas
>
>
> Ard Biesheuvel (1):
> efi: use typed function pointers for runtime services table
>
> Lukas Wunner (9):
> efi: Constify GetVariable() / SetVariable() signatures
> efi: Constify GetVariable() / SetVariable() arguments
> efi: Constify HandleProtocol() / LocateHandle() / LocateProtocol()
> efi: Constify InstallConfigurationTable()
> efi: Constify EFI_FILE_PROTOCOL.GetInfo()
> efi: Constify EFI_RNG_PROTOCOL.GetRNG()
> efi: Constify EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString()
> efi: Constify efi_guidcmp() arguments
> uuid: Constify UUID compound literals
Cool! Thanks for doing this!
Reviewed-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 00/10] efi: Constify all the things
[not found] ` <cover.1483318593.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
` (10 preceding siblings ...)
2017-01-03 21:19 ` [PATCH 00/10] efi: Constify all the things Kees Cook
@ 2017-01-04 17:26 ` Ard Biesheuvel
11 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2017-01-04 17:26 UTC (permalink / raw)
To: Lukas Wunner
Cc: Matt Fleming, linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Tony Luck, Fenghua Yu, Boris Ostrovsky, David Vrabel,
Juergen Gross, Anton Vorontsov, Colin Cross, Kees Cook,
Artur Paszkiewicz, Mike Marciniszyn, Dennis Dalessandro,
Huang Ying, Rasmus Villemoes
Hi Lukas,
Thanks for putting this together.
On 2 January 2017 at 10:24, Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org> wrote:
> When disassembling the EFI stub I noticed that GUIDs and efi_char16_t
> strings are not placed in rodata but generated on the stack at runtime.
> GUIDs occupy up to 80 bytes of text instead of just 16 bytes rodata
> which annoyed me enough to go down the rabbit hole of fixing it.
> This series constifies all GUIDs and EFI-related strings I could find.
>
> The most important patch is [10/10] to constify the EFI_GUID() macro.
> This has to be at the end of the series to avoid compiler warnings.
> Reviewers may want to read its commit message first as it contains
> background information. In particular it explains why this series
> only fixes the kernel side of things. Additional work is necessary
> on the compiler side to actually have all the GUIDs and strings in
> rodata. At the moment both gcc and clang achieve that only partially.
>
> Ard Biesheuvel expressed an interest in these patches in November but
> stressed the importance that arguments to EFI boot/runtime/protocol
> services may only be declared const if the spec marks them "IN":
> https://lkml.org/lkml/2016/11/21/459
>
Indeed. I think it is reasonable to infer constness from the IN (and
not OUT) annotation of by-reference arguments. In general, I think the
complete lack of distinction between read-write and read-execute is a
huge oversight, and anything that improves the situation on either
side of the firmware-OS boundary is a welcome change IMO.
> Hence I've split the series by boot/runtime/protocol services to allow
> for easy verification of this fact. Constification of GetVariable() /
> SetVariable() is further split in one patch for the function signatures
> and another for the actual arguments because changing the function
> signatures is quite involved in this case.
>
> efi.git/next needs to be rebased on v4.10-rc1 before this series can be
> applied because it requires commits f6697df36bdf ("x86/efi: Prevent mixed
> mode boot corruption with CONFIG_VMAP_STACK=y") and 2fbadc3002c5
> ("arm/arm64: xen: Move shared architecture headers to include/xen/arm").
>
> Unfortunately I was not able to compile-test on ia64, there's no cross-
> compiler package available on Debian. :-( I did not receive complaints
> from 0-day, so I'm cautiously optimistic that I didn't cause breakage
> there.
>
> I've pushed the patches to GitHub to ease reviewing/fetching:
> https://github.com/l1k/linux/commits/efi_const_v1
>
> Thanks,
>
> Lukas
>
>
> Ard Biesheuvel (1):
> efi: use typed function pointers for runtime services table
>
> Lukas Wunner (9):
> efi: Constify GetVariable() / SetVariable() signatures
> efi: Constify GetVariable() / SetVariable() arguments
> efi: Constify HandleProtocol() / LocateHandle() / LocateProtocol()
> efi: Constify InstallConfigurationTable()
> efi: Constify EFI_FILE_PROTOCOL.GetInfo()
> efi: Constify EFI_RNG_PROTOCOL.GetRNG()
> efi: Constify EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString()
> efi: Constify efi_guidcmp() arguments
> uuid: Constify UUID compound literals
>
> arch/ia64/kernel/efi.c | 24 ++++------
> arch/x86/boot/compressed/eboot.c | 50 ++++++++++-----------
> arch/x86/include/asm/xen/interface.h | 1 +
> arch/x86/platform/efi/efi_64.c | 18 ++++----
> arch/x86/platform/efi/quirks.c | 2 +-
> drivers/firmware/efi/efi-pstore.c | 10 ++---
> drivers/firmware/efi/efibc.c | 3 +-
> drivers/firmware/efi/libstub/arm-stub.c | 17 +++----
> drivers/firmware/efi/libstub/arm32-stub.c | 2 +-
> drivers/firmware/efi/libstub/efi-stub-helper.c | 7 ++-
> drivers/firmware/efi/libstub/efistub.h | 2 +-
> drivers/firmware/efi/libstub/fdt.c | 3 +-
> drivers/firmware/efi/libstub/gop.c | 12 +++--
> drivers/firmware/efi/libstub/random.c | 16 +++----
> drivers/firmware/efi/runtime-wrappers.c | 15 ++++---
> drivers/firmware/google/gsmi.c | 10 ++---
> drivers/infiniband/hw/hfi1/efivar.c | 6 +--
> drivers/scsi/isci/probe_roms.c | 2 +-
> drivers/xen/efi.c | 12 ++---
> include/linux/efi.h | 61 ++++++++++++++------------
> include/uapi/linux/uuid.h | 4 +-
> include/xen/arm/interface.h | 1 +
> include/xen/interface/platform.h | 11 ++++-
> include/xen/xen-ops.h | 12 ++---
> 24 files changed, 143 insertions(+), 158 deletions(-)
>
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread