From: Marta Rybczynska <rybczyn...@gmail.com> This change fixes the malformed device paths in EFI handling. Device paths of length 4 or shorter could cause different kinds of unexpected behaviours.
This patch is NOT a part of [1], but is a dependency of one of the patches included in the series. [1] https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00007.html Signed-off-by: Marta Rybczynska <marta.rybczyn...@huawei.com> Signed-off-by: Steve Sakoman <st...@sakoman.com> --- ...formed-device-path-arithmetic-errors.patch | 235 ++++++++++++++++++ meta/recipes-bsp/grub/grub2.inc | 1 + 2 files changed, 236 insertions(+) create mode 100644 meta/recipes-bsp/grub/files/0005-efi-Fix-some-malformed-device-path-arithmetic-errors.patch diff --git a/meta/recipes-bsp/grub/files/0005-efi-Fix-some-malformed-device-path-arithmetic-errors.patch b/meta/recipes-bsp/grub/files/0005-efi-Fix-some-malformed-device-path-arithmetic-errors.patch new file mode 100644 index 0000000000..04748befc8 --- /dev/null +++ b/meta/recipes-bsp/grub/files/0005-efi-Fix-some-malformed-device-path-arithmetic-errors.patch @@ -0,0 +1,235 @@ +From 16a4d739b19f8680cf93a3c8fa0ae9fc1b1c310b Mon Sep 17 00:00:00 2001 +From: Peter Jones <pjo...@redhat.com> +Date: Sun, 19 Jul 2020 16:53:27 -0400 +Subject: [PATCH] efi: Fix some malformed device path arithmetic errors + +Several places we take the length of a device path and subtract 4 from +it, without ever checking that it's >= 4. There are also cases where +this kind of malformation will result in unpredictable iteration, +including treating the length from one dp node as the type in the next +node. These are all errors, no matter where the data comes from. + +This patch adds a checking macro, GRUB_EFI_DEVICE_PATH_VALID(), which +can be used in several places, and makes GRUB_EFI_NEXT_DEVICE_PATH() +return NULL and GRUB_EFI_END_ENTIRE_DEVICE_PATH() evaluate as true when +the length is too small. Additionally, it makes several places in the +code check for and return errors in these cases. + +Signed-off-by: Peter Jones <pjo...@redhat.com> +Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> + +Upstream-Status: Backport [https://git.savannah.gnu.org/cgit/grub.git/commit/?id=d2cf823d0e31818d1b7a223daff6d5e006596543] +Signed-off-by: Marta Rybczynska <marta.rybczyn...@huawei.com> +--- + grub-core/kern/efi/efi.c | 64 +++++++++++++++++++++++++----- + grub-core/loader/efi/chainloader.c | 13 +++++- + grub-core/loader/i386/xnu.c | 9 +++-- + include/grub/efi/api.h | 14 ++++--- + 4 files changed, 79 insertions(+), 21 deletions(-) + +diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c +index ad170c7..6a38080 100644 +--- a/grub-core/kern/efi/efi.c ++++ b/grub-core/kern/efi/efi.c +@@ -360,7 +360,7 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0) + + dp = dp0; + +- while (1) ++ while (dp) + { + grub_efi_uint8_t type = GRUB_EFI_DEVICE_PATH_TYPE (dp); + grub_efi_uint8_t subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (dp); +@@ -370,9 +370,15 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0) + if (type == GRUB_EFI_MEDIA_DEVICE_PATH_TYPE + && subtype == GRUB_EFI_FILE_PATH_DEVICE_PATH_SUBTYPE) + { +- grub_efi_uint16_t len; +- len = ((GRUB_EFI_DEVICE_PATH_LENGTH (dp) - 4) +- / sizeof (grub_efi_char16_t)); ++ grub_efi_uint16_t len = GRUB_EFI_DEVICE_PATH_LENGTH (dp); ++ ++ if (len < 4) ++ { ++ grub_error (GRUB_ERR_OUT_OF_RANGE, ++ "malformed EFI Device Path node has length=%d", len); ++ return NULL; ++ } ++ len = (len - 4) / sizeof (grub_efi_char16_t); + filesize += GRUB_MAX_UTF8_PER_UTF16 * len + 2; + } + +@@ -388,7 +394,7 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0) + if (!name) + return NULL; + +- while (1) ++ while (dp) + { + grub_efi_uint8_t type = GRUB_EFI_DEVICE_PATH_TYPE (dp); + grub_efi_uint8_t subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (dp); +@@ -404,8 +410,15 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0) + + *p++ = '/'; + +- len = ((GRUB_EFI_DEVICE_PATH_LENGTH (dp) - 4) +- / sizeof (grub_efi_char16_t)); ++ len = GRUB_EFI_DEVICE_PATH_LENGTH (dp); ++ if (len < 4) ++ { ++ grub_error (GRUB_ERR_OUT_OF_RANGE, ++ "malformed EFI Device Path node has length=%d", len); ++ return NULL; ++ } ++ ++ len = (len - 4) / sizeof (grub_efi_char16_t); + fp = (grub_efi_file_path_device_path_t *) dp; + /* According to EFI spec Path Name is NULL terminated */ + while (len > 0 && fp->path_name[len - 1] == 0) +@@ -480,7 +493,26 @@ grub_efi_duplicate_device_path (const grub_efi_device_path_t *dp) + ; + p = GRUB_EFI_NEXT_DEVICE_PATH (p)) + { +- total_size += GRUB_EFI_DEVICE_PATH_LENGTH (p); ++ grub_size_t len = GRUB_EFI_DEVICE_PATH_LENGTH (p); ++ ++ /* ++ * In the event that we find a node that's completely garbage, for ++ * example if we get to 0x7f 0x01 0x02 0x00 ... (EndInstance with a size ++ * of 2), GRUB_EFI_END_ENTIRE_DEVICE_PATH() will be true and ++ * GRUB_EFI_NEXT_DEVICE_PATH() will return NULL, so we won't continue, ++ * and neither should our consumers, but there won't be any error raised ++ * even though the device path is junk. ++ * ++ * This keeps us from passing junk down back to our caller. ++ */ ++ if (len < 4) ++ { ++ grub_error (GRUB_ERR_OUT_OF_RANGE, ++ "malformed EFI Device Path node has length=%d", len); ++ return NULL; ++ } ++ ++ total_size += len; + if (GRUB_EFI_END_ENTIRE_DEVICE_PATH (p)) + break; + } +@@ -525,7 +557,7 @@ dump_vendor_path (const char *type, grub_efi_vendor_device_path_t *vendor) + void + grub_efi_print_device_path (grub_efi_device_path_t *dp) + { +- while (1) ++ while (GRUB_EFI_DEVICE_PATH_VALID (dp)) + { + grub_efi_uint8_t type = GRUB_EFI_DEVICE_PATH_TYPE (dp); + grub_efi_uint8_t subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (dp); +@@ -937,7 +969,10 @@ grub_efi_compare_device_paths (const grub_efi_device_path_t *dp1, + /* Return non-zero. */ + return 1; + +- while (1) ++ if (dp1 == dp2) ++ return 0; ++ ++ while (GRUB_EFI_DEVICE_PATH_VALID (dp1) && GRUB_EFI_DEVICE_PATH_VALID (dp2)) + { + grub_efi_uint8_t type1, type2; + grub_efi_uint8_t subtype1, subtype2; +@@ -973,5 +1008,14 @@ grub_efi_compare_device_paths (const grub_efi_device_path_t *dp1, + dp2 = (grub_efi_device_path_t *) ((char *) dp2 + len2); + } + ++ /* ++ * There's no "right" answer here, but we probably don't want to call a valid ++ * dp and an invalid dp equal, so pick one way or the other. ++ */ ++ if (GRUB_EFI_DEVICE_PATH_VALID (dp1) && !GRUB_EFI_DEVICE_PATH_VALID (dp2)) ++ return 1; ++ else if (!GRUB_EFI_DEVICE_PATH_VALID (dp1) && GRUB_EFI_DEVICE_PATH_VALID (dp2)) ++ return -1; ++ + return 0; + } +diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c +index daf8c6b..a8d7b91 100644 +--- a/grub-core/loader/efi/chainloader.c ++++ b/grub-core/loader/efi/chainloader.c +@@ -156,9 +156,18 @@ make_file_path (grub_efi_device_path_t *dp, const char *filename) + + size = 0; + d = dp; +- while (1) ++ while (d) + { +- size += GRUB_EFI_DEVICE_PATH_LENGTH (d); ++ grub_size_t len = GRUB_EFI_DEVICE_PATH_LENGTH (d); ++ ++ if (len < 4) ++ { ++ grub_error (GRUB_ERR_OUT_OF_RANGE, ++ "malformed EFI Device Path node has length=%d", len); ++ return NULL; ++ } ++ ++ size += len; + if ((GRUB_EFI_END_ENTIRE_DEVICE_PATH (d))) + break; + d = GRUB_EFI_NEXT_DEVICE_PATH (d); +diff --git a/grub-core/loader/i386/xnu.c b/grub-core/loader/i386/xnu.c +index b7d176b..c50cb54 100644 +--- a/grub-core/loader/i386/xnu.c ++++ b/grub-core/loader/i386/xnu.c +@@ -516,14 +516,15 @@ grub_cmd_devprop_load (grub_command_t cmd __attribute__ ((unused)), + + devhead = buf; + buf = devhead + 1; +- dpstart = buf; ++ dp = dpstart = buf; + +- do ++ while (GRUB_EFI_DEVICE_PATH_VALID (dp) && buf < bufend) + { +- dp = buf; + buf = (char *) buf + GRUB_EFI_DEVICE_PATH_LENGTH (dp); ++ if (GRUB_EFI_END_ENTIRE_DEVICE_PATH (dp)) ++ break; ++ dp = buf; + } +- while (!GRUB_EFI_END_ENTIRE_DEVICE_PATH (dp) && buf < bufend); + + dev = grub_xnu_devprop_add_device (dpstart, (char *) buf + - (char *) dpstart); +diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h +index addcbfa..cf1355a 100644 +--- a/include/grub/efi/api.h ++++ b/include/grub/efi/api.h +@@ -625,6 +625,7 @@ typedef struct grub_efi_device_path grub_efi_device_path_protocol_t; + #define GRUB_EFI_DEVICE_PATH_TYPE(dp) ((dp)->type & 0x7f) + #define GRUB_EFI_DEVICE_PATH_SUBTYPE(dp) ((dp)->subtype) + #define GRUB_EFI_DEVICE_PATH_LENGTH(dp) ((dp)->length) ++#define GRUB_EFI_DEVICE_PATH_VALID(dp) ((dp) != NULL && GRUB_EFI_DEVICE_PATH_LENGTH (dp) >= 4) + + /* The End of Device Path nodes. */ + #define GRUB_EFI_END_DEVICE_PATH_TYPE (0xff & 0x7f) +@@ -633,13 +634,16 @@ typedef struct grub_efi_device_path grub_efi_device_path_protocol_t; + #define GRUB_EFI_END_THIS_DEVICE_PATH_SUBTYPE 0x01 + + #define GRUB_EFI_END_ENTIRE_DEVICE_PATH(dp) \ +- (GRUB_EFI_DEVICE_PATH_TYPE (dp) == GRUB_EFI_END_DEVICE_PATH_TYPE \ +- && (GRUB_EFI_DEVICE_PATH_SUBTYPE (dp) \ +- == GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE)) ++ (!GRUB_EFI_DEVICE_PATH_VALID (dp) || \ ++ (GRUB_EFI_DEVICE_PATH_TYPE (dp) == GRUB_EFI_END_DEVICE_PATH_TYPE \ ++ && (GRUB_EFI_DEVICE_PATH_SUBTYPE (dp) \ ++ == GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE))) + + #define GRUB_EFI_NEXT_DEVICE_PATH(dp) \ +- ((grub_efi_device_path_t *) ((char *) (dp) \ +- + GRUB_EFI_DEVICE_PATH_LENGTH (dp))) ++ (GRUB_EFI_DEVICE_PATH_VALID (dp) \ ++ ? ((grub_efi_device_path_t *) \ ++ ((char *) (dp) + GRUB_EFI_DEVICE_PATH_LENGTH (dp))) \ ++ : NULL) + + /* Hardware Device Path. */ + #define GRUB_EFI_HARDWARE_DEVICE_PATH_TYPE 1 diff --git a/meta/recipes-bsp/grub/grub2.inc b/meta/recipes-bsp/grub/grub2.inc index 2e4e6d7ac2..f7f2aa892f 100644 --- a/meta/recipes-bsp/grub/grub2.inc +++ b/meta/recipes-bsp/grub/grub2.inc @@ -51,6 +51,7 @@ SRC_URI = "${GNU_MIRROR}/grub/grub-${PV}.tar.gz \ file://0002-net-net-Fix-possible-dereference-to-of-a-NULL-pointe.patch \ file://0003-net-tftp-Fix-dangling-memory-pointer.patch \ file://0004-kern-parser-Fix-resource-leak-if-argc-0.patch \ + file://0005-efi-Fix-some-malformed-device-path-arithmetic-errors.patch \ " SRC_URI[md5sum] = "5ce674ca6b2612d8939b9e6abed32934" SRC_URI[sha256sum] = "f10c85ae3e204dbaec39ae22fa3c5e99f0665417e91c2cb49b7e5031658ba6ea" -- 2.25.1
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#162355): https://lists.openembedded.org/g/openembedded-core/message/162355 Mute This Topic: https://lists.openembedded.org/mt/89388996/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
