From: Jan Janssen <medhefgo@web.de>
Date: Sun, 18 Jun 2023 08:54:20 +0000 (+0200)
Subject: boot: Improve device_path_to_str_internal()
X-Git-Tag: v254-rc1~181^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F28069%2Fhead;p=thirdparty%2Fsystemd.git

boot: Improve device_path_to_str_internal()

The UEFI spec has a generic `Path` node representation that can be used
for device path nodes that are unknown. So we can use that instead of
giving up when we see a node other than FilePath.

This also simplifies the FilePath case by just using xasprintf(). The
code is really just a fallback for silly firmware that does not
implement EFI_DEVICE_PATH_TO_TEXT_PROTOCOL (looking at you, Apple).

The correctness of this was tested by round-tripping it through
EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL, which yielded an identical device
compared to our input path.
---

diff --git a/src/boot/efi/device-path-util.c b/src/boot/efi/device-path-util.c
index 203bdd5e654..2a85e8bbfc0 100644
--- a/src/boot/efi/device-path-util.c
+++ b/src/boot/efi/device-path-util.c
@@ -39,34 +39,41 @@ EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DE
 }
 
 static char16_t *device_path_to_str_internal(const EFI_DEVICE_PATH *dp) {
-        _cleanup_free_ char16_t *str = NULL;
-        size_t size = 0;
-
-        for (const EFI_DEVICE_PATH *node = dp; !device_path_is_end(node);
-                node = device_path_next_node(node)) {
+        char16_t *str = NULL;
 
-                if (node->Type != MEDIA_DEVICE_PATH || node->SubType != MEDIA_FILEPATH_DP)
-                        return NULL;
+        for (const EFI_DEVICE_PATH *node = dp; !device_path_is_end(node); node = device_path_next_node(node)) {
+                _cleanup_free_ char16_t *old = str;
 
-                size_t path_size = node->Length;
-                if (path_size <= offsetof(FILEPATH_DEVICE_PATH, PathName) || path_size % sizeof(char16_t))
-                        return NULL;
-                path_size -= offsetof(FILEPATH_DEVICE_PATH, PathName);
+                if (node->Type == END_DEVICE_PATH_TYPE && node->SubType == END_INSTANCE_DEVICE_PATH_SUBTYPE) {
+                        str = xasprintf("%ls%s,", strempty(old), old ? "\\" : "");
+                        continue;
+                }
 
-                _cleanup_free_ char16_t *old = str;
-                str = xmalloc(size + path_size);
-                if (old) {
-                        memcpy(str, old, size);
-                        str[size / sizeof(char16_t) - 1] = '\\';
+                /* Special-case this so that FilePath-only device path string look and behave nicely. */
+                if (node->Type == MEDIA_DEVICE_PATH && node->SubType == MEDIA_FILEPATH_DP) {
+                        str = xasprintf("%ls%s%ls",
+                                        strempty(old),
+                                        old ? "\\" : "",
+                                        ((FILEPATH_DEVICE_PATH *) node)->PathName);
+                        continue;
                 }
 
-                memcpy(str + (size / sizeof(char16_t)),
-                        ((uint8_t *) node) + offsetof(FILEPATH_DEVICE_PATH, PathName),
-                        path_size);
-                size += path_size;
+                /* Instead of coding all the different types and sub-types here we just use the
+                 * generic node form. This function is a best-effort for firmware that does not
+                 * provide the EFI_DEVICE_PATH_TO_TEXT_PROTOCOL after all. */
+
+                size_t size = node->Length - sizeof(EFI_DEVICE_PATH);
+                _cleanup_free_ char16_t *hex_data = hexdump((uint8_t *) node + sizeof(EFI_DEVICE_PATH), size);
+                str = xasprintf("%ls%sPath(%u,%u%s%ls)",
+                                strempty(old),
+                                old ? "/" : "",
+                                node->Type,
+                                node->SubType,
+                                size == 0 ? "" : ",",
+                                hex_data);
         }
 
-        return TAKE_PTR(str);
+        return str;
 }
 
 EFI_STATUS device_path_to_str(const EFI_DEVICE_PATH *dp, char16_t **ret) {
@@ -79,13 +86,7 @@ EFI_STATUS device_path_to_str(const EFI_DEVICE_PATH *dp, char16_t **ret) {
 
         err = BS->LocateProtocol(MAKE_GUID_PTR(EFI_DEVICE_PATH_TO_TEXT_PROTOCOL), NULL, (void **) &dp_to_text);
         if (err != EFI_SUCCESS) {
-                /* If the device path to text protocol is not available we can still do a best-effort attempt
-                 * to convert it ourselves if we are given filepath-only device path. */
-                str = device_path_to_str_internal(dp);
-                if (!str)
-                        return err;
-
-                *ret = TAKE_PTR(str);
+                *ret = device_path_to_str_internal(dp);
                 return EFI_SUCCESS;
         }