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
Signed-off-by: Eugene Korenevsky <[email protected]>
---
block/partitions/efi.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index 39f70d968754..c8ff7860973d 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -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;
--
2.18.0
On Wed, 12 Sep 2018, Eugene Korenevsky wrote:
> /**
> * 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++) {
Please rearrange this like:
for (i = 0; < 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))
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
> Sent: 12 September 2018 19:55
>
> On Wed, 12 Sep 2018, Eugene Korenevsky wrote:
> > /**
> > * 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++) {
>
> 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.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
> > 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.
--
Eugene