Merge pull request #28069 from medhefgo/boot-dp

boot: device_path_to_str improvements

src/boot/efi/device-path-util.c

@@ -38,6 +38,44 @@ EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DE
         return EFI_SUCCESS;
 }
 
+static char16_t *device_path_to_str_internal(const EFI_DEVICE_PATH *dp) {
+        char16_t *str = 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;
+
+                if (node->Type == END_DEVICE_PATH_TYPE && node->SubType == END_INSTANCE_DEVICE_PATH_SUBTYPE) {
+                        str = xasprintf("%ls%s,", strempty(old), old ? "\\" : "");
+                        continue;
+                }
+
+                /* 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;
+                }
+
+                /* 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 str;
+}
+
 EFI_STATUS device_path_to_str(const EFI_DEVICE_PATH *dp, char16_t **ret) {
         EFI_DEVICE_PATH_TO_TEXT_PROTOCOL *dp_to_text;
         EFI_STATUS err;
@@ -48,35 +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. */
-
-                size_t size = 0;
-                for (const EFI_DEVICE_PATH *node = dp; !device_path_is_end(node);
-                     node = device_path_next_node(node)) {
-
-                        if (node->Type != MEDIA_DEVICE_PATH || node->SubType != MEDIA_FILEPATH_DP)
-                                return err;
-
-                        size_t path_size = node->Length;
-                        if (path_size <= offsetof(FILEPATH_DEVICE_PATH, PathName) || path_size % sizeof(char16_t))
-                                return EFI_INVALID_PARAMETER;
-                        path_size -= offsetof(FILEPATH_DEVICE_PATH, PathName);
-
-                        _cleanup_free_ char16_t *old = str;
-                        str = xmalloc(size + path_size);
-                        if (old) {
-                                memcpy(str, old, size);
-                                str[size / sizeof(char16_t) - 1] = '\\';
-                        }
-
-                        memcpy(str + (size / sizeof(char16_t)),
-                               ((uint8_t *) node) + offsetof(FILEPATH_DEVICE_PATH, PathName),
-                               path_size);
-                        size += path_size;
-                }
-
-                *ret = TAKE_PTR(str);
+                *ret = device_path_to_str_internal(dp);
                 return EFI_SUCCESS;
         }
 

src/boot/efi/efi-string.c

@@ -391,6 +391,23 @@ bool efi_fnmatch(const char16_t *pattern, const char16_t *haystack) {
 DEFINE_PARSE_NUMBER(char, parse_number8);
 DEFINE_PARSE_NUMBER(char16_t, parse_number16);
 
+char16_t *hexdump(const void *data, size_t size) {
+        static const char hex[16] = "0123456789abcdef";
+        const uint8_t *d = data;
+
+        assert(data || size == 0);
+
+        char16_t *buf = xnew(char16_t, size * 2 + 1);
+
+        for (size_t i = 0; i < size; i++) {
+                buf[i * 2] = hex[d[i] >> 4];
+                buf[i * 2 + 1] = hex[d[i] & 0x0F];
+        }
+
+        buf[size * 2] = 0;
+        return buf;
+}
+
 static const char * const warn_table[] = {
         [EFI_SUCCESS]               = "Success",
         [EFI_WARN_UNKNOWN_GLYPH]    = "Unknown glyph",

src/boot/efi/efi-string.h

@@ -108,6 +108,8 @@ bool efi_fnmatch(const char16_t *pattern, const char16_t *haystack);
 bool parse_number8(const char *s, uint64_t *ret_u, const char **ret_tail);
 bool parse_number16(const char16_t *s, uint64_t *ret_u, const char16_t **ret_tail);
 
+char16_t *hexdump(const void *data, size_t size);
+
 #ifdef __clang__
 #  define _gnu_printf_(a, b) _printf_(a, b)
 #else

src/boot/efi/log.c

@@ -3,6 +3,7 @@
 #include "log.h"
 #include "proto/rng.h"
 #include "proto/simple-text-io.h"
+#include "util.h"
 
 static unsigned log_count = 0;
 
@@ -52,6 +53,15 @@ EFI_STATUS log_internal(EFI_STATUS status, const char *format, ...) {
         return status;
 }
 
+#ifdef EFI_DEBUG
+void log_hexdump(const char16_t *prefix, const void *data, size_t size) {
+        /* Debugging helper — please keep this around, even if not used */
+
+        _cleanup_free_ char16_t *hex = hexdump(data, size);
+        log_internal(EFI_SUCCESS, "%ls[%zu]: %ls", prefix, size, hex);
+}
+#endif
+
 void log_wait(void) {
         if (log_count == 0)
                 return;

src/boot/efi/log.h

@@ -26,3 +26,7 @@ _gnu_printf_(2, 3) EFI_STATUS log_internal(EFI_STATUS status, const char *format
 #define log_error(...) log_internal(EFI_INVALID_PARAMETER, __VA_ARGS__)
 #define log_oom() log_internal(EFI_OUT_OF_RESOURCES, "Out of memory.")
 #define log_trace() log_internal(EFI_SUCCESS, "%s:%i@%s", __FILE__, __LINE__, __func__)
+
+#ifdef EFI_DEBUG
+void log_hexdump(const char16_t *prefix, const void *data, size_t size);
+#endif

src/boot/efi/test-efi-string.c

@@ -484,6 +484,26 @@ TEST(parse_number16) {
         assert_se(streq16(tail, u"rest"));
 }
 
+TEST(test_hexdump) {
+        char16_t *hex;
+
+        hex = hexdump(NULL, 0);
+        assert(streq16(hex, u""));
+        free(hex);
+
+        hex = hexdump("1", 2);
+        assert(streq16(hex, u"3100"));
+        free(hex);
+
+        hex = hexdump("abc", 4);
+        assert(streq16(hex, u"61626300"));
+        free(hex);
+
+        hex = hexdump((uint8_t[]){ 0x0, 0x42, 0xFF, 0xF1, 0x1F }, 5);
+        assert(streq16(hex, u"0042fff11f"));
+        free(hex);
+}
+
 _printf_(1, 2) static void test_printf_one(const char *format, ...) {
         va_list ap, ap_efi;
         va_start(ap, format);

src/boot/efi/util.c

@@ -568,30 +568,6 @@ __attribute__((noinline)) void notify_debugger(const char *identity, volatile bo
 #endif
 }
 
-#ifdef EFI_DEBUG
-void hexdump(const char16_t *prefix, const void *data, size_t size) {
-        static const char hex[16] = "0123456789abcdef";
-        _cleanup_free_ char16_t *buf = NULL;
-        const uint8_t *d = data;
-
-        assert(prefix);
-        assert(data || size == 0);
-
-        /* Debugging helper — please keep this around, even if not used */
-
-        buf = xnew(char16_t, size*2+1);
-
-        for (size_t i = 0; i < size; i++) {
-                buf[i*2] = hex[d[i] >> 4];
-                buf[i*2+1] = hex[d[i] & 0x0F];
-        }
-
-        buf[size*2] = 0;
-
-        log_error("%ls[%zu]: %ls", prefix, size, buf);
-}
-#endif
-
 #if defined(__i386__) || defined(__x86_64__)
 static inline uint8_t inb(uint16_t port) {
         uint8_t value;

src/boot/efi/util.h

@@ -167,10 +167,6 @@ uint64_t get_os_indications_supported(void);
  * be attached. See debug-sd-boot.sh for how this can be done. */
 void notify_debugger(const char *identity, bool wait);
 
-#ifdef EFI_DEBUG
-void hexdump(const char16_t *prefix, const void *data, size_t size);
-#endif
-
 /* On x86 the compiler assumes a different incoming stack alignment than what we get.
  * This will cause long long variables to be misaligned when building with
  * '-mlong-double' (for correct struct layouts). Normally, the compiler realigns the