From 9270e0586fd116fee117215d6a025cc4708e8d19 Mon Sep 17 00:00:00 2001 From: "Ni, Erchang" Date: Wed, 31 Jul 2024 07:52:15 +0000 Subject: [PATCH] Fix Coverity high impact issues under kernelflinger component 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 --- ...eaks-and-Uninitialized-pointer-reads.patch | 196 +++++++++++++++ ...source-leaks-and-32-bit-time_t-usage.patch | 223 ++++++++++++++++++ ...t-of-bounds-access-and-32-bit-time_t.patch | 69 ++++++ 3 files changed, 488 insertions(+) create mode 100644 bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0056-Fix-Resource-leaks-and-Uninitialized-pointer-reads.patch create mode 100644 bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0057-Fix-Resource-leaks-and-32-bit-time_t-usage.patch create mode 100644 bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0058-Fix-Out-of-bounds-access-and-32-bit-time_t.patch diff --git a/bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0056-Fix-Resource-leaks-and-Uninitialized-pointer-reads.patch b/bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0056-Fix-Resource-leaks-and-Uninitialized-pointer-reads.patch new file mode 100644 index 0000000000..a08b0c2ded --- /dev/null +++ b/bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0056-Fix-Resource-leaks-and-Uninitialized-pointer-reads.patch @@ -0,0 +1,196 @@ +From 3481a459859289053d794beae31ed7beb31f4ff2 Mon Sep 17 00:00:00 2001 +From: "Ni, Erchang" +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 +--- + 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 + diff --git a/bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0057-Fix-Resource-leaks-and-32-bit-time_t-usage.patch b/bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0057-Fix-Resource-leaks-and-32-bit-time_t-usage.patch new file mode 100644 index 0000000000..10eb6a9e5b --- /dev/null +++ b/bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0057-Fix-Resource-leaks-and-32-bit-time_t-usage.patch @@ -0,0 +1,223 @@ +From 81e55dc41d2752c156123f6d7263df76a386575f Mon Sep 17 00:00:00 2001 +From: "Ni, Erchang" +Date: Fri, 5 Jul 2024 08:41:14 +0000 +Subject: [PATCH 2/2] Fix Resource leaks and 32-bit time_t usage + +Tracked-On: OAM-121940 +Signed-off-by: Ni, Erchang +--- + libadb/lspartition.c | 5 ++++- + libfastboot/fastboot.c | 13 ++++++++++--- + libkernelflinger/android.c | 9 ++++++++- + libkernelflinger/efilinux.c | 4 +++- + libkernelflinger/storage.c | 4 +++- + libsslsupport/wrapper.c | 34 +++++++++++++++++++--------------- + 6 files changed, 47 insertions(+), 22 deletions(-) + +diff --git a/libadb/lspartition.c b/libadb/lspartition.c +index 66ec68e..1ea6f41 100644 +--- a/libadb/lspartition.c ++++ b/libadb/lspartition.c +@@ -48,8 +48,11 @@ static EFI_STATUS lspartition_main(INTN argc, + + + ret = gpt_list_partition(&gparti, &part_count, LOGICAL_UNIT_USER); +- if (EFI_ERROR(ret) || part_count == 0) ++ if (EFI_ERROR(ret) || part_count == 0) { ++ if (gparti) ++ FreePool(gparti); + return EFI_SUCCESS; ++ } + + for (i = 0; i < part_count; i++) + max_len = max(max_len, StrLen(gparti[i].part.name)); +diff --git a/libfastboot/fastboot.c b/libfastboot/fastboot.c +index e888b54..30e7df1 100644 +--- a/libfastboot/fastboot.c ++++ b/libfastboot/fastboot.c +@@ -522,8 +522,11 @@ static EFI_STATUS publish_partsize(void) + UINTN i; + + ret = gpt_list_partition(&gparti, &part_count, LOGICAL_UNIT_USER); +- if (EFI_ERROR(ret) || part_count == 0) ++ if (EFI_ERROR(ret) || part_count == 0) { ++ if (gparti) ++ FreePool(gparti); + return EFI_SUCCESS; ++ } + + for (i = 0; i < part_count; i++) { + UINT64 size; +@@ -538,12 +541,16 @@ static EFI_STATUS publish_partsize(void) + /* stay compatible with userdata/data naming */ + if (!StrCmp(gparti[i].part.name, L"data")) { + ret = publish_part(L"userdata", size, &gparti[i].part.type); +- if (EFI_ERROR(ret)) ++ if (EFI_ERROR(ret)) { ++ FreePool(gparti); + return ret; ++ } + } else if (!StrCmp(gparti[i].part.name, L"userdata")) { + ret = publish_part(L"data", size, &gparti[i].part.type); +- if (EFI_ERROR(ret)) ++ if (EFI_ERROR(ret)) { ++ FreePool(gparti); + return ret; ++ } + } + } + +diff --git a/libkernelflinger/android.c b/libkernelflinger/android.c +index 2cdb769..ad38296 100644 +--- a/libkernelflinger/android.c ++++ b/libkernelflinger/android.c +@@ -1293,7 +1293,7 @@ static EFI_STATUS setup_command_line( + char *serialno = NULL; + CHAR16 *serialport = NULL; + CHAR16 *bootreason = NULL; +- EFI_PHYSICAL_ADDRESS cmdline_addr; ++ EFI_PHYSICAL_ADDRESS cmdline_addr = 0; + CHAR8 *cmdline; + CHAR8 *cmd_conf= NULL; + UINTN cmdlen; +@@ -1651,6 +1651,13 @@ out: + if (bootreason) { + FreePool(bootreason); + } ++ if (EFI_ERROR(ret) && cmdline_addr) { ++ if (is_uefi) { ++ free_pages(cmdline_addr, EFI_SIZE_TO_PAGES(cmdsize)); ++ } else { ++ FreePool((void *)(UINTN)cmdline_addr); ++ } ++ } + return ret; + } + +diff --git a/libkernelflinger/efilinux.c b/libkernelflinger/efilinux.c +index 0d5b66e..452d2be 100644 +--- a/libkernelflinger/efilinux.c ++++ b/libkernelflinger/efilinux.c +@@ -129,8 +129,10 @@ EFI_STATUS emalloc(UINTN size, UINTN align, EFI_PHYSICAL_ADDRESS *addr, BOOLEAN + if (d == map_end) + err = EFI_OUT_OF_RESOURCES; + +- free_pool(map_buf); + fail: ++ if (map_buf) { ++ free_pool(map_buf); ++ } + return err; + } + +diff --git a/libkernelflinger/storage.c b/libkernelflinger/storage.c +index 8776c8d..966c52b 100644 +--- a/libkernelflinger/storage.c ++++ b/libkernelflinger/storage.c +@@ -546,7 +546,9 @@ EFI_STATUS storage_get_erase_block_size(UINTN *erase_blk_size) + if (i == nb_handle) + goto notfound; + +- return cur_storage->get_erase_block_size(handles[i], erase_blk_size); ++ ret = cur_storage->get_erase_block_size(handles[i], erase_blk_size); ++ FreePool(handles); ++ return ret; + } + + notfound: +diff --git a/libsslsupport/wrapper.c b/libsslsupport/wrapper.c +index ecfa4de..a6e2a13 100644 +--- a/libsslsupport/wrapper.c ++++ b/libsslsupport/wrapper.c +@@ -1,6 +1,7 @@ + #include + #include + #include ++#include + #include "openssl_support.h" + + FILE *__sF = NULL; +@@ -399,11 +400,11 @@ struct tm + char *tm_zone; /* Timezone abbreviation. */ + }; + +-struct tm *gmtime_r(const time_t *timep, struct tm *tmp) ++struct tm *gmtime_r(const int64_t *timep, struct tm *tmp) + __attribute__((weak)); +-struct tm *gmtime_r(const time_t *timep, struct tm *tmp) ++struct tm *gmtime_r(const int64_t *timep, struct tm *tmp) + { +- time_t tdays; ++ int64_t tdays; + int idays; /* unsigned would be so 2003 */ + long long rem; + int y; +@@ -414,12 +415,12 @@ struct tm *gmtime_r(const time_t *timep, struct tm *tmp) + rem = (long long) (*timep - tdays * SECSPERDAY); + while (tdays < 0 || tdays >= year_lengths[isleap(y)]) { + int newy; +- time_t tdelta; ++ int64_t tdelta; + int idelta; + int leapdays; + + tdelta = tdays / DAYSPERLYEAR; +- if (! ((! TYPE_SIGNED(time_t) || INT_MIN <= tdelta) ++ if (! ((! TYPE_SIGNED(int64_t) || INT_MIN <= tdelta) + && tdelta <= INT_MAX)) + return NULL; + idelta = tdelta; +@@ -430,7 +431,7 @@ struct tm *gmtime_r(const time_t *timep, struct tm *tmp) + return NULL; + leapdays = leaps_thru_end_of(newy - 1) - + leaps_thru_end_of(y - 1); +- tdays -= (time_t) (((time_t) newy - y) * DAYSPERNYEAR); ++ tdays -= (int64_t) (((int64_t) newy - y) * DAYSPERNYEAR); + tdays -= leapdays; + y = newy; + } +@@ -528,29 +529,32 @@ static UINTN CumulativeDays[2][14] = { + } + }; + +-time_t time(time_t *timer) ++int64_t time(int64_t *timer) + __attribute__((weak)); +-time_t time(time_t *timer) ++int64_t time(int64_t *timer) + { + EFI_TIME Time; + UINTN Year; + ++ /* Defaultly set timezone as EFI_UNSPECIFIED_TIMEZONE */ ++ Time.TimeZone = EFI_UNSPECIFIED_TIMEZONE; ++ + /* Get the current time and date information */ + uefi_call_wrapper(RT->GetTime, 2, &Time, NULL); + + /* Years Handling + * UTime should now be set to 00:00:00 on Jan 1 of the current year. */ + for (Year = 1970, *timer = 0; Year != Time.Year; Year++) +- *timer = *timer + (time_t)(CumulativeDays[isleap(Year)][13] * SECSPERDAY); ++ *timer = *timer + (int64_t)(CumulativeDays[isleap(Year)][13] * SECSPERDAY); + + /* Add in number of seconds for current Month, Day, Hour, Minute, Seconds, and TimeZone adjustment */ + *timer = *timer + +- (time_t)((Time.TimeZone != EFI_UNSPECIFIED_TIMEZONE) ? (Time.TimeZone * 60) : 0) + +- (time_t)(CumulativeDays[isleap(Time.Year)][Time.Month] * SECSPERDAY) + +- (time_t)(((Time.Day > 0) ? (time_t)Time.Day - 1 : 0) * SECSPERDAY) + +- (time_t)((time_t)Time.Hour * SECSPERHOUR) + +- (time_t)((time_t)Time.Minute * 60) + +- (time_t)Time.Second; ++ (int64_t)((Time.TimeZone != EFI_UNSPECIFIED_TIMEZONE) ? (Time.TimeZone * 60) : 0) + ++ (int64_t)(CumulativeDays[isleap(Time.Year)][Time.Month] * SECSPERDAY) + ++ (int64_t)(((Time.Day > 0) ? (int64_t)Time.Day - 1 : 0) * SECSPERDAY) + ++ (int64_t)((int64_t)Time.Hour * SECSPERHOUR) + ++ (int64_t)((int64_t)Time.Minute * 60) + ++ (int64_t)Time.Second; + + return *timer; + } +-- +2.34.1 + diff --git a/bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0058-Fix-Out-of-bounds-access-and-32-bit-time_t.patch b/bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0058-Fix-Out-of-bounds-access-and-32-bit-time_t.patch new file mode 100644 index 0000000000..a9fe2c61a2 --- /dev/null +++ b/bsp_diff/aaos_iasw/hardware/intel/kernelflinger/0058-Fix-Out-of-bounds-access-and-32-bit-time_t.patch @@ -0,0 +1,69 @@ +From ebeec11be4f3d473617b318f6a4d1351b9afb3c7 Mon Sep 17 00:00:00 2001 +From: "Ni, Erchang" +Date: Tue, 9 Jul 2024 06:58:38 +0000 +Subject: [PATCH] Fix Out-of-bounds access and 32-bit time_t + +Tracked-On: OAM-122082 +Signed-off-by: Ni, Erchang +--- + kernelflinger.c | 2 ++ + libfastboot/fastboot_transport.c | 2 +- + libkernelflinger/android.c | 2 +- + libkernelflinger/log.c | 2 +- + 4 files changed, 5 insertions(+), 3 deletions(-) + +diff --git a/kernelflinger.c b/kernelflinger.c +index c411798..5378384 100644 +--- a/kernelflinger.c ++++ b/kernelflinger.c +@@ -620,6 +620,8 @@ static enum boot_target check_command_line() + ret = str_to_stra((CHAR8 *)arg8, argv[i], arglen + 1); + if (EFI_ERROR(ret)) { + efi_perror(ret, L"Non-ascii characters in command line"); ++ FreePool(argv); ++ FreePool(options); + return FASTBOOT; + } + +diff --git a/libfastboot/fastboot_transport.c b/libfastboot/fastboot_transport.c +index feb9645..571c737 100644 +--- a/libfastboot/fastboot_transport.c ++++ b/libfastboot/fastboot_transport.c +@@ -220,7 +220,7 @@ EFI_STATUS fastboot_tcp_write(void *buf, UINT32 size) + } + + *((UINT64 *)write_buf) = htobe64(size); +- ret = memcpy_s(write_buf + sizeof(UINT64), sizeof(write_buf), buf, size); ++ ret = memcpy_s(write_buf + sizeof(UINT64), sizeof(write_buf) - sizeof(UINT64), buf, size); + if (EFI_ERROR(ret)) + return ret; + +diff --git a/libkernelflinger/android.c b/libkernelflinger/android.c +index ad38296..fbbe3d9 100644 +--- a/libkernelflinger/android.c ++++ b/libkernelflinger/android.c +@@ -952,7 +952,7 @@ static CHAR16 *get_command_line(IN struct boot_img_hdr *aosp_header, + /* legacy boot.img format cmdline is NUL terminated */ + if (!aosp_header->cmdline[BOOT_ARGS_SIZE - 1]) + offset--; +- ret = memcpy_s(full_cmdline + offset, sizeof(full_cmdline), ++ ret = memcpy_s(full_cmdline + offset, sizeof(full_cmdline) - offset, + aosp_header->extra_cmdline, BOOT_EXTRA_ARGS_SIZE); + if (EFI_ERROR(ret)) + goto failed; +diff --git a/libkernelflinger/log.c b/libkernelflinger/log.c +index 3d54ef8..42f66dc 100644 +--- a/libkernelflinger/log.c ++++ b/libkernelflinger/log.c +@@ -119,7 +119,7 @@ static void log_append_to_buffer(CHAR8 *msg, UINTN length) + pos = 0; + } + +- ret = memcpy_s(log_buf + pos, sizeof(log_buf), msg, length); ++ ret = memcpy_s(log_buf + pos, sizeof(log_buf) - pos, msg, length); + if (EFI_ERROR(ret)) + return; + +-- +2.34.1 +