[U-Boot] [PATCH] efi_loader: disk: install file system protocol to a whole disk

[Heinrich Schuchardt]

On 9/11/19 8:16 AM, AKASHI Takahiro wrote:
> Heinrich,
>
> On Fri, Aug 23, 2019 at 09:04:21AM +0900, AKASHI Takahiro wrote:
>> On Thu, Aug 22, 2019 at 12:52:41PM +0200, Heinrich Schuchardt wrote:
>>> On 8/22/19 11:11 AM, Mark Kettenis wrote:
>>>>> From: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>>>> Date: Thu, 22 Aug 2019 17:06:25 +0900
>>>>>
>>>>> Currently, a whole disk without any partitions is not associated
>>>>> with EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. So even if it houses FAT
>>>>> file system, there is a chance that we may not be able to access
>>>>> it, particularly, when accesses are to be attempted after searching
>>>>> that protocol against a device handle.
>>>>>
>>>>> With this patch, EFI_SIMPLE_FILE_SYSTEM_PROTOCOL is installed
>>>>> to such a disk if part_get_info() shows there is not partition
>>>>> table installed on it.
>>>>
>>>> Do other UEFI implementations support this?
>>>
>>> What use cases exist that come without partition table?
>>
>> I didn't find any *explicit* description in UEFI specification
>> that mandates that any block device should have a partition table.
>> It may be mandatory for boot(able) disks, but for others?
>>
>>> You can create an MBR with partition table that is a valid start of a
>>> file system.
>>
>> Obviously we can do that, but if this is not a mandatory requirement,
>> we'd better support no-partitioned cases.

I did not mean this as a requirement but as a source of errors.

My idea is that could have an MBR which by chance looks like the start
of a file system. When you now expose this non-existent file system the
UEFI client may create havoc.

>
> Any further comments?
>
> -Takahiro Akashi
>
>>
>>> So you should first check if a partition table exists. Only if none
>>> exists you can test for a possible file system.
>>
>> I don't get your point. Are you saying that we should support a file system
>> for a disk only if it has a file system?
>> This is not true even for existing partitions as FILE_SYSTEM PROTOCOL
>> is always installed to every partition whether or not it really
>> houses a file system under the current implementation.

That sounds like a bug. If a partition isn't formatted we would not even
know whether to call the FAT or the EXT4 driver.

Regards

Heinrich

>>
>> Thanks,
>> -Takahiro Akashi
>>
>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>>
>>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>>>> ---
>>>>>   lib/efi_loader/efi_disk.c | 4 +++-
>>>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>>>>> index 7a6b06821a47..548fe667e6f8 100644
>>>>> --- a/lib/efi_loader/efi_disk.c
>>>>> +++ b/lib/efi_loader/efi_disk.c
>>>>> @@ -239,6 +239,7 @@ static efi_status_t efi_disk_add_dev(
>>>>>   				struct efi_disk_obj **disk)
>>>>>   {
>>>>>   	struct efi_disk_obj *diskobj;
>>>>> +	disk_partition_t info;
>>>>>   	efi_status_t ret;
>>>>>
>>>>>   	/* Don't add empty devices */
>>>>> @@ -270,7 +271,8 @@ static efi_status_t efi_disk_add_dev(
>>>>>   			       diskobj->dp);
>>>>>   	if (ret != EFI_SUCCESS)
>>>>>   		return ret;
>>>>> -	if (part >= 1) {
>>>>> +	/* partitions or whole disk without partitions */
>>>>> +	if (part >= 1 || part_get_info(desc, part, &info)) {
>>>>>   		diskobj->volume = efi_simple_file_system(desc, part,
>>>>>   							 diskobj->dp);
>>>>>   		ret = efi_add_protocol(&diskobj->header,
>>>>> --
>>>>> 2.21.0
>>>>>
>>>>> _______________________________________________
>>>>> U-Boot mailing list
>>>>> U-Boot at lists.denx.de
>>>>> https://lists.denx.de/listinfo/u-boot
>>>>
>>>
>