[PATCH 1/2] boot: android: fix AvbOps and verify-data leaks in AVB path

[Igor Opaniuk]

run_avb_verification() allocates an AvbOps via avb_ops_alloc() but never
frees it on any return path. Every Android boot attempt therefore leaks
the AvbOpsData structure and, when CONFIG_OPTEE_TA_AVB is enabled, leaves
the OP-TEE session open (it is only closed inside avb_ops_free()).

In addition, the AvbSlotVerifyData returned by avb_slot_verify() is only
released on the failure branches. The successful "return 0" paths (both
the locked GREEN/OK case and the unlocked ORANGE/ERROR_VERIFICATION case)
return without freeing it, leaking the whole out_data (cmdline and loaded
partition metadata) on every good boot.

Route all exit paths through a single cleanup label that frees both
out_data and avb_ops.

Fixes: 125d9f3306ea ("bootstd: Add a bootmeth for Android")
Signed-off-by: Igor Opaniuk <igor.opaniuk at gmail.com>
---
 boot/bootmeth_android.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
index 1d70e8d5c05..ec255b072af 100644
--- a/boot/bootmeth_android.c
+++ b/boot/bootmeth_android.c
@@ -428,7 +428,7 @@ static int run_avb_verification(struct bootflow *bflow)
 	const char * const requested_partitions[] = {"boot", "vendor_boot", NULL};
 	struct AvbOps *avb_ops;
 	AvbSlotVerifyResult result;
-	AvbSlotVerifyData *out_data;
+	AvbSlotVerifyData *out_data = NULL;
 	enum avb_boot_state boot_state;
 	char *extra_args;
 	char slot_suffix[3] = "";
@@ -443,8 +443,10 @@ static int run_avb_verification(struct bootflow *bflow)
 		sprintf(slot_suffix, "_%s", priv->slot);
 
 	ret = avb_ops->read_is_device_unlocked(avb_ops, &unlocked);
-	if (ret != AVB_IO_RESULT_OK)
-		return log_msg_ret("avb lock", -EIO);
+	if (ret != AVB_IO_RESULT_OK) {
+		ret = log_msg_ret("avb lock", -EIO);
+		goto out;
+	}
 
 	result = avb_slot_verify(avb_ops,
 				 requested_partitions,
@@ -458,9 +460,8 @@ static int run_avb_verification(struct bootflow *bflow)
 		if (result != AVB_SLOT_VERIFY_RESULT_OK) {
 			printf("Verification failed, reason: %s\n",
 			       str_avb_slot_error(result));
-			if (out_data)
-				avb_slot_verify_data_free(out_data);
-			return log_msg_ret("avb verify", -EIO);
+			ret = log_msg_ret("avb verify", -EIO);
+			goto out;
 		}
 		boot_state = AVB_GREEN;
 	} else {
@@ -469,9 +470,8 @@ static int run_avb_verification(struct bootflow *bflow)
 		    result != AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION) {
 			printf("Unlocked verification failed, reason: %s\n",
 			       str_avb_slot_error(result));
-			if (out_data)
-				avb_slot_verify_data_free(out_data);
-			return log_msg_ret("avb verify unlocked", -EIO);
+			ret = log_msg_ret("avb verify unlocked", -EIO);
+			goto out;
 		}
 		boot_state = AVB_ORANGE;
 	}
@@ -480,23 +480,28 @@ static int run_avb_verification(struct bootflow *bflow)
 	if (extra_args) {
 		/* extra_args will be modified after this. This is fine */
 		ret = avb_append_commandline_arg(bflow, extra_args);
-		if (ret < 0)
-			goto free_out_data;
+		if (ret < 0) {
+			ret = log_msg_ret("avb cmdline", ret);
+			goto out;
+		}
 	}
 
 	if (result == AVB_SLOT_VERIFY_RESULT_OK) {
 		ret = avb_append_commandline(bflow, out_data->cmdline);
-		if (ret < 0)
-			goto free_out_data;
+		if (ret < 0) {
+			ret = log_msg_ret("avb cmdline", ret);
+			goto out;
+		}
 	}
 
-	return 0;
+	ret = 0;
 
- free_out_data:
+ out:
 	if (out_data)
 		avb_slot_verify_data_free(out_data);
+	avb_ops_free(avb_ops);
 
-	return log_msg_ret("avb cmdline", ret);
+	return ret;
 }
 #else
 static int run_avb_verification(struct bootflow *bflow)

-- 
2.53.0