[PATCH 0/4] efi_loader: validate device path length in boot manager
Bootxxxx variables are provided by the user and therefore cannot be trusted. We have to validate them before usage.
A device path provided by a Bootxxxx variable must have an end node within the indicated device path length.
* Provide function efi_dp_check_length() to check the length of device paths. * Provide a unit test of the function. * Use the function in the boot manager to check device paths.
Heinrich Schuchardt (4): include: kernel.h: define SSIZE_MAX efi_loader: efi_dp_check_length() test: unit test for efi_dp_check_length() efi_loader: validate device path length in boot manager
include/efi_loader.h | 2 ++ include/linux/kernel.h | 3 ++ lib/efi_loader/efi_bootmgr.c | 6 ++-- lib/efi_loader/efi_device_path.c | 33 +++++++++++++++++++++ test/lib/Makefile | 1 + test/lib/efi_device_path.c | 50 ++++++++++++++++++++++++++++++++ 6 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 test/lib/efi_device_path.c
-- 2.28.0
Define SSIZE_MAX, the largest value fitting into a variable of type ssize_t.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/linux/kernel.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h index b88c210065..3e71d61074 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -19,6 +19,9 @@ #ifndef SIZE_MAX #define SIZE_MAX (~(size_t)0) #endif +#ifndef SSIZE_MAX +#define SSIZE_MAX ((ssize_t)(SIZE_MAX >> 1)) +#endif
#define U8_MAX ((u8)~0U) #define S8_MAX ((s8)(U8_MAX>>1)) -- 2.28.0
We need to check that device paths provided via UEFI variables are not malformed.
Provide function efi_dp_check_length() to check if a device path has an end node within a given number of bytes.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 2 ++ lib/efi_loader/efi_device_path.c | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 50a17a33ca..0baa1d2324 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -631,6 +631,8 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr, const char *path, struct efi_device_path **device, struct efi_device_path **file); +ssize_t efi_dp_check_length(const struct efi_device_path *dp, + const size_t maxlen);
#define EFI_DP_TYPE(_dp, _type, _subtype) \ (((_dp)->type == DEVICE_PATH_TYPE_##_type) && \ diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 7ae14f3423..8a5c13c424 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -1127,3 +1127,36 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
return EFI_SUCCESS; } + +/** + * efi_dp_check_length() - check length of a device path + * + * @dp: pointer to device path + * @maxlen: maximum length of the device path + * Return: + * * length of the device path if it is less or equal @maxlen + * * -1 if the device path is longer then @maxlen + * * -1 if a device path node has a length of less than 4 + * * -EINVAL if maxlen exceeds SSIZE_MAX + */ +ssize_t efi_dp_check_length(const struct efi_device_path *dp, + const size_t maxlen) +{ + ssize_t ret = 0; + u16 len; + + if (maxlen > SSIZE_MAX) + return -EINVAL; + for (;;) { + len = dp->length; + if (len < 4) + return -1; + ret += len; + if (ret > maxlen) + return -1; + if (dp->type == DEVICE_PATH_TYPE_END && + dp->sub_type == DEVICE_PATH_SUB_TYPE_END) + return ret; + dp = (const struct efi_device_path *)((const u8 *)dp + len); + } +} -- 2.28.0
Provide a unit test for function efi_dp_check_length().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- test/lib/Makefile | 1 + test/lib/efi_device_path.c | 50 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 test/lib/efi_device_path.c
diff --git a/test/lib/Makefile b/test/lib/Makefile index b6a0a208c5..ada62fe46b 100644 --- a/test/lib/Makefile +++ b/test/lib/Makefile @@ -3,6 +3,7 @@ # (C) Copyright 2018 # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-y += cmd_ut_lib.o +obj-$(CONFIG_EFI_LOADER) += efi_device_path.o obj-$(CONFIG_EFI_SECURE_BOOT) += efi_image_region.o obj-y += hexdump.o obj-y += lmb.o diff --git a/test/lib/efi_device_path.c b/test/lib/efi_device_path.c new file mode 100644 index 0000000000..24e2f23c5a --- /dev/null +++ b/test/lib/efi_device_path.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device path functions + * + * Copyright (c) 2020 Heinrich Schuchardt xypron.glpk@gmx.de + */ + +#include <common.h> +#include <efi_loader.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h> + +static int lib_test_efi_dp_check_length(struct unit_test_state *uts) +{ + /* end of device path */ + u8 d1[] __aligned(2) = { + 0x7f, 0xff, 0x04, 0x00 }; + /* device path node with length less then 4 */ + u8 d2[] __aligned(2) = { + 0x01, 0x02, 0x02, 0x00, 0x04, 0x00, 0x7f, 0xff, 0x04, 0x00 }; + /* well formed device path */ + u8 d3[] __aligned(2) = { + 0x03, 0x02, 0x08, 0x00, 0x01, 0x00, 0x01, 0x00, + 0x7f, 0xff, 0x04, 0x00 }; + + struct efi_device_path *p1 = (struct efi_device_path *)d1; + struct efi_device_path *p2 = (struct efi_device_path *)d2; + struct efi_device_path *p3 = (struct efi_device_path *)d3; + + ut_asserteq((ssize_t)-EINVAL, efi_dp_check_length(p1, SIZE_MAX)); + ut_asserteq((ssize_t)sizeof(d1), efi_dp_check_length(p1, sizeof(d1))); + ut_asserteq((ssize_t)sizeof(d1), + efi_dp_check_length(p1, sizeof(d1) + 4)); + ut_asserteq((ssize_t)-1, efi_dp_check_length(p1, sizeof(d1) - 1)); + + ut_asserteq((ssize_t)-1, efi_dp_check_length(p2, sizeof(d2))); + + ut_asserteq((ssize_t)-1, efi_dp_check_length(p3, sizeof(d3) - 1)); + ut_asserteq((ssize_t)sizeof(d3), efi_dp_check_length(p3, sizeof(d3))); + ut_asserteq((ssize_t)sizeof(d3), efi_dp_check_length(p3, SSIZE_MAX)); + ut_asserteq((ssize_t)-EINVAL, + efi_dp_check_length(p3, (size_t)SSIZE_MAX + 1)); + ut_asserteq((ssize_t)sizeof(d3), + efi_dp_check_length(p3, sizeof(d3) + 4)); + + return 0; +} + +LIB_TEST(lib_test_efi_dp_check_length, 0); -- 2.28.0
Bootxxxx variables are provided by the user and therefore cannot be trusted. We have to validate them before usage.
A device path provided by a Bootxxxx variable must have an end node within the indicated device path length.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_bootmgr.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 1e06e60963..61dc72a23d 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -105,10 +105,8 @@ efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data, if (*size < len) return EFI_INVALID_PARAMETER; lo->file_path = (struct efi_device_path *)data; - /* - * TODO: validate device path. There should be an end node within - * the indicated file_path_length. - */ + if (efi_dp_check_length(lo->file_path, len) < 0) + return EFI_INVALID_PARAMETER; data += len; *size -= len;
-- 2.28.0
participants (1)
-
Heinrich Schuchardt