Message ID | 20200422175133.6664-3-xypron.glpk@gmx.de |
---|---|
State | Superseded |
Headers | show |
Series | efi_loader: identify EFI system partition | expand |
Heinrich, On Wed, Apr 22, 2020 at 07:51:33PM +0200, Heinrich Schuchardt wrote: > In subsequent patches UEFI variables shalled be stored on the EFI system > partition. Hence we need to identify the EFI system partition. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > --- > v3: > adjust commit message I said 'no' along with my counter proposals[1], and you haven't replied yet. [1] https://lists.denx.de/pipermail/u-boot/2020-April/406678.html -Takahiro Akashi > v2: > no change > --- > include/efi_loader.h | 7 +++++++ > lib/efi_loader/efi_disk.c | 20 ++++++++++++++++++++ > 2 files changed, 27 insertions(+) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 0ba9a1f702..b7bccf50b3 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -47,6 +47,13 @@ static inline void *guidcpy(void *dst, const void *src) > /* Root node */ > extern efi_handle_t efi_root; > > +/* EFI system partition */ > +extern struct efi_system_partition { > + enum if_type if_type; > + int devnum; > + u8 part; > +} efi_system_partition; > + > int __efi_entry_check(void); > int __efi_exit_check(void); > const char *__efi_nesting(void); > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > index fd8fe17567..fd3df80b0b 100644 > --- a/lib/efi_loader/efi_disk.c > +++ b/lib/efi_loader/efi_disk.c > @@ -13,6 +13,8 @@ > #include <part.h> > #include <malloc.h> > > +struct efi_system_partition efi_system_partition; > + > const efi_guid_t efi_block_io_guid = EFI_BLOCK_IO_PROTOCOL_GUID; > > /** > @@ -418,6 +420,24 @@ static efi_status_t efi_disk_add_dev( > diskobj->ops.media = &diskobj->media; > if (disk) > *disk = diskobj; > + > + /* Store first EFI system partition */ > + if (part && !efi_system_partition.if_type) { > + int r; > + disk_partition_t info; > + > + r = part_get_info(desc, part, &info); > + if (r) > + return EFI_DEVICE_ERROR; > + if (info.bootable & PART_EFI_SYSTEM_PARTITION) { > + efi_system_partition.if_type = desc->if_type; > + efi_system_partition.devnum = desc->devnum; > + efi_system_partition.part = part; > + EFI_PRINT("EFI system partition: %s %d:%d\n", > + blk_get_if_type_name(desc->if_type), > + desc->devnum, part); > + } > + } > return EFI_SUCCESS; > } > > -- > 2.26.1 >
On 4/23/20 2:19 AM, AKASHI Takahiro wrote: > Heinrich, > > On Wed, Apr 22, 2020 at 07:51:33PM +0200, Heinrich Schuchardt wrote: >> In subsequent patches UEFI variables shalled be stored on the EFI system >> partition. Hence we need to identify the EFI system partition. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >> --- >> v3: >> adjust commit message > > > I said 'no' along with my counter proposals[1], and you haven't > replied yet. > > [1] https://lists.denx.de/pipermail/u-boot/2020-April/406678.html For some scenarios it may make sense to be able to specify a boot device containing variables via a Kconfig option. This patch does not stop us for implementing this. If you take a Linux distribution that can either be installed on eMMC or on an SD-card of the same board, autodetection of the EFI system partition may be a more a better choice than providing multiple U-Boot builds. Both approaches have there merits and both could be enabled via Kconfig. Best regards Heinrich > > -Takahiro Akashi > >> v2: >> no change >> --- >> include/efi_loader.h | 7 +++++++ >> lib/efi_loader/efi_disk.c | 20 ++++++++++++++++++++ >> 2 files changed, 27 insertions(+) >> >> diff --git a/include/efi_loader.h b/include/efi_loader.h >> index 0ba9a1f702..b7bccf50b3 100644 >> --- a/include/efi_loader.h >> +++ b/include/efi_loader.h >> @@ -47,6 +47,13 @@ static inline void *guidcpy(void *dst, const void *src) >> /* Root node */ >> extern efi_handle_t efi_root; >> >> +/* EFI system partition */ >> +extern struct efi_system_partition { >> + enum if_type if_type; >> + int devnum; >> + u8 part; >> +} efi_system_partition; >> + >> int __efi_entry_check(void); >> int __efi_exit_check(void); >> const char *__efi_nesting(void); >> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c >> index fd8fe17567..fd3df80b0b 100644 >> --- a/lib/efi_loader/efi_disk.c >> +++ b/lib/efi_loader/efi_disk.c >> @@ -13,6 +13,8 @@ >> #include <part.h> >> #include <malloc.h> >> >> +struct efi_system_partition efi_system_partition; >> + >> const efi_guid_t efi_block_io_guid = EFI_BLOCK_IO_PROTOCOL_GUID; >> >> /** >> @@ -418,6 +420,24 @@ static efi_status_t efi_disk_add_dev( >> diskobj->ops.media = &diskobj->media; >> if (disk) >> *disk = diskobj; >> + >> + /* Store first EFI system partition */ >> + if (part && !efi_system_partition.if_type) { >> + int r; >> + disk_partition_t info; >> + >> + r = part_get_info(desc, part, &info); >> + if (r) >> + return EFI_DEVICE_ERROR; >> + if (info.bootable & PART_EFI_SYSTEM_PARTITION) { >> + efi_system_partition.if_type = desc->if_type; >> + efi_system_partition.devnum = desc->devnum; >> + efi_system_partition.part = part; >> + EFI_PRINT("EFI system partition: %s %d:%d\n", >> + blk_get_if_type_name(desc->if_type), >> + desc->devnum, part); >> + } >> + } >> return EFI_SUCCESS; >> } >> >> -- >> 2.26.1 >>
On Wed, Apr 22, 2020 at 07:51:33PM +0200, Heinrich Schuchardt wrote: > In subsequent patches UEFI variables shalled be stored on the EFI system > partition. Hence we need to identify the EFI system partition. Hi, I'm sorry, but, I'm wondering if this is a good idea? The EFI system partition is just some FAT-Partition, and if the system is using secure boot and someone happens to manage to mount that partition, then the variables can be changed pretty easily. Also I guess changing variables using the Runtime Services would then try to access the partition? What if the OS is accessing the partition as well at the same time? I'm currently storing the U-Boot environment, including the UEFI Secure Boot environment, on a eMMC partition with a temporary write protect. This means I cannot change the variables with Runtime Services after leaving U-Boot, but it also means that an exploit on my OS doesn't allow the attacker to change the variables, because they are write- protected until the machine reboots and enters U-Boot again. I hope we will keep the possibility to store the UEFI variables in the U-Boot environment, or in some raw sector on the MMC partition, since otherwise the safety of those variables could be in danger. Best regards, Patrick
diff --git a/include/efi_loader.h b/include/efi_loader.h index 0ba9a1f702..b7bccf50b3 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -47,6 +47,13 @@ static inline void *guidcpy(void *dst, const void *src) /* Root node */ extern efi_handle_t efi_root; +/* EFI system partition */ +extern struct efi_system_partition { + enum if_type if_type; + int devnum; + u8 part; +} efi_system_partition; + int __efi_entry_check(void); int __efi_exit_check(void); const char *__efi_nesting(void); diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index fd8fe17567..fd3df80b0b 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -13,6 +13,8 @@ #include <part.h> #include <malloc.h> +struct efi_system_partition efi_system_partition; + const efi_guid_t efi_block_io_guid = EFI_BLOCK_IO_PROTOCOL_GUID; /** @@ -418,6 +420,24 @@ static efi_status_t efi_disk_add_dev( diskobj->ops.media = &diskobj->media; if (disk) *disk = diskobj; + + /* Store first EFI system partition */ + if (part && !efi_system_partition.if_type) { + int r; + disk_partition_t info; + + r = part_get_info(desc, part, &info); + if (r) + return EFI_DEVICE_ERROR; + if (info.bootable & PART_EFI_SYSTEM_PARTITION) { + efi_system_partition.if_type = desc->if_type; + efi_system_partition.devnum = desc->devnum; + efi_system_partition.part = part; + EFI_PRINT("EFI system partition: %s %d:%d\n", + blk_get_if_type_name(desc->if_type), + desc->devnum, part); + } + } return EFI_SUCCESS; }
In subsequent patches UEFI variables shalled be stored on the EFI system partition. Hence we need to identify the EFI system partition. Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> --- v3: adjust commit message v2: no change --- include/efi_loader.h | 7 +++++++ lib/efi_loader/efi_disk.c | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+) -- 2.26.1