2006-03-19 18:46:23

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH] - make sure that EFI variable data size is always 64 bit

The EFI spec states that the data size of an EFI variable is 64 bits.
"unsigned long", on the other hand, isn't on IA32.

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index bda5bce..488c24c 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -110,7 +110,7 @@ static LIST_HEAD(efivar_list);
struct efi_variable {
efi_char16_t VariableName[1024/sizeof(efi_char16_t)];
efi_guid_t VendorGuid;
- unsigned long DataSize;
+ __u64 DataSize;
__u8 Data[1024];
efi_status_t Status;
__u32 Attributes;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 9e97bc2..3f0a179 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -163,7 +163,7 @@ typedef efi_status_t efi_get_wakeup_time
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,
- unsigned long *data_size, void *data);
+ __u64 *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,

--
Matthew Garrett | [email protected]


2006-03-19 21:29:01

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH] - make sure that EFI variable data size is always 64 bit

On Sun, Mar 19, 2006 at 06:43:25PM +0000, Matthew Garrett wrote:
> The EFI spec states that the data size of an EFI variable is 64 bits.
> "unsigned long", on the other hand, isn't on IA32.
>
> diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> index bda5bce..488c24c 100644
> --- a/drivers/firmware/efivars.c
> +++ b/drivers/firmware/efivars.c
> @@ -110,7 +110,7 @@ static LIST_HEAD(efivar_list);
> struct efi_variable {
> efi_char16_t VariableName[1024/sizeof(efi_char16_t)];
> efi_guid_t VendorGuid;
> - unsigned long DataSize;
> + __u64 DataSize;
> __u8 Data[1024];
> efi_status_t Status;
> __u32 Attributes;
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 9e97bc2..3f0a179 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -163,7 +163,7 @@ typedef efi_status_t efi_get_wakeup_time
> 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,
> - unsigned long *data_size, void *data);
> + __u64 *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,



NAK. efibootmgr, the main userspace consumer of this struct, also
thinks this is an "unsigned long". This wasn't specified up through
EFI 1.10, which I complained about (it, and efi_status_t being defined
as an unsigned long) both force userspace consumers of this struct to
match size of the kernel (i.e. compile a 32-bit binary for 32-bit kernel,
and compile a 64-bit binary for 64-bit kernel).

-Matt


--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

2006-03-19 21:36:11

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] - make sure that EFI variable data size is always 64 bit

On Sun, Mar 19, 2006 at 03:29:01PM -0600, Matt Domsch wrote:

> NAK. efibootmgr, the main userspace consumer of this struct, also
> thinks this is an "unsigned long".

Hm. My copy of efibootmgr has:

typedef struct _efi_variable_t {
efi_char16_t VariableName[1024/sizeof(efi_char16_t)];
efi_guid_t VendorGuid;
uint64_t DataSize;
uint8_t Data[1024];
efi_status_t Status;
uint32_t Attributes;
} __attribute__((packed)) efi_variable_t;

which certainly makes it look like it's expecting a 64-bit value. But
checking the spec does seem to suggest that datasize is a native value,
so presumably it's an efibootmgr bug rather than a kernel one? In that
case, this ought to be dropped.

--
Matthew Garrett | [email protected]

2006-03-19 22:49:45

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH] - make sure that EFI variable data size is always 64 bit

On Sun, Mar 19, 2006 at 09:33:01PM +0000, Matthew Garrett wrote:
> On Sun, Mar 19, 2006 at 03:29:01PM -0600, Matt Domsch wrote:
>
> > NAK. efibootmgr, the main userspace consumer of this struct, also
> > thinks this is an "unsigned long".
>
> Hm. My copy of efibootmgr has:
>
> typedef struct _efi_variable_t {
> efi_char16_t VariableName[1024/sizeof(efi_char16_t)];
> efi_guid_t VendorGuid;
> uint64_t DataSize;
> uint8_t Data[1024];
> efi_status_t Status;
> uint32_t Attributes;
> } __attribute__((packed)) efi_variable_t;
>
> which certainly makes it look like it's expecting a 64-bit value. But
> checking the spec does seem to suggest that datasize is a native value,
> so presumably it's an efibootmgr bug rather than a kernel one? In that
> case, this ought to be dropped.

Sorry, I did mean to post a link to the latest efibootmgr that has
this fixed:

Home page: http://linux.dell.com/efibootmgr/

For distros that care about 32-bit EFI, please upgrade to efibootmgr 0.5.3.
http://linux.dell.com/efibootmgr/efibootmgr-0.5.3.tar.gz
http://linux.dell.com/efibootmgr/efibootmgr-0.5.3.tar.gz.sign
http://linux.dell.com/efibootmgr/ChangeLog

Thanks,
Matt

--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

2006-03-19 23:21:28

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] - make sure that EFI variable data size is always 64 bit

On Sun, Mar 19, 2006 at 04:49:40PM -0600, Matt Domsch wrote:

> Sorry, I did mean to post a link to the latest efibootmgr that has
> this fixed:

Thanks - the copy I had was rather older than I thought. Sorry about the
confusion.

--
Matthew Garrett | [email protected]

2006-03-20 08:55:07

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] - make sure that EFI variable data size is always 64 bit

On Ne 19-03-06 18:43:25, Matthew Garrett wrote:
> The EFI spec states that the data size of an EFI variable is 64 bits.
> "unsigned long", on the other hand, isn't on IA32.
>
> diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> index bda5bce..488c24c 100644
> --- a/drivers/firmware/efivars.c
> +++ b/drivers/firmware/efivars.c
> @@ -110,7 +110,7 @@ static LIST_HEAD(efivar_list);
> struct efi_variable {
> efi_char16_t VariableName[1024/sizeof(efi_char16_t)];
> efi_guid_t VendorGuid;
> - unsigned long DataSize;
> + __u64 DataSize;
> __u8 Data[1024];
> efi_status_t Status;
> __u32 Attributes;

Please, use u64 (not __u64).

Pavel

--
86: {