[PATCH 2/2] efi_driver: fix error handling

[Heinrich Schuchardt]

If creating the block device fails,

* delete all created objects and references
* close the protocol interface on the controller

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
---
 include/efi_driver.h              |  2 +-
 lib/efi_driver/efi_block_device.c | 60 +++++++++++++++++--------------
 lib/efi_driver/efi_uclass.c       | 23 ++++++------
 3 files changed, 46 insertions(+), 39 deletions(-)

diff --git a/include/efi_driver.h b/include/efi_driver.h
index 2b62219c5b..dc0c1c7ac0 100644
--- a/include/efi_driver.h
+++ b/include/efi_driver.h
@@ -25,7 +25,7 @@
 struct efi_driver_ops {
 	const efi_guid_t *protocol;
 	const efi_guid_t *child_protocol;
-	int (*bind)(efi_handle_t handle, void *interface);
+	efi_status_t (*bind)(efi_handle_t handle, void *interface);
 };
 
 /*
diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
index 3177ab67b8..9ccc148590 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -112,12 +112,13 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
  *
  * @handle:	handle
  * @interface:	block io protocol
- * Return:	0 = success
+ * Return:	status code
  */
-static int efi_bl_bind(efi_handle_t handle, void *interface)
+static efi_status_t efi_bl_bind(efi_handle_t handle, void *interface)
 {
-	struct udevice *bdev, *parent = dm_root();
-	int ret, devnum;
+	struct udevice *bdev = NULL, *parent = dm_root();
+	efi_status_t ret;
+	int devnum;
 	char *name;
 	struct efi_object *obj = efi_search_obj(handle);
 	struct efi_block_io *io = interface;
@@ -125,28 +126,28 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
 
 	EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io);
 
-	if (!obj)
-		return -ENOENT;
+	if (!obj || !interface)
+		return EFI_INVALID_PARAMETER;
 
 	devnum = blk_find_max_devnum(UCLASS_EFI_LOADER);
 	if (devnum == -ENODEV)
 		devnum = 0;
 	else if (devnum < 0)
-		return devnum;
+		return EFI_OUT_OF_RESOURCES;
 
 	name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
 	if (!name)
-		return -ENOMEM;
+		return EFI_OUT_OF_RESOURCES;
 	sprintf(name, "efiblk#%d", devnum);
 
 	/* Create driver model udevice for the EFI block io device */
-	ret = blk_create_device(parent, "efi_blk", name, UCLASS_EFI_LOADER,
-				devnum, io->media->block_size,
-				(lbaint_t)io->media->last_block, &bdev);
-	if (ret)
-		return ret;
-	if (!bdev)
-		return -ENOENT;
+	if (blk_create_device(parent, "efi_blk", name, UCLASS_EFI_LOADER,
+			      devnum, io->media->block_size,
+			      (lbaint_t)io->media->last_block, &bdev)) {
+		ret = EFI_OUT_OF_RESOURCES;
+		free(name);
+		goto err;
+	}
 	/* Set the DM_FLAG_NAME_ALLOCED flag to avoid a memory leak */
 	device_set_name_alloced(bdev);
 
@@ -154,20 +155,25 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
 	plat->handle = handle;
 	plat->io = interface;
 
-	/*
-	 * FIXME: necessary because we won't do almost nothing in
-	 * efi_disk_create() when called from device_probe().
-	 */
-	if (efi_link_dev(handle, bdev))
-		/* FIXME: cleanup for bdev */
-		return ret;
-
-	ret = device_probe(bdev);
-	if (ret)
-		return ret;
+	if (efi_link_dev(handle, bdev)) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto err;
+	}
+
+	if (device_probe(bdev)) {
+		ret = EFI_DEVICE_ERROR;
+		goto err;
+	}
 	EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name);
 
-	return 0;
+	return EFI_SUCCESS;
+
+err:
+	efi_unlink_dev(handle);
+	if (bdev)
+		device_unbind(bdev);
+
+	return ret;
 }
 
 /* Block device driver operators */
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
index 74dd003243..5a285aad89 100644
--- a/lib/efi_driver/efi_uclass.c
+++ b/lib/efi_driver/efi_uclass.c
@@ -11,7 +11,7 @@
  * The uclass provides the bind, start, and stop entry points for the driver
  * binding protocol.
  *
- * In bind() and stop() it checks if the controller implements the protocol
+ * In supported() and bind() it checks if the controller implements the protocol
  * supported by the EFI driver. In the start() function it calls the bind()
  * function of the EFI driver. In the stop() function it destroys the child
  * controllers.
@@ -144,18 +144,19 @@ static efi_status_t EFIAPI efi_uc_start(
 		goto out;
 	}
 	ret = check_node_type(controller_handle);
-	if (ret != EFI_SUCCESS) {
-		r = EFI_CALL(systab.boottime->close_protocol(
-				controller_handle, bp->ops->protocol,
-				this->driver_binding_handle,
-				controller_handle));
-		if (r != EFI_SUCCESS)
-			EFI_PRINT("Failure to close handle\n");
+	if (ret != EFI_SUCCESS)
+		goto err;
+	ret = bp->ops->bind(controller_handle, interface);
+	if (ret == EFI_SUCCESS)
 		goto out;
-	}
 
-	/* TODO: driver specific stuff */
-	bp->ops->bind(controller_handle, interface);
+err:
+	r = EFI_CALL(systab.boottime->close_protocol(
+			controller_handle, bp->ops->protocol,
+			this->driver_binding_handle,
+			controller_handle));
+	if (r != EFI_SUCCESS)
+		EFI_PRINT("Failure to close handle\n");
 
 out:
 	return EFI_EXIT(ret);
-- 
2.37.2