| Message ID | 20170913220546.19560-4-robdclark@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | efi_loader: enough UEFI for standard distro boot | expand |
On 09/14/2017 12:05 AM, Rob Clark wrote: > All of the device-path related structures should be packed. UEFI > defines the device-path as a byte-aligned data structure. > > Signed-off-by: Rob Clark <robdclark@gmail.com> > --- > include/efi_api.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/efi_api.h b/include/efi_api.h > index ec1b321e8e..175341348e 100644 > --- a/include/efi_api.h > +++ b/include/efi_api.h > @@ -284,11 +284,11 @@ struct efi_device_path { > u8 type; > u8 sub_type; > u16 length; > -}; > +} __packed; > > struct efi_mac_addr { > u8 addr[32]; Hello Alex, hello Rob, 32 bytes is more than the 6 bytes than we need. Why was the structure defined this way? > -}; > +} __packed; > > #define DEVICE_PATH_TYPE_MESSAGING_DEVICE 0x03 > # define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR 0x0b > @@ -297,7 +297,7 @@ struct efi_device_path_mac_addr { > struct efi_device_path dp; > struct efi_mac_addr mac; > u8 if_type; > -}; > +} __packed; > > #define DEVICE_PATH_TYPE_MEDIA_DEVICE 0x04 > # define DEVICE_PATH_SUB_TYPE_FILE_PATH 0x04 > @@ -305,7 +305,7 @@ struct efi_device_path_mac_addr { > struct efi_device_path_file_path { > struct efi_device_path dp; > u16 str[32]; Where do we get this 32 from? Couldn't file/directory names be shorter (1 character) or longer (255 characters)? Regards Heinrich > -}; > +} __packed; > > #define BLOCK_IO_GUID \ > EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \ >
On Fri, Sep 15, 2017 at 2:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 09/14/2017 12:05 AM, Rob Clark wrote: >> All of the device-path related structures should be packed. UEFI >> defines the device-path as a byte-aligned data structure. >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> >> --- >> include/efi_api.h | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/include/efi_api.h b/include/efi_api.h >> index ec1b321e8e..175341348e 100644 >> --- a/include/efi_api.h >> +++ b/include/efi_api.h >> @@ -284,11 +284,11 @@ struct efi_device_path { >> u8 type; >> u8 sub_type; >> u16 length; >> -}; >> +} __packed; >> >> struct efi_mac_addr { >> u8 addr[32]; > > Hello Alex, hello Rob, > > 32 bytes is more than the 6 bytes than we need. > Why was the structure defined this way? > Off the top of my head, I'm not sure where that came from, maybe this dp node is meant to be variable length? But I guess that is also a separate patch. >> -}; >> +} __packed; >> >> #define DEVICE_PATH_TYPE_MESSAGING_DEVICE 0x03 >> # define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR 0x0b >> @@ -297,7 +297,7 @@ struct efi_device_path_mac_addr { >> struct efi_device_path dp; >> struct efi_mac_addr mac; >> u8 if_type; >> -}; >> +} __packed; >> >> #define DEVICE_PATH_TYPE_MEDIA_DEVICE 0x04 >> # define DEVICE_PATH_SUB_TYPE_FILE_PATH 0x04 >> @@ -305,7 +305,7 @@ struct efi_device_path_mac_addr { >> struct efi_device_path_file_path { >> struct efi_device_path dp; >> u16 str[32]; > > Where do we get this 32 from? > Couldn't file/directory names be shorter (1 character) or longer (255 > characters)? > yup, file-path nodes should be variable length (and are after my patches.. I have a later one that changes it to 'u16 str[]') I believe the hard-coded 32 is due to current staticly initialized device-path structs which go away with "efi_loader: refactor boot device and loaded_image handling" BR, -R
> All of the device-path related structures should be packed. UEFI > defines the device-path as a byte-aligned data structure. > > Signed-off-by: Rob Clark <robdclark@gmail.com> Thanks, applied to efi-next Alex
On Fri, Sep 15, 2017 at 06:53:39PM +0000, Heinrich Schuchardt wrote: > On 09/14/2017 12:05 AM, Rob Clark wrote: > > All of the device-path related structures should be packed. UEFI > > defines the device-path as a byte-aligned data structure. > > > > Signed-off-by: Rob Clark <robdclark@gmail.com> > > --- > > include/efi_api.h | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/include/efi_api.h b/include/efi_api.h > > index ec1b321e8e..175341348e 100644 > > --- a/include/efi_api.h > > +++ b/include/efi_api.h > > @@ -284,11 +284,11 @@ struct efi_device_path { > > u8 type; > > u8 sub_type; > > u16 length; > > -}; > > +} __packed; > > > > struct efi_mac_addr { > > u8 addr[32]; > > Hello Alex, hello Rob, > > 32 bytes is more than the 6 bytes than we need. > Why was the structure defined this way? It probably *should* have been variable length in the spec, but it is defined as 32-bytes. But 6 is also not correct - it's right for ethernet, but not for everything. I suspect they picked the length of an IPoIB (Infiniband) MAC here. > > > -}; > > +} __packed; > > > > #define DEVICE_PATH_TYPE_MESSAGING_DEVICE 0x03 > > # define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR 0x0b > > @@ -297,7 +297,7 @@ struct efi_device_path_mac_addr { > > struct efi_device_path dp; > > struct efi_mac_addr mac; > > u8 if_type; > > -}; > > +} __packed; > > > > #define DEVICE_PATH_TYPE_MEDIA_DEVICE 0x04 > > # define DEVICE_PATH_SUB_TYPE_FILE_PATH 0x04 > > @@ -305,7 +305,7 @@ struct efi_device_path_mac_addr { > > struct efi_device_path_file_path { > > struct efi_device_path dp; > > u16 str[32]; > > Where do we get this 32 from? > Couldn't file/directory names be shorter (1 character) or longer (255 > characters)? Note there's already another patch in this series fixing this.
diff --git a/include/efi_api.h b/include/efi_api.h index ec1b321e8e..175341348e 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -284,11 +284,11 @@ struct efi_device_path { u8 type; u8 sub_type; u16 length; -}; +} __packed; struct efi_mac_addr { u8 addr[32]; -}; +} __packed; #define DEVICE_PATH_TYPE_MESSAGING_DEVICE 0x03 # define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR 0x0b @@ -297,7 +297,7 @@ struct efi_device_path_mac_addr { struct efi_device_path dp; struct efi_mac_addr mac; u8 if_type; -}; +} __packed; #define DEVICE_PATH_TYPE_MEDIA_DEVICE 0x04 # define DEVICE_PATH_SUB_TYPE_FILE_PATH 0x04 @@ -305,7 +305,7 @@ struct efi_device_path_mac_addr { struct efi_device_path_file_path { struct efi_device_path dp; u16 str[32]; -}; +} __packed; #define BLOCK_IO_GUID \ EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \
All of the device-path related structures should be packed. UEFI defines the device-path as a byte-aligned data structure. Signed-off-by: Rob Clark <robdclark@gmail.com> --- include/efi_api.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)