From 8956d6d2adad36123df68a93604f19b38b7fa8e7 Mon Sep 17 00:00:00 2001
From: Janis Danisevskis <jdanisevskis@aurora.tech>
Date: Sat, 23 Nov 2024 11:55:08 -0800
Subject: [PATCH] Fix efi_bind_block.

efi_bind_block had two issues.
1. A pointer to a the stack was inserted as plat structure and thus used
beyond its life time.
2. Only the first segment of the device path was copied into the
platfom data structure resulting in an unterminated device path.

Signed-off-by: Janis Danisevskis <jdanisevskis@aurora.tech>
Signed-off-by: Matthew Garrett <mgarrett@aurora.tech>
---
 lib/efi/efi_app_init.c | 53 ++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/lib/efi/efi_app_init.c b/lib/efi/efi_app_init.c
index 9704020b749..f3ba82280f4 100644
--- a/lib/efi/efi_app_init.c
+++ b/lib/efi/efi_app_init.c
@@ -19,6 +19,15 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static size_t device_path_length(const struct efi_device_path *device_path)
+{
+	const struct efi_device_path *d;
+
+	for (d = device_path; d->type != DEVICE_PATH_TYPE_END; d = (void *)d + d->length) {
+	}
+	return (void *)d - (void *)device_path + d->length;
+}
+
 /**
  * efi_bind_block() - bind a new block device to an EFI device
  *
@@ -39,27 +48,47 @@ int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
 		   struct efi_device_path *device_path, int len,
 		   struct udevice **devp)
 {
-	struct efi_media_plat plat;
+	struct efi_media_plat *plat = NULL;
 	struct udevice *dev;
 	char name[18];
-	int ret;
-
-	plat.handle = handle;
-	plat.blkio = blkio;
-	plat.device_path = malloc(device_path->length);
-	if (!plat.device_path)
-		return log_msg_ret("path", -ENOMEM);
-	memcpy(plat.device_path, device_path, device_path->length);
+	int ret, err;
+	size_t device_path_len = device_path_length(device_path);
+
+	plat = malloc(sizeof(struct efi_media_plat));
+	if (!plat) {
+		return log_msg_ret("plat", -ENOMEM);
+	}
+	plat->handle = handle;
+	plat->blkio = blkio;
+	plat->device_path = malloc(device_path_len);
+	if (!plat->device_path) {
+		err = log_msg_ret("path", -ENOMEM);
+		goto err_out;
+	}
+	memcpy(plat->device_path, device_path, device_path_len);
 	ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media",
-			  &plat, ofnode_null(), &dev);
-	if (ret)
-		return log_msg_ret("bind", ret);
+			  plat, ofnode_null(), &dev);
+	if (ret) {
+		err = log_msg_ret("bind", ret);
+		goto err_out;
+	}
 
 	snprintf(name, sizeof(name), "efi_media_%x", dev_seq(dev));
 	device_set_name(dev, name);
 	*devp = dev;
 
 	return 0;
+
+err_out:
+	if (plat) {
+		// If plat is not NULL, plat->device_path
+		// is the return value of malloc. So it is
+		// either NULL or a valid pointer that needs
+		// to be freed.
+		free(plat->device_path);
+	}
+	free(plat);
+	return err;
 }
 
 /**
-- 
2.34.1

