[PATCH 0/4] efi_loader: validate device path length in boot manager

[PATCH 0/4] efi_loader: validate device path length in boot manager

[Heinrich Schuchardt]

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

[PATCH 1/4] include: kernel.h: define SSIZE_MAX

[Heinrich Schuchardt]

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

[PATCH 2/4] efi_loader: efi_dp_check_length()

[Heinrich Schuchardt]

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

[PATCH 3/4] test: unit test for efi_dp_check_length()

[Heinrich Schuchardt]

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 at 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

[PATCH 4/4] efi_loader: validate device path length in boot manager

[Heinrich Schuchardt]

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