Skip to content

Commit

Permalink
Fix Coverity high impact issues under kernelflinger component
Browse files Browse the repository at this point in the history 
Port coverity related patches from base_aaos to aaos_iasw,
which solved resource leaks, uninitialized pointer reads,
32-bit time_t usage and Out-of-bounds access.

Tracked-On: OAM-122919
Signed-off-by: Ni, Erchang <[email protected]>
  • Loading branch information
@Francesca0901 @sysopenci
Francesca0901 authored and sysopenci committed Aug 2, 2024
1 parent a089ca1 commit 9270e05
Show file tree
Hide file tree
Showing 3 changed files with 488 additions and 0 deletions.
196 changes: 196 additions & 0 deletions ...ardware/intel/kernelflinger/0056-Fix-Resource-leaks-and-Uninitialized-pointer-reads.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
From 3481a459859289053d794beae31ed7beb31f4ff2 Mon Sep 17 00:00:00 2001
From: "Ni, Erchang" <[email protected]>
Date: Fri, 5 Jul 2024 08:35:31 +0000
Subject: [PATCH 1/2] Fix Resource leaks and uninitialized pointer reads

Tracked-On: OAM-121940
Signed-off-by: Ni, Erchang <[email protected]>
---
avb/libavb/avb_cmdline.c | 5 +++++
libfastboot/bootmgr.c | 1 +
libfastboot/fastboot.c | 1 +
libkernelflinger/android.c | 7 +++++--
libkernelflinger/lib.c | 11 ++++++++++-
libkernelflinger/security_sbl.c | 1 +
libkernelflinger/vars.c | 25 +++++++++++++++++++++----
7 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/avb/libavb/avb_cmdline.c b/avb/libavb/avb_cmdline.c
index bf10865..0fad4d2 100644
--- a/avb/libavb/avb_cmdline.c
+++ b/avb/libavb/avb_cmdline.c
@@ -364,6 +364,11 @@ AvbSlotVerifyResult avb_append_options(
// remapped by avb_manage_hashtree_error_mode().
avb_assert_not_reached();
break;
+ default:
+ // Handle unexpected value of resolved_hashtree_error_mode
+ // avoid dm_verity_mode and verity_mode being uninitialized
+ avb_assert_not_reached();
+ break;
}
new_ret = avb_replace(
slot_data->cmdline, "$(ANDROID_VERITY_MODE)", dm_verity_mode);
diff --git a/libfastboot/bootmgr.c b/libfastboot/bootmgr.c
index 8d889c8..c715e24 100644
--- a/libfastboot/bootmgr.c
+++ b/libfastboot/bootmgr.c
@@ -142,6 +142,7 @@ static EFI_STATUS find_load_option_entry(CHAR16 *description, UINT16 *entry)
goto exit;

*entry = strtoul((char *)number, NULL, 16);
+ FreePool(name);
return EFI_SUCCESS;
}

diff --git a/libfastboot/fastboot.c b/libfastboot/fastboot.c
index 036c883..e888b54 100644
--- a/libfastboot/fastboot.c
+++ b/libfastboot/fastboot.c
@@ -890,6 +890,7 @@ static void cmd_erase(INTN argc, CHAR8 **argv)
}

if (!can_erase_or_flash_partition(label)) {
+ FreePool(label);
fastboot_fail("Currently virtual a/b ota is in progress...");
return;
}
diff --git a/libkernelflinger/android.c b/libkernelflinger/android.c
index f1673bc..2cdb769 100644
--- a/libkernelflinger/android.c
+++ b/libkernelflinger/android.c
@@ -1907,9 +1907,10 @@ EFI_STATUS android_image_load_file(
return ret;
}
fileinfo = AllocatePool(buffersize);
- if (!fileinfo)
+ if (!fileinfo) {
FreePool(path);
return EFI_OUT_OF_RESOURCES;
+ }

ret = uefi_call_wrapper(imagefile->GetInfo, 4, imagefile,
&EfiFileInfoId, &buffersize, fileinfo);
@@ -1918,9 +1919,11 @@ EFI_STATUS android_image_load_file(
* the request */
FreePool(fileinfo);
fileinfo = AllocatePool(buffersize);
- if (!fileinfo)
+ if (!fileinfo) {
FreePool(path);
return EFI_OUT_OF_RESOURCES;
+ }
+
ret = uefi_call_wrapper(imagefile->GetInfo, 4, imagefile,
&EfiFileInfoId, &buffersize, fileinfo);
}
diff --git a/libkernelflinger/lib.c b/libkernelflinger/lib.c
index 6c3e6fc..cb6762f 100644
--- a/libkernelflinger/lib.c
+++ b/libkernelflinger/lib.c
@@ -844,10 +844,19 @@ CHAR16 *get_efi_variable_str8(const EFI_GUID *guid, CHAR16 *key)
CHAR16 *value;
EFI_STATUS ret;
UINTN size;
+ BOOLEAN dataFreeable = FALSE;

ret = get_efi_variable(guid, key, &size, (VOID **)&data, NULL);
- if (EFI_ERROR(ret) || !data || !size)
+
+ if (!EFI_ERROR(ret) && data && size) {
+ dataFreeable = TRUE;
+ }
+ if (EFI_ERROR(ret) || !data || !size) {
+ if (dataFreeable && data) {
+ FreePool(data);
+ }
return NULL;
+ }

if (data[size - 1] != '\0') {
FreePool(data);
diff --git a/libkernelflinger/security_sbl.c b/libkernelflinger/security_sbl.c
index da33dec..9226ca8 100644
--- a/libkernelflinger/security_sbl.c
+++ b/libkernelflinger/security_sbl.c
@@ -182,6 +182,7 @@ BOOLEAN is_platform_secure_boot_enabled(VOID)

debug(L"Getting platform secure boot to value[%d], size[%d]", value, cursize);

+ FreePool(curdata);
return value == 1;
}

diff --git a/libkernelflinger/vars.c b/libkernelflinger/vars.c
index ae33242..889a9ad 100644
--- a/libkernelflinger/vars.c
+++ b/libkernelflinger/vars.c
@@ -254,6 +254,7 @@ static EFI_STATUS read_device_state_efi(UINT8 *state)
}
if (!dsize) {
error(L"Read device state from EFI variable, but data size is 0");
+ FreePool(stored_state);
ret = EFI_COMPROMISED_DATA;
return ret;
}
@@ -547,10 +548,14 @@ EFI_STATUS get_watchdog_status(UINT8 *counter, EFI_TIME *time)
if (EFI_ERROR(ret))
return ret;

- if (size != sizeof(*time))
+ if (size != sizeof(*time)) {
+ FreePool(tmp);
return EFI_COMPROMISED_DATA;
+ }

- return memcpy_s(time, sizeof(*time), tmp, size);
+ ret = memcpy_s(time, sizeof(*time), tmp, size);
+ FreePool(tmp);
+ return ret;
}

EFI_STATUS reset_watchdog_status(VOID)
@@ -798,10 +803,19 @@ char *get_serialno_var()
CHAR8 *data;
EFI_STATUS ret;
UINTN size;
+ BOOLEAN dataFreeable = FALSE;

ret = get_efi_variable(&loader_guid, SERIAL_NUM_VAR, &size, (VOID **)&data,NULL);
- if (EFI_ERROR(ret) || !data || !size)
+
+ if (!EFI_ERROR(ret) && data && size) {
+ dataFreeable = TRUE;
+ }
+ if (EFI_ERROR(ret) || !data || !size) {
+ if (dataFreeable && data) {
+ FreePool(data);
+ }
return NULL;
+ }
if (data[size - 1] != '\0') {
FreePool(data);
return NULL;
@@ -977,8 +991,10 @@ EFI_STATUS read_efi_rollback_index(UINTN rollback_index_slot, uint64_t* out_roll
}
debug(L"Success to read EFI variable %s: 0x%llx ", name, *(uint64_t *)data);

- if (size != sizeof(*out_rollback_index))
+ if (size != sizeof(*out_rollback_index)) {
+ FreePool(data);
return EFI_COMPROMISED_DATA;
+ }

*out_rollback_index = *(uint64_t *)data;

@@ -1037,6 +1053,7 @@ EFI_STATUS get_efi_loaded_slot_failed(UINT8 slot, EFI_STATUS *error)

if (size != sizeof(error)) {
debug(L"The sizeof %s is not %d", name, size);
+ FreePool(data);
return EFI_COMPROMISED_DATA;
}
*error = *((EFI_STATUS *)data);
--
2.34.1

Loading

0 comments on commit 9270e05

Please sign in to comment.