[U-Boot] [PATCH] efi_loader: disk: install file system protocol to a whole disk
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.
Signed-off-by: AKASHI Takahiro takahiro.akashi@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,
From: AKASHI Takahiro takahiro.akashi@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?
Signed-off-by: AKASHI Takahiro takahiro.akashi@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@lists.denx.de https://lists.denx.de/listinfo/u-boot
On 8/22/19 11:11 AM, Mark Kettenis wrote:
From: AKASHI Takahiro takahiro.akashi@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?
You can create an MBR with partition table that is a valid start of a file system.
So you should first check if a partition table exists. Only if none exists you can test for a possible file system.
Best regards
Heinrich
Signed-off-by: AKASHI Takahiro takahiro.akashi@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@lists.denx.de https://lists.denx.de/listinfo/u-boot
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@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.
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.
Thanks, -Takahiro Akashi
Best regards
Heinrich
Signed-off-by: AKASHI Takahiro takahiro.akashi@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@lists.denx.de https://lists.denx.de/listinfo/u-boot
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@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.
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.
Thanks, -Takahiro Akashi
Best regards
Heinrich
Signed-off-by: AKASHI Takahiro takahiro.akashi@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@lists.denx.de https://lists.denx.de/listinfo/u-boot
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@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@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@lists.denx.de https://lists.denx.de/listinfo/u-boot
On Wed, Sep 11, 2019 at 07:31:31PM +0200, Heinrich Schuchardt wrote:
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@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.
See below.
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.
Yes, the issue does exist in the current implementation. So a good approach is 1. Check if a partition table is installed or not 2. If yes, 2-1 If a partition has a file system, install FILE_SYSTEM_PROTOCOL to *that* partition. 2-2 not install FILE_SYSTEM_PROTOCOL to a whole disk 3. If no, and only if a file system exists, install FILE_SYSTEM_PROTOCOL to a whole disk
(2.1) and additional check at (3) should be added.
-Takahiro Akashi
Regards
Heinrich
Thanks, -Takahiro Akashi
Best regards
Heinrich
Signed-off-by: AKASHI Takahiro takahiro.akashi@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@lists.denx.de https://lists.denx.de/listinfo/u-boot
participants (3)
-
AKASHI Takahiro -
Heinrich Schuchardt -
Mark Kettenis