From 01be7d6bcf83ff33ace34babf1acbabeb169dd63 Mon Sep 17 00:00:00 2001 From: Prasad Shetty <11435405+prashymh@users.noreply.github.com> Date: Sat, 11 Apr 2026 14:25:38 +0530 Subject: [PATCH] coverity - fix issues reported in val & pal sources - multiple issues are reported in val around out-of-bound access and out-of-bound read and pointer access, these are fixed with this patch - coverity issues reported in pal are also fixed Change-Id: I59779ce107ac7f2fd17da9b710974b489ead65af --- pal/baremetal/base/src/pal_pptt.c | 2 +- pal/uefi_acpi/src/pal_pptt.c | 2 +- pal/uefi_acpi/src/pal_timer_wd.c | 33 ++++++++++---- val/src/acs_ete.c | 71 ++++++++++++++++++++++--------- val/src/acs_iovirt.c | 2 +- val/src/acs_pe.c | 14 +++++- val/src/bsa_execute_test.c | 24 ++++++++--- 7 files changed, 108 insertions(+), 40 deletions(-) diff --git a/pal/baremetal/base/src/pal_pptt.c b/pal/baremetal/base/src/pal_pptt.c index 862fbbba..99714653 100644 --- a/pal/baremetal/base/src/pal_pptt.c +++ b/pal/baremetal/base/src/pal_pptt.c @@ -54,7 +54,7 @@ pal_cache_dump_info_table(CACHE_INFO_TABLE *CacheTable, PE_INFO_TABLE *PeTable) print(ACS_PRINT_INFO, "\nPE Index * %d *", i); print(ACS_PRINT_INFO, "\n Level 1 Cache index(s) :"); - for (j = 0; pe_entry->level_1_res[j] != DEFAULT_CACHE_IDX && j < MAX_L1_CACHE_RES; j++) { + for (j = 0; j < MAX_L1_CACHE_RES && pe_entry->level_1_res[j] != DEFAULT_CACHE_IDX; j++) { print(ACS_PRINT_INFO, " %d,", pe_entry->level_1_res[j]); } print(ACS_PRINT_INFO, "\n"); diff --git a/pal/uefi_acpi/src/pal_pptt.c b/pal/uefi_acpi/src/pal_pptt.c index d17da6ac..23f475d9 100644 --- a/pal/uefi_acpi/src/pal_pptt.c +++ b/pal/uefi_acpi/src/pal_pptt.c @@ -64,7 +64,7 @@ pal_cache_dump_info_table(CACHE_INFO_TABLE *CacheTable, PE_INFO_TABLE *PeTable) acs_print(ACS_PRINT_INFO, L"\nPE Index * %d *", i); acs_print(ACS_PRINT_INFO, L"\n Level 1 Cache index(s) :"); - for (j = 0; pe_entry->level_1_res[j] != DEFAULT_CACHE_IDX && j < MAX_L1_CACHE_RES; j++) { + for (j = 0; j < MAX_L1_CACHE_RES && pe_entry->level_1_res[j] != DEFAULT_CACHE_IDX; j++) { acs_print(ACS_PRINT_INFO, L" %d,", pe_entry->level_1_res[j]); } acs_print(ACS_PRINT_INFO, L"\n"); diff --git a/pal/uefi_acpi/src/pal_timer_wd.c b/pal/uefi_acpi/src/pal_timer_wd.c index 0f30dc2e..78a510ef 100644 --- a/pal/uefi_acpi/src/pal_timer_wd.c +++ b/pal/uefi_acpi/src/pal_timer_wd.c @@ -71,7 +71,8 @@ pal_timer_create_info_table(TIMER_INFO_TABLE *TimerTable) UINT32 i; UINT32 num_of_entries; UINT32 revision = 0; - UINT32 *virtualpl2 = NULL; + UINT8 *gtdt_ptr = NULL; + UINTN el2_virt_timer_offset; if (TimerTable == NULL) { acs_print(ACS_PRINT_ERR, L" Input Timer Table Pointer is NULL. Cannot create Timer INFO\n"); @@ -87,7 +88,9 @@ pal_timer_create_info_table(TIMER_INFO_TABLE *TimerTable) acs_print(ACS_PRINT_ERR, L" GTDT not found\n"); return; } - acs_print(ACS_PRINT_INFO, L" GTDT is at %x and length is %x\n", gGtdtHdr, gGtdtHdr->Header.Length); + gtdt_ptr = (UINT8 *)gGtdtHdr; + acs_print(ACS_PRINT_INFO, L" GTDT is at %x and length is %x\n", + gGtdtHdr, gGtdtHdr->Header.Length); revision = gGtdtHdr->Header.Revision; acs_print(ACS_PRINT_INFO, L" GTDT revision is at %d\n", revision); @@ -118,7 +121,8 @@ pal_timer_create_info_table(TIMER_INFO_TABLE *TimerTable) GtEntry->block_cntl_base = Entry->CntCtlBase; GtEntry->timer_count = Entry->GTBlockTimerCount; acs_print(ACS_PRINT_DEBUG, L" CNTCTLBase = %llx\n", GtEntry->block_cntl_base); - GtBlockTimer = (EFI_ACPI_6_1_GTDT_GT_BLOCK_TIMER_STRUCTURE *)(((UINT8 *)Entry) + Entry->GTBlockTimerOffset); + GtBlockTimer = (EFI_ACPI_6_1_GTDT_GT_BLOCK_TIMER_STRUCTURE *) + (((UINT8 *)Entry) + Entry->GTBlockTimerOffset); for (i = 0; i < GtEntry->timer_count; i++) { acs_print(ACS_PRINT_INFO, L" Found timer entry\n"); GtEntry->frame_num[i] = GtBlockTimer->GTFrameNumber; @@ -126,7 +130,9 @@ pal_timer_create_info_table(TIMER_INFO_TABLE *TimerTable) GtEntry->GtCntEl0Base[i] = GtBlockTimer->CntEL0BaseX; GtEntry->gsiv[i] = GtBlockTimer->GTxPhysicalTimerGSIV; GtEntry->virt_gsiv[i] = GtBlockTimer->GTxVirtualTimerGSIV; - GtEntry->flags[i] = GtBlockTimer->GTxPhysicalTimerFlags | (GtBlockTimer->GTxVirtualTimerFlags << 8) | (GtBlockTimer->GTxCommonFlags << 16); + GtEntry->flags[i] = GtBlockTimer->GTxPhysicalTimerFlags | + (GtBlockTimer->GTxVirtualTimerFlags << 8) | + (GtBlockTimer->GTxCommonFlags << 16); acs_print(ACS_PRINT_DEBUG, L" CNTBaseN = %llx for sys counter = %d\n", GtEntry->GtCntBase[i], i); GtBlockTimer++; @@ -141,9 +147,16 @@ pal_timer_create_info_table(TIMER_INFO_TABLE *TimerTable) }; if (revision == 3) { - virtualpl2 = &(gGtdtHdr->PlatformTimerOffset); - TimerTable->header.el2_virt_timer_gsiv = *(++virtualpl2); - TimerTable->header.el2_virt_timer_flag = *(++virtualpl2); + el2_virt_timer_offset = (UINTN)((UINT8 *)&gGtdtHdr->PlatformTimerOffset - + gtdt_ptr) + sizeof(UINT32); + if (gGtdtHdr->Header.Length >= (el2_virt_timer_offset + (2 * sizeof(UINT32)))) { + CopyMem(&TimerTable->header.el2_virt_timer_gsiv, + gtdt_ptr + el2_virt_timer_offset, + sizeof(UINT32)); + CopyMem(&TimerTable->header.el2_virt_timer_flag, + gtdt_ptr + el2_virt_timer_offset + sizeof(UINT32), + sizeof(UINT32)); + } if (TimerTable->header.el2_virt_timer_gsiv == 0) acs_print(ACS_PRINT_DEBUG, L" GTDT don't have el2 virt timer info\n"); } @@ -208,7 +221,8 @@ pal_wd_create_info_table(WD_INFO_TABLE *WdTable) } Length = gGtdtHdr->PlatformTimerOffset; - Entry = (EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE *) ((UINT8 *)gGtdtHdr + Length); + Entry = (EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE *) + ((UINT8 *)gGtdtHdr + Length); Length = sizeof (EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE); num_of_entries = gGtdtHdr->PlatformTimerCount; @@ -229,7 +243,8 @@ pal_wd_create_info_table(WD_INFO_TABLE *WdTable) WdEntry->wd_ctrl_base, WdEntry->wd_gsiv); WdEntry++; } - Entry = (EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE *) ((UINT8 *)Entry + (Entry->Length)); + Entry = (EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE *) + ((UINT8 *)Entry + (Entry->Length)); num_of_entries--; } diff --git a/val/src/acs_ete.c b/val/src/acs_ete.c index a927a9d2..df3d58c8 100644 --- a/val/src/acs_ete.c +++ b/val/src/acs_ete.c @@ -65,7 +65,7 @@ uint64_t parse_tracestream(uint8_t *trace_bytes, uint64_t trace_size) uint64_t byte_index = 0; uint64_t pkt_len = 0; uint64_t pkt_type = 0; - uint64_t length; + uint64_t length = 0; uint64_t trcidr0_read = val_pe_reg_read(TRCIDR0); uint32_t pe_index = val_pe_get_index_mpid(val_pe_get_mpid()); @@ -82,11 +82,15 @@ uint64_t parse_tracestream(uint8_t *trace_bytes, uint64_t trace_size) switch (trace_bytes[byte_index]) { case TRACE_ALIGNMENT_PKT: + if (byte_index + 1 >= trace_size) + return TRACE_PKT_INVALID; pkt_type = VAL_EXTRACT_BITS(trace_bytes[byte_index + 1], 0, 2); pkt_len = (pkt_type != 0) ? DISCARD_OVERFLOW_PKT_LEN : ALIGN_SYNC_PKT_LEN; break; case TRACE_INFO_PKT: + if (byte_index + 1 >= trace_size) + return TRACE_PKT_INVALID; pkt_len = TRACE_INFO_PKT_LEN; /* Check for CC field (bit 0) */ @@ -122,6 +126,8 @@ uint64_t parse_tracestream(uint8_t *trace_bytes, uint64_t trace_size) break; case TRACE_EXCEPTION_PKT: + if (byte_index + 2 >= trace_size) + return TRACE_PKT_INVALID; uint8_t val2_full = VAL_EXTRACT_BITS(trace_bytes[byte_index + 2], 0, 7); uint8_t val2_partial = VAL_EXTRACT_BITS(trace_bytes[byte_index + 2], 2, 7); @@ -132,6 +138,9 @@ uint64_t parse_tracestream(uint8_t *trace_bytes, uint64_t trace_size) pkt_len = (val2_full < 0x85) ? TRACE_EXCEPTION_32_PKT_LEN : TRACE_EXCEPTION_64_PKT_LEN; + if (byte_index + pkt_len > trace_size) + return TRACE_PKT_INVALID; + if (VAL_EXTRACT_BITS(trace_bytes[byte_index + (pkt_len - 1)], 6, 6)) pkt_len = pkt_len + VMID_LAYOUT_LEN; @@ -146,6 +155,8 @@ uint64_t parse_tracestream(uint8_t *trace_bytes, uint64_t trace_size) break; case EXCEPTION_SHORT_ADDR_PKT: pkt_len = EXCEPTION_SHORT_ADDR_PKT_LEN; + if (byte_index + pkt_len > trace_size) + return TRACE_PKT_INVALID; if (!(trace_bytes[byte_index + (pkt_len - 2)] & CONTINUITY_BIT_MASK)) pkt_len = pkt_len - 1; break; @@ -204,15 +215,22 @@ uint64_t parse_tracestream(uint8_t *trace_bytes, uint64_t trace_size) case CTX_32BIT_IS1_PKT: case CTX_64BIT_IS0_PKT: case CTX_64BIT_IS1_PKT: + length = 0; if (trace_bytes[byte_index] == TRACE_CONTEXT_PKT) length = 2; - if ((trace_bytes[byte_index] == CTX_32BIT_IS0_PKT) || - (trace_bytes[byte_index] == CTX_32BIT_IS1_PKT)) + else if ((trace_bytes[byte_index] == CTX_32BIT_IS0_PKT) || + (trace_bytes[byte_index] == CTX_32BIT_IS1_PKT)) length = 6; - if ((trace_bytes[byte_index] == CTX_64BIT_IS0_PKT) || - (trace_bytes[byte_index] == CTX_64BIT_IS1_PKT)) + else if ((trace_bytes[byte_index] == CTX_64BIT_IS0_PKT) || + (trace_bytes[byte_index] == CTX_64BIT_IS1_PKT)) length = 10; + if (length == 0) + return TRACE_PKT_INVALID; + + if (byte_index + length > trace_size) + return TRACE_PKT_INVALID; + switch (VAL_EXTRACT_BITS(trace_bytes[byte_index + (length - 1)], 6, 7)) { case 0: /* Packet Variant 1 */ @@ -233,6 +251,8 @@ uint64_t parse_tracestream(uint8_t *trace_bytes, uint64_t trace_size) case TARGET_ADDR_SHORT_IS0_PKT: case TARGET_ADDR_SHORT_IS1_PKT: pkt_len = TRACE_SHORT_PKT_LEN; + if (byte_index + pkt_len > trace_size) + return TRACE_PKT_INVALID; if (!(trace_bytes[byte_index + (pkt_len - 2)] & CONTINUITY_BIT_MASK)) pkt_len = pkt_len - 1; break; @@ -248,6 +268,8 @@ uint64_t parse_tracestream(uint8_t *trace_bytes, uint64_t trace_size) case Q_SHORT_ADDR_IS0_PKT: case Q_SHORT_ADDR_IS1_PKT: pkt_len = TRACE_SHORT_PKT_LEN; + if (byte_index + pkt_len > trace_size) + return TRACE_PKT_INVALID; if (!(trace_bytes[byte_index + (pkt_len - 2)] & CONTINUITY_BIT_MASK)) pkt_len = pkt_len - 1; pkt_len = pkt_len + trace_cbit_len(trace_bytes, byte_index, pkt_len, 5); @@ -266,6 +288,8 @@ uint64_t parse_tracestream(uint8_t *trace_bytes, uint64_t trace_size) case SRC_SHORT_ADDR_IS0_PKT: case SRC_SHORT_ADDR_IS1_PKT: pkt_len = TRACE_SHORT_PKT_LEN; + if (byte_index + pkt_len > trace_size) + return TRACE_PKT_INVALID; if (!(trace_bytes[byte_index + (pkt_len - 2)] & CONTINUITY_BIT_MASK)) pkt_len = pkt_len - 1; break; @@ -321,28 +345,35 @@ uint64_t val_ete_get_trace_timestamp(uint64_t buffer_address) uint32_t i = 0; uint64_t timestamp = 0; uint64_t ts_value = 0; - uint32_t ts_start_byte = 0, ts_continue = 0; + uint64_t ts_start_byte = 0; + uint32_t ts_continue = 0; + uint64_t trace_buf_size = sizeof(trace_bytes); uint32_t index = val_pe_get_index_mpid(val_pe_get_mpid()); val_memcpy(trace_bytes, (void *)buffer_address, 100); - ts_start_byte = parse_tracestream(trace_bytes, sizeof(trace_bytes)); - if (ts_start_byte == TRACE_PKT_INVALID) { - val_print_primary_pe(DEBUG, "\n ETE Parsing failed", 0, index); - return 0; - } + ts_start_byte = parse_tracestream(trace_bytes, trace_buf_size); + if ((ts_start_byte == TRACE_PKT_INVALID) || (ts_start_byte >= trace_buf_size)) { + val_print_primary_pe(DEBUG, "\n ETE Parsing failed", 0, index); + return 0; + } if ((trace_bytes[ts_start_byte] == TRACE_TIMESTAMP_V1_PKT) || (trace_bytes[ts_start_byte] == TRACE_TIMESTAMP_V2_PKT)) { - ts_continue = 1; - ts_start_byte++; - i = 0; - while (ts_continue && (i < 9)) { - ts_value = (trace_bytes[ts_start_byte + i] & TS_VALUE_MASK); - timestamp = timestamp | ((ts_value << (i * 8)) >> i); - ts_continue = (trace_bytes[ts_start_byte + i] & CONTINUITY_BIT_MASK); - i++; - } + ts_continue = 1; + ts_start_byte++; + i = 0; + while (ts_continue && (i < 9) && ((ts_start_byte + i) < trace_buf_size)) { + ts_value = (trace_bytes[ts_start_byte + i] & TS_VALUE_MASK); + timestamp = timestamp | ((ts_value << (i * 8)) >> i); + ts_continue = (trace_bytes[ts_start_byte + i] & CONTINUITY_BIT_MASK); + i++; + } + + if (ts_continue) { + val_print_primary_pe(DEBUG, "\n Timestamp packet exceeds trace buffer", 0, index); + return 0; + } } if (timestamp == 0) { diff --git a/val/src/acs_iovirt.c b/val/src/acs_iovirt.c index b7ba3d6e..e850b38b 100644 --- a/val/src/acs_iovirt.c +++ b/val/src/acs_iovirt.c @@ -489,7 +489,7 @@ val_iovirt_get_named_comp_device_info(const char *dev_name, uint32_t identifier, while ((dev_len < MAX_NAMED_COMP_LENGTH) && (dev_name[dev_len] != '\0')) dev_len++; - if (dev_len == 0u) { + if ((dev_len == 0u) || (dev_len >= MAX_NAMED_COMP_LENGTH)) { val_print(ERROR, "\n GET_DEVICE_ID: Invalid device name"); return ACS_STATUS_ERR; } diff --git a/val/src/acs_pe.c b/val/src/acs_pe.c index d2431ba9..17d603cb 100644 --- a/val/src/acs_pe.c +++ b/val/src/acs_pe.c @@ -543,7 +543,14 @@ uint64_t val_cache_get_info(CACHE_INFO_e type, uint32_t cache_index) { CACHE_INFO_ENTRY *entry; - char *cache_info_type[] = {"cache_type", "cache_size", "cache_identifier", "associativity"}; + static const char *cache_info_type[] = { + "cache_type", + "cache_size", + "cache_identifier", + "next_level_index", + "private_flag", + "associativity" + }; if (cache_index >= g_cache_info_table->num_of_cache) { val_print(ERROR, "\n invalid cache index: %d", cache_index); @@ -579,7 +586,10 @@ val_cache_get_info(CACHE_INFO_e type, uint32_t cache_index) val_print(ERROR, "\n cache %d has invalid ", cache_index); - val_print(ERROR, cache_info_type[type]); + if (type < (sizeof(cache_info_type) / sizeof(cache_info_type[0]))) + val_print(ERROR, cache_info_type[type]); + else + val_print(ERROR, "cache_info_type"); return INVALID_CACHE_INFO; } diff --git a/val/src/bsa_execute_test.c b/val/src/bsa_execute_test.c index 0a92a2a1..e5205ad0 100644 --- a/val/src/bsa_execute_test.c +++ b/val/src/bsa_execute_test.c @@ -43,21 +43,33 @@ extern uint32_t g_its_init; #define OPERATING_SYSTEM 0 #define HYPERVISOR 1 #define PLATFORM_SECURITY 2 +#define VIEW_COUNT 3 -#define MODULE_END 3 +/* command to reset view information */ +#define MODULE_END VIEW_COUNT + +static void view_reset_info(uint32_t *view_check) +{ + view_check[OPERATING_SYSTEM] = 0; + view_check[HYPERVISOR] = 0; + view_check[PLATFORM_SECURITY] = 0; +} void view_print_info(uint32_t view) { - static uint32_t view_check[3] = {0, 0, 0}; + static uint32_t view_check[VIEW_COUNT] = {0, 0, 0}; + + if (view > MODULE_END) + return; if (view == MODULE_END) { - //memset(view_check, 0, sizeof(view_check)); //Need to include string.h - view_check[OPERATING_SYSTEM] = 0; - view_check[HYPERVISOR] = 0; - view_check[PLATFORM_SECURITY] = 0; + view_reset_info(view_check); return; } + if (view >= VIEW_COUNT) + return; + if (view_check[view] == 1) return; else