efi: take size of partition entry from GPT header
Subsystems (3)
PATCH v3 1/1 efi: take size of partition entry from GPT header
Use gpt_header.sizeof_partition_entry instead of sizeof(gpt_entry) for GPT entry size.
According to UEFI 2.7 spec 5.3.1 "GPT overview":, the size of a GUID Partition Entry element is defined in the Size Of Partition Entry field of GPT header. The GPT with entries sized more than sizeof(gpt_entry) is not illegal. OVMF firmware from EDK2 perfectly works with it, see edk2-tianocore source code.
Changes since v1: refactoring (extract get_gpt_entry function),
fix (&ptes[i] -> pte)
Changes since v2: use le32_to_cpu, fix typo, sanity check for
sizeof_partition_entry
--- block/partitions/efi.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-)
block/partitions/efi.c +21 -11
@@ -429,8 +429,8 @@ static int is_gpt_valid(struct parsed_partitions *state, u64 lba, goto fail; }/* Check that sizeof_partition_entry has the correct value */ -if (le32_to_cpu ((*gpt)->sizeof_partition_entry) !=sizeof (gpt_entry)) { -pr_debug ("GUID Partition Entry Size check failed.\n" ); +if (le32_to_cpu ((*gpt)->sizeof_partition_entry) <sizeof (gpt_entry)) { +pr_debug ("GUID Partition Entry Size is too small.\n" ); goto fail; }
@@ -670,6 +670,12 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,return 0 ; } +static gpt_entry *get_gpt_entry (gpt_header *gpt,gpt_entry *ptes,u32 index) +{ +return (gpt_entry *)((u8 *)ptes + +le32_to_cpu (gpt->sizeof_partition_entry) * index); +} +/** * efi_partition(struct parsed_partitions *state) * @state: disk parsed partitions
@@ -704,32 +710,36 @@ int efi_partition(struct parsed_partitions *state)pr_debug ("GUID Partition Table is valid! Yea!\n" ); -for (i =0 ; i <le32_to_cpu (gpt->num_partition_entries) && i < state->limit-1 ; i++) { +for (i =0 ; + i <le32_to_cpu (gpt->num_partition_entries) && i < state->limit-1 ; + i++) { +gpt_entry *pte =get_gpt_entry (gpt, ptes, i);struct partition_meta_info *info;unsigned label_count =0 ;unsigned label_max; -u64 start =le64_to_cpu (ptes[i].starting_lba); -u64 size =le64_to_cpu (ptes[i].ending_lba) - -le64_to_cpu (ptes[i].starting_lba) +1ULL ; +u64 start =le64_to_cpu (pte->starting_lba); +u64 size =le64_to_cpu (pte->ending_lba) - +le64_to_cpu (pte->starting_lba) +1ULL ; -if (!is_pte_valid (&ptes[i],last_lba (state->bdev))) +if (!is_pte_valid (pte,last_lba (state->bdev)))continue ;put_partition (state, i+1 , start * ssz, size * ssz);/* If this is a RAID volume, tell md */ -if (!efi_guidcmp (ptes[i].partition_type_guid,PARTITION_LINUX_RAID_GUID )) +if (!efi_guidcmp ( + pte->partition_type_guid,PARTITION_LINUX_RAID_GUID )) state->parts[i +1 ].flags =ADDPART_FLAG_RAID ; info = &state->parts[i +1 ].info; -efi_guid_to_str (&ptes[i].unique_partition_guid, info->uuid); +efi_guid_to_str (&pte->unique_partition_guid, info->uuid);/* Naively convert UTF16-LE to 7 bits. */ label_max =min (ARRAY_SIZE (info->volname) -1 , -ARRAY_SIZE (ptes[i].partition_name)); +ARRAY_SIZE (pte->partition_name)); info->volname[label_max] =0 ;while (label_count < label_max) { -u8 c = ptes[i].partition_name[label_count] &0xff ; +u8 c = pte->partition_name[label_count] &0xff ;if (c && !isprint (c)) c ='!' ; info->volname[label_count] = c;
Signature · 1 line
2.18.0
Davidlohr Bueso
maintainerblock/partitions/efi.c int efi_partition(struct parsed_partitions *state) · line 715
Quoted 11 lines · Eugene Korenevsky
/**
*efi_partition(struct parsed_partitions *state)
* @state: disk parsed partitionsblock/partitions/efi.c +3
-1@@ -704,32 +710,36 @@ int efi_partition(struct parsed_partitions *state)pr_debug ("GUID Partition Table is valid! Yea!\n" ); -for (i =0 ; i <le32_to_cpu (gpt->num_partition_entries) && i < state->limit-1 ; i++) { +for (i =0 ; + i <le32_to_cpu (gpt->num_partition_entries) && i < state->limit-1 ; + i++) {
Please rearrange this like:
for (i = 0; < le32_to_cpu(gpt->num_partition_entries) &&
i < state->limit-1; i++)Quoted 11 lines
block/partitions/efi.c +4
-3+gpt_entry *pte =get_gpt_entry (gpt, ptes, i);struct partition_meta_info *info;unsigned label_count =0 ;unsigned label_max; -u64 start =le64_to_cpu (ptes[i].starting_lba); -u64 size =le64_to_cpu (ptes[i].ending_lba) - -le64_to_cpu (ptes[i].starting_lba) +1ULL ; +u64 start =le64_to_cpu (pte->starting_lba); +u64 size =le64_to_cpu (pte->ending_lba) - +le64_to_cpu (pte->starting_lba) +1ULL ;
block/partitions/efi.c int efi_partition(struct parsed_partitions *state) · line 731
Quoted 10 lines
block/partitions/efi.c +3
-2-if (!is_pte_valid (&ptes[i],last_lba (state->bdev))) +if (!is_pte_valid (pte,last_lba (state->bdev)))continue ;put_partition (state, i+1 , start * ssz, size * ssz);/* If this is a RAID volume, tell md */ -if (!efi_guidcmp (ptes[i].partition_type_guid,PARTITION_LINUX_RAID_GUID )) +if (!efi_guidcmp ( + pte->partition_type_guid,PARTITION_LINUX_RAID_GUID ))
This is ugly. If you are worried about 80-chars, please do:
if (!efi_guidcmp(pte->partition_type_guid, PARTITION_LINUX_RAID_GUID))
Thanks,
Davidlohr
From: Davidlohr Bueso
Quoted 17 lines
Sent: 12 September 2018 19:55
block/partitions/efi.c int efi_partition(struct parsed_partitions *state) · line 715
Quoted 11 lines · Eugene Korenevsky
/**
*efi_partition(struct parsed_partitions *state)
* @state: disk parsed partitionsblock/partitions/efi.c +3
-1@@ -704,32 +710,36 @@ int efi_partition(struct parsed_partitions *state)pr_debug ("GUID Partition Table is valid! Yea!\n" ); -for (i =0 ; i <le32_to_cpu (gpt->num_partition_entries) && i < state->limit-1 ; i++) { +for (i =0 ; + i <le32_to_cpu (gpt->num_partition_entries) && i < state->limit-1 ; + i++) {Please rearrange this like:
Quoted 2 lines
for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) &&
i < state->limit-1; i++)Better still cache le32_to_cpu(gpt->num_partition_entries) to a local so that it only has to be calculated once (even if it gets spilled to the stack).
It can be bounded by state->limit-1 at the same time.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Quoted 9 lines
Quoted 4 lines
Please rearrange this like:
for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit-1; i++)Better still cache
le32_to_cpu(gpt->num_partition_entries)to a local so that it only has to be calculated once (even if it gets spilled to the stack).
It can be bounded by state->limit-1 at the same time.
This is block I/O, slow path. No need for such micro optimizations here. GCC is capable to do such optimizations automatically, and it does as I can see from disassembly.
Anyway this would be a subject for another patch.
Signature · 1 line
Eugene