[PATCH 0/2] efi_loader: multi part device paths to text
Our current implementation of EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi part device paths after the first part. We should convert all parts.
Render device path instance ends as commas. This is not explicitly described in the UEFI spec but mimics what EDK II does.
Provide a unit test for multi part device paths.
Heinrich Schuchardt (2): efi_loader: multi part device paths to text efi_selftest: multi part device path to text
lib/efi_loader/efi_device_path_to_text.c | 17 ++++-- lib/efi_selftest/efi_selftest_devicepath.c | 65 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-)
-- 2.30.0
Our current implementation of EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi part device paths after the first part. We should convert all parts.
Render device path instance ends as commas. This is not explicitly described in the UEFI spec but mimics what EDK II does.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_device_path_to_text.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 1aaa9f94fa..81b8ac23ba 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -369,11 +369,18 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
if (!device_path) goto out; - while (device_path && - str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) { - *str++ = '/'; - str = efi_convert_single_device_node_to_text(str, device_path); - device_path = efi_dp_next(device_path); + while (device_path && str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) { + if (device_path->type == DEVICE_PATH_TYPE_END) { + if (device_path->sub_type != + DEVICE_PATH_SUB_TYPE_INSTANCE_END) + break; + *str++ = ','; + } else { + *str++ = '/'; + str = efi_convert_single_device_node_to_text( + str, device_path); + } + *(u8 **)&device_path += device_path->length; }
text = efi_str_to_u16(buffer); -- 2.30.0
Hi Heinrich
On Thu, Feb 18, 2021 at 06:30:43PM +0100, Heinrich Schuchardt wrote:
Our current implementation of EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi part device paths after the first part. We should convert all parts.
Render device path instance ends as commas. This is not explicitly described in the UEFI spec but mimics what EDK II does.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/efi_device_path_to_text.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 1aaa9f94fa..81b8ac23ba 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -369,11 +369,18 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
if (!device_path) goto out;
- while (device_path &&
str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {*str++ = '/';str = efi_convert_single_device_node_to_text(str, device_path);device_path = efi_dp_next(device_path);
- while (device_path && str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
if (device_path->type == DEVICE_PATH_TYPE_END) {if (device_path->sub_type !=DEVICE_PATH_SUB_TYPE_INSTANCE_END)
maybe it's better to check explicitly for '== DEVICE_PATH_SUB_TYPE_END'?
break;*str++ = ',';} else {*str++ = '/';str = efi_convert_single_device_node_to_text(str, device_path);}*(u8 **)&device_path += device_path->length;}
text = efi_str_to_u16(buffer);
-- 2.30.0
Cheers /Ilias
On 19.02.21 11:13, Ilias Apalodimas wrote:
Hi Heinrich
On Thu, Feb 18, 2021 at 06:30:43PM +0100, Heinrich Schuchardt wrote:
Our current implementation of EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi part device paths after the first part. We should convert all parts.
Render device path instance ends as commas. This is not explicitly described in the UEFI spec but mimics what EDK II does.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/efi_device_path_to_text.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 1aaa9f94fa..81b8ac23ba 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -369,11 +369,18 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
if (!device_path) goto out;
- while (device_path &&
str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {*str++ = '/';str = efi_convert_single_device_node_to_text(str, device_path);device_path = efi_dp_next(device_path);
- while (device_path && str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
if (device_path->type == DEVICE_PATH_TYPE_END) {if (device_path->sub_type !=DEVICE_PATH_SUB_TYPE_INSTANCE_END)maybe it's better to check explicitly for '== DEVICE_PATH_SUB_TYPE_END'?
Thank you for reviewing.
I want to leave the loop when an unexpected value occurs.
Best regards
Heinrich
break;*str++ = ',';} else {*str++ = '/';str = efi_convert_single_device_node_to_text(str, device_path);}*(u8 **)&device_path += device_path->length;}
text = efi_str_to_u16(buffer);
-- 2.30.0
Cheers /Ilias
On Fri, Feb 19, 2021 at 12:45:38PM +0100, Heinrich Schuchardt wrote:
On 19.02.21 11:13, Ilias Apalodimas wrote:
Hi Heinrich
On Thu, Feb 18, 2021 at 06:30:43PM +0100, Heinrich Schuchardt wrote:
Our current implementation of EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi part device paths after the first part. We should convert all parts.
Render device path instance ends as commas. This is not explicitly described in the UEFI spec but mimics what EDK II does.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/efi_device_path_to_text.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 1aaa9f94fa..81b8ac23ba 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -369,11 +369,18 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
if (!device_path) goto out;
- while (device_path &&
str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {*str++ = '/';str = efi_convert_single_device_node_to_text(str, device_path);device_path = efi_dp_next(device_path);
- while (device_path && str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
if (device_path->type == DEVICE_PATH_TYPE_END) {if (device_path->sub_type !=DEVICE_PATH_SUB_TYPE_INSTANCE_END)maybe it's better to check explicitly for '== DEVICE_PATH_SUB_TYPE_END'?
Thank you for reviewing.
I want to leave the loop when an unexpected value occurs.
Fair enough
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Best regards
Heinrich
break;*str++ = ',';} else {*str++ = '/';str = efi_convert_single_device_node_to_text(str, device_path);}*(u8 **)&device_path += device_path->length;}
text = efi_str_to_u16(buffer);
-- 2.30.0
Cheers /Ilias
Test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() for a multi part device path.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_selftest/efi_selftest_devicepath.c | 65 ++++++++++++++++++++++ 1 file changed, 65 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_devicepath.c b/lib/efi_selftest/efi_selftest_devicepath.c index 4ce3fad895..d87b9f7dcd 100644 --- a/lib/efi_selftest/efi_selftest_devicepath.c +++ b/lib/efi_selftest/efi_selftest_devicepath.c @@ -45,6 +45,55 @@ static u8 *dp1; static u8 *dp2; static u8 *dp3;
+static struct { + struct efi_device_path_sd_mmc_path sd1; + struct efi_device_path sep1; + struct efi_device_path_sd_mmc_path sd2; + struct efi_device_path sep2; + struct efi_device_path_sd_mmc_path sd3; + struct efi_device_path end; +} multi_part_dp = { + { + { + DEVICE_PATH_TYPE_MESSAGING_DEVICE, + DEVICE_PATH_SUB_TYPE_MSG_SD, + sizeof(struct efi_device_path_sd_mmc_path), + }, + 0, + }, + { + DEVICE_PATH_TYPE_END, + DEVICE_PATH_SUB_TYPE_INSTANCE_END, + sizeof(struct efi_device_path), + }, + { + { + DEVICE_PATH_TYPE_MESSAGING_DEVICE, + DEVICE_PATH_SUB_TYPE_MSG_SD, + sizeof(struct efi_device_path_sd_mmc_path), + }, + 1, + }, + { + DEVICE_PATH_TYPE_END, + DEVICE_PATH_SUB_TYPE_INSTANCE_END, + sizeof(struct efi_device_path), + }, + { + { + DEVICE_PATH_TYPE_MESSAGING_DEVICE, + DEVICE_PATH_SUB_TYPE_MSG_SD, + sizeof(struct efi_device_path_sd_mmc_path), + }, + 2, + }, + { + DEVICE_PATH_TYPE_END, + DEVICE_PATH_SUB_TYPE_END, + sizeof(struct efi_device_path), + }, +}; + struct efi_device_path_to_text_protocol *device_path_to_text;
/* @@ -340,6 +389,22 @@ static int execute(void) return EFI_ST_FAILURE; }
+ string = device_path_to_text->convert_device_path_to_text( + (struct efi_device_path *)&multi_part_dp, true, false); + if (efi_st_strcmp_16_8( + string, + "/SD(0),/SD(1),/SD(2)") + ) { + efi_st_printf("multi_part_dp: %ps\n", string); + efi_st_error("Incorrect text from ConvertDevicePathToText\n"); + return EFI_ST_FAILURE; + } + ret = boottime->free_pool(string); + if (ret != EFI_SUCCESS) { + efi_st_error("FreePool failed\n"); + return EFI_ST_FAILURE; + } + /* Test ConvertDeviceNodeToText */ string = device_path_to_text->convert_device_node_to_text( (struct efi_device_path *)&dp_node, true, false); -- 2.30.0
On Thu, Feb 18, 2021 at 06:30:44PM +0100, Heinrich Schuchardt wrote:
Test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() for a multi part device path.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_selftest/efi_selftest_devicepath.c | 65 ++++++++++++++++++++++ 1 file changed, 65 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_devicepath.c b/lib/efi_selftest/efi_selftest_devicepath.c index 4ce3fad895..d87b9f7dcd 100644 --- a/lib/efi_selftest/efi_selftest_devicepath.c +++ b/lib/efi_selftest/efi_selftest_devicepath.c @@ -45,6 +45,55 @@ static u8 *dp1; static u8 *dp2; static u8 *dp3;
+static struct {
- struct efi_device_path_sd_mmc_path sd1;
- struct efi_device_path sep1;
- struct efi_device_path_sd_mmc_path sd2;
- struct efi_device_path sep2;
- struct efi_device_path_sd_mmc_path sd3;
- struct efi_device_path end;
+} multi_part_dp = {
- {
{DEVICE_PATH_TYPE_MESSAGING_DEVICE,DEVICE_PATH_SUB_TYPE_MSG_SD,sizeof(struct efi_device_path_sd_mmc_path),},0,- },
- {
DEVICE_PATH_TYPE_END,DEVICE_PATH_SUB_TYPE_INSTANCE_END,sizeof(struct efi_device_path),- },
- {
{DEVICE_PATH_TYPE_MESSAGING_DEVICE,DEVICE_PATH_SUB_TYPE_MSG_SD,sizeof(struct efi_device_path_sd_mmc_path),},1,- },
- {
DEVICE_PATH_TYPE_END,DEVICE_PATH_SUB_TYPE_INSTANCE_END,sizeof(struct efi_device_path),- },
- {
{DEVICE_PATH_TYPE_MESSAGING_DEVICE,DEVICE_PATH_SUB_TYPE_MSG_SD,sizeof(struct efi_device_path_sd_mmc_path),},2,- },
- {
DEVICE_PATH_TYPE_END,DEVICE_PATH_SUB_TYPE_END,sizeof(struct efi_device_path),- },
+};
struct efi_device_path_to_text_protocol *device_path_to_text;
/* @@ -340,6 +389,22 @@ static int execute(void) return EFI_ST_FAILURE; }
- string = device_path_to_text->convert_device_path_to_text(
(struct efi_device_path *)&multi_part_dp, true, false);- if (efi_st_strcmp_16_8(
string,"/SD(0),/SD(1),/SD(2)")) {efi_st_printf("multi_part_dp: %ps\n", string);efi_st_error("Incorrect text from ConvertDevicePathToText\n");return EFI_ST_FAILURE;- }
- ret = boottime->free_pool(string);
- if (ret != EFI_SUCCESS) {
efi_st_error("FreePool failed\n");return EFI_ST_FAILURE;- }
- /* Test ConvertDeviceNodeToText */ string = device_path_to_text->convert_device_node_to_text( (struct efi_device_path *)&dp_node, true, false);
-- 2.30.0
Acked-by: Ilias Apalodimas ilias.apalodimas@linaro.org
participants (2)
-
Heinrich Schuchardt -
Ilias Apalodimas